Skip to content

Commit 4041fd1

Browse files
committed
fix: handle race condition in parallel batches with swr
fix #144
1 parent 93a46a9 commit 4041fd1

File tree

1 file changed

+29
-19
lines changed

1 file changed

+29
-19
lines changed

src/getCachedValue.ts

Lines changed: 29 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { Context, CacheEntry } from './common';
1+
import { Context, CacheEntry, CachifiedOptions } from './common';
22
import { assertCacheEntry } from './assertCacheEntry';
33
import { HANDLE } from './common';
44
import { isExpired } from './isExpired';
@@ -32,8 +32,9 @@ export async function getCachedValue<Value>(
3232
staleWhileRevalidate,
3333
staleRefreshTimeout,
3434
metadata,
35-
getFreshValue: { [HANDLE]: handle },
35+
getFreshValue,
3636
} = context;
37+
3738
try {
3839
const cached = await getCacheEntry(context, report);
3940

@@ -52,24 +53,33 @@ export async function getCachedValue<Value>(
5253
}
5354

5455
if (staleRefresh) {
56+
const staleRefreshOptions: CachifiedOptions<Value> = {
57+
...context,
58+
async getFreshValue({ metadata }) {
59+
/* TODO: When staleRefreshTimeout option is removed we should
60+
also remove this or set it to ~0-200ms depending on ttl values.
61+
The intention of the delay is to not take sync resources for
62+
background refreshing – still we need to queue the refresh
63+
directly so that the de-duplication works.
64+
See https://github.com/epicweb-dev/cachified/issues/132 */
65+
await sleep(staleRefreshTimeout);
66+
report({ name: 'refreshValueStart' });
67+
return getFreshValue({
68+
metadata,
69+
background: true,
70+
});
71+
},
72+
forceFresh: true,
73+
fallbackToCache: false,
74+
};
75+
76+
// pass down batch handle when present
77+
// https://github.com/epicweb-dev/cachified/issues/144
78+
staleRefreshOptions.getFreshValue[HANDLE] = context.getFreshValue[HANDLE];
79+
5580
// refresh cache in background so future requests are faster
5681
context.waitUntil(
57-
cachified({
58-
...context,
59-
async getFreshValue({ metadata }) {
60-
/* TODO: When staleRefreshTimeout option is removed we should
61-
also remove this or set it to ~0-200ms depending on ttl values.
62-
The intention of the delay is to not take sync resources for
63-
background refreshing – still we need to queue the refresh
64-
directly so that the de-duplication works.
65-
See https://github.com/epicweb-dev/cachified/issues/132 */
66-
await sleep(staleRefreshTimeout);
67-
report({ name: 'refreshValueStart' });
68-
return context.getFreshValue({ metadata, background: true });
69-
},
70-
forceFresh: true,
71-
fallbackToCache: false,
72-
})
82+
cachified(staleRefreshOptions)
7383
.then((value) => {
7484
report({ name: 'refreshValueSuccess', value });
7585
})
@@ -89,7 +99,7 @@ export async function getCachedValue<Value>(
8999
});
90100
if (!staleRefresh) {
91101
// Notify batch that we handled this call using cached value
92-
handle?.();
102+
getFreshValue[HANDLE]?.();
93103
}
94104

95105
if (valueCheck.migrated) {

0 commit comments

Comments
 (0)