Skip to content

Commit fce5d1e

Browse files
committed
fix(loaders): restore context in sequential awaits
Close #2653 Close #2652
1 parent 9ab4cbf commit fce5d1e

File tree

3 files changed

+72
-7
lines changed

3 files changed

+72
-7
lines changed

packages/router/src/experimental/data-loaders/defineColadaLoader.ts

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -506,14 +506,17 @@ export function defineColadaLoader<Data>(
506506

507507
// load ensures there is a pending load
508508
const promise = entry
509-
.pendingLoad!.then(() => {
509+
.pendingLoad!.then(() =>
510510
// nested loaders might wait for all loaders to be ready before setting data
511511
// so we need to return the staged value if it exists as it will be the latest one
512-
return entry.staged === STAGED_NO_VALUE ? ext.data.value : entry.staged
513-
})
512+
entry.staged === STAGED_NO_VALUE ? ext.data.value : entry.staged
513+
)
514514
// we only want the error if we are nesting the loader
515515
// otherwise this will end up in "Unhandled promise rejection"
516-
.catch(e => (parentEntry ? Promise.reject(e) : null))
516+
.catch((e: unknown) => (parentEntry ? Promise.reject(e) : null))
517+
// restore the caller's context so that sequential awaits
518+
// (e.g. await useOne(); await useTwo()) preserve the parent
519+
.finally(() => setCurrentContext(currentContext))
517520

518521
// Restore the context to avoid sequential calls to be nested
519522
setCurrentContext(currentContext)

packages/router/src/experimental/data-loaders/defineLoader.ts

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -397,14 +397,17 @@ export function defineBasicLoader<Data>(
397397

398398
// load ensures there is a pending load
399399
const promise = entry
400-
.pendingLoad!.then(() => {
400+
.pendingLoad!.then(() =>
401401
// nested loaders might wait for all loaders to be ready before setting data
402402
// so we need to return the staged value if it exists as it will be the latest one
403-
return entry.staged === STAGED_NO_VALUE ? data.value : entry.staged
404-
})
403+
entry.staged === STAGED_NO_VALUE ? data.value : entry.staged
404+
)
405405
// we only want the error if we are nesting the loader
406406
// otherwise this will end up in "Unhandled promise rejection"
407407
.catch((e: unknown) => (parentEntry ? Promise.reject(e) : null))
408+
// restore the caller's context so that sequential awaits
409+
// (e.g. await useOne(); await useTwo()) preserve the parent
410+
.finally(() => setCurrentContext(currentContext))
408411

409412
setCurrentContext(currentContext)
410413
return Object.assign(promise, useDataLoaderResult)

packages/router/src/tests/data-loaders/tester.ts

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1302,6 +1302,65 @@ export function testDefineLoader<Context = void>(
13021302
expect(data.value).toEqual('one,two')
13031303
})
13041304

1305+
it('preserves context across sequential nested loader awaits', async () => {
1306+
const l1 = mockedLoader({ key: 'one' })
1307+
const l2 = mockedLoader({ key: 'two' })
1308+
1309+
const useLoaderThree = loaderFactory({
1310+
fn: async () => {
1311+
const one = await l1.loader()
1312+
const two = await l2.loader()
1313+
return `${one},${two}`
1314+
},
1315+
key: 'three',
1316+
})
1317+
1318+
const router = getRouter()
1319+
router.addRoute({
1320+
name: '_test',
1321+
path: '/fetch',
1322+
component: defineComponent({
1323+
setup() {
1324+
const { data, error, isLoading } = useLoaderThree()
1325+
1326+
return { data, error, isLoading }
1327+
},
1328+
template: `<p>{{ isLoading }}|{{ error ?? '<no>' }}|{{ data }}</p>`,
1329+
}),
1330+
meta: {
1331+
// All three exported as root loaders (like when all are exported from page component)
1332+
// The nested loaders are listed BEFORE the parent to trigger the bug:
1333+
// the guard calls load() for l1/l2 as roots with context [],
1334+
// so their .finally() restores context to [] instead of the parent's context
1335+
loaders: [l1.loader, l2.loader, useLoaderThree],
1336+
nested: { foo: 'bar' },
1337+
},
1338+
})
1339+
const wrapper = mount(RouterViewMock, {
1340+
global: {
1341+
plugins: [
1342+
[DataLoaderPlugin, { router }],
1343+
...(plugins?.(customContext!) || []),
1344+
router,
1345+
],
1346+
},
1347+
})
1348+
1349+
const p = router.push('/fetch')
1350+
await vi.advanceTimersByTimeAsync(0)
1351+
l1.resolve('1')
1352+
await vi.advanceTimersByTimeAsync(0)
1353+
// If context is lost after l1 resolves, l2.loader() inside useLoaderThree's fn
1354+
// would throw because router is undefined (no parent context)
1355+
l2.resolve('2')
1356+
await vi.advanceTimersByTimeAsync(0)
1357+
// Navigation should complete without errors
1358+
await expect(p).resolves.not.toThrow()
1359+
expect(l1.spy).toHaveBeenCalledTimes(1)
1360+
expect(l2.spy).toHaveBeenCalledTimes(1)
1361+
expect(wrapper.text()).toBe('false|<no>|1,2')
1362+
})
1363+
13051364
it('fetches once with lazy across components', async () => {
13061365
const router = getRouter()
13071366
router.addRoute({

0 commit comments

Comments
 (0)