From 57e834e941788b44129b57a2416c1499ddd4f560 Mon Sep 17 00:00:00 2001 From: Eduardo San Martin Morote Date: Mon, 1 Jun 2026 12:43:28 +0200 Subject: [PATCH] fix(data-loaders): ignore leaked loader context in components MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A still-pending lazy nested loader leaves the module-level loader context set to its own entry. When a component renders during that window, `useDataLoader()` in `setup()` reads the stale context as its parent, which re-runs the parent loader, nests a loader under itself ("… has itself as parent") and can spin into an infinite render loop. In a component, loaders are always used at the root, so drop any active context unless we are synchronously inside a loader function (genuine nesting). A small depth counter (`runInsideLoaderFn`/`isInsideLoaderFn`) keeps real nested calls working even when triggered from a component. Closes #2684 --- .../data-loaders/defineColadaLoader.ts | 24 +++-- .../experimental/data-loaders/defineLoader.ts | 18 +++- .../data-loaders/entries/index.ts | 2 + .../src/experimental/data-loaders/utils.ts | 35 ++++++++ .../router/src/tests/data-loaders/tester.ts | 87 +++++++++++++++++++ 5 files changed, 159 insertions(+), 7 deletions(-) diff --git a/packages/router/src/experimental/data-loaders/defineColadaLoader.ts b/packages/router/src/experimental/data-loaders/defineColadaLoader.ts index 204dd10e0..99151685d 100644 --- a/packages/router/src/experimental/data-loaders/defineColadaLoader.ts +++ b/packages/router/src/experimental/data-loaders/defineColadaLoader.ts @@ -19,11 +19,13 @@ import { NavigationResult, assign, getCurrentContext, + isInsideLoaderFn, isSubsetOf, + runInsideLoaderFn, setCurrentContext, trackRoute, } from './entries/index' -import { type ShallowRef, shallowRef, watch } from 'vue' +import { type ShallowRef, getCurrentInstance, shallowRef, watch } from 'vue' import { type EntryKey, type UseQueryOptions, @@ -133,10 +135,14 @@ export function defineColadaLoader( // needed for automatic refetching and nested loaders // https://github.com/posva/unplugin-vue-router/issues/583 return router[APP_KEY].runWithContext(() => - loader(trackedRoute, { - // TODO: provide the query signal too - signal: route.meta[ABORT_CONTROLLER_KEY]?.signal, - }) + // mark that we are synchronously inside a loader function so nested + // loaders can tell apart their parent context from a leaked one + runInsideLoaderFn(() => + loader(trackedRoute, { + // TODO: provide the query signal too + signal: route.meta[ABORT_CONTROLLER_KEY]?.signal, + }) + ) ) }, key: () => toValueWithParameters(options.key, entry.route.value), @@ -399,6 +405,14 @@ export function defineColadaLoader( // @ts-expect-error: requires the internals and symbol that are added later const useDataLoader: // for ts UseDataLoaderColada_LaxData = () => { + // When called from within a component (setup/render) and not synchronously + // from within a loader function, any active loader context is stale: it was + // left behind by a still-pending lazy loader. In a component, loaders are + // always used at the root, so we must drop it to avoid nesting a loader + // under an unrelated (or its own) entry. https://github.com/vuejs/router/issues/2684 + if (getCurrentInstance() && !isInsideLoaderFn()) { + setCurrentContext([]) + } // work with nested data loaders const currentContext = getCurrentContext() // TODO: should _route also contain from? diff --git a/packages/router/src/experimental/data-loaders/defineLoader.ts b/packages/router/src/experimental/data-loaders/defineLoader.ts index 4b275c38c..e5d7a24e9 100644 --- a/packages/router/src/experimental/data-loaders/defineLoader.ts +++ b/packages/router/src/experimental/data-loaders/defineLoader.ts @@ -14,12 +14,14 @@ import { NavigationResult, getCurrentContext, setCurrentContext, + isInsideLoaderFn, + runInsideLoaderFn, IS_SSR_KEY, LOADER_SET_KEY, type _DefineLoaderEntryMap, } from './entries/index' -import { shallowRef } from 'vue' +import { getCurrentInstance, shallowRef } from 'vue' import { type DefineDataLoaderOptionsBase_DefinedData, toLazyValue, @@ -208,7 +210,11 @@ export function defineBasicLoader( // Promise.resolve() allows loaders to also be sync const currentLoad = Promise.resolve( - loader(to, { signal: to.meta[ABORT_CONTROLLER_KEY]?.signal }) + // mark that we are synchronously inside a loader function so nested + // loaders can tell apart their parent context from a leaked one + runInsideLoaderFn(() => + loader(to, { signal: to.meta[ABORT_CONTROLLER_KEY]?.signal }) + ) ) .then(d => { // console.log( @@ -327,6 +333,14 @@ export function defineBasicLoader( // @ts-expect-error: return type has the generics const useDataLoader// for ts : UseDataLoaderBasic_LaxData = () => { + // When called from within a component (setup/render) and not synchronously + // from within a loader function, any active loader context is stale: it was + // left behind by a still-pending lazy loader. In a component, loaders are + // always used at the root, so we must drop it to avoid nesting a loader + // under an unrelated (or its own) entry. https://github.com/vuejs/router/issues/2684 + if (getCurrentInstance() && !isInsideLoaderFn()) { + setCurrentContext([]) + } // work with nested data loaders const currentContext = getCurrentContext() const [parentEntry, _router, _route] = currentContext diff --git a/packages/router/src/experimental/data-loaders/entries/index.ts b/packages/router/src/experimental/data-loaders/entries/index.ts index 699ce47fe..accfb06c8 100644 --- a/packages/router/src/experimental/data-loaders/entries/index.ts +++ b/packages/router/src/experimental/data-loaders/entries/index.ts @@ -29,6 +29,8 @@ export type { export { getCurrentContext, setCurrentContext, + isInsideLoaderFn, + runInsideLoaderFn, type _PromiseMerged, assign, isSubsetOf, diff --git a/packages/router/src/experimental/data-loaders/utils.ts b/packages/router/src/experimental/data-loaders/utils.ts index 82ffdacfc..b48806569 100644 --- a/packages/router/src/experimental/data-loaders/utils.ts +++ b/packages/router/src/experimental/data-loaders/utils.ts @@ -30,6 +30,41 @@ export function getCurrentContext() { return currentContext || ([] as const) } +/** + * Synchronous execution depth of data loader functions. Used to tell apart a + * genuinely nested loader call (a loader function calling another loader, even + * synchronously from within a component) from a stale context left behind by a + * still-pending lazy loader and read by an unrelated component render. + * INTERNAL ONLY. + * @internal + */ +let loaderFnDepth = 0 + +/** + * Whether we are currently executing the synchronous part of a data loader + * function. INTERNAL ONLY. + * @internal + */ +export function isInsideLoaderFn(): boolean { + return loaderFnDepth > 0 +} + +/** + * Runs the synchronous part of a data loader function while marking that we are + * inside a loader execution. INTERNAL ONLY. + * + * @param fn - the data loader function call to run + * @internal + */ +export function runInsideLoaderFn(fn: () => T): T { + loaderFnDepth++ + try { + return fn() + } finally { + loaderFnDepth-- + } +} + // TODO: rename parentContext /** * Sets the current context for data loaders. This allows for nested loaders to be aware of their parent context. diff --git a/packages/router/src/tests/data-loaders/tester.ts b/packages/router/src/tests/data-loaders/tester.ts index 1c75e1e27..f0357310d 100644 --- a/packages/router/src/tests/data-loaders/tester.ts +++ b/packages/router/src/tests/data-loaders/tester.ts @@ -1477,6 +1477,93 @@ export function testDefineLoader( expect(wrapper.text()).toBe('ok,one ok') }) + // https://github.com/vuejs/router/issues/2684 + it(`does not leak a pending nested lazy loader's context into components`, async () => { + const parentSpy = vi + .fn<(to: RouteLocationNormalizedLoaded) => Promise>() + // resolves instantly, like a cache hit + .mockImplementation(async () => 'parent') + const useParentLoader = loaderFactory({ + fn: parentSpy, + key: 'parent', + lazy: true, + }) + const childSpy = vi + .fn<(to: RouteLocationNormalizedLoaded) => Promise>() + .mockImplementation(async to => { + // nested loader awaits the parent loader, then keeps loading. While it + // is still pending, the global loader context stays set to this child + // entry, so a component rendering meanwhile must not pick it up as a + // parent (which would re-run the parent loader and nest loaders under + // themselves) + const parent = await useParentLoader() + await delay(10) + return `${parent},${to.query.p}` + }) + const useChildLoader = loaderFactory({ + fn: childSpy, + key: 'child', + lazy: true, + }) + + // child component (like a nested page) reuses the parent loader directly + // plus its own nested loader + const ChildComp = defineComponent({ + setup() { + const { data: parent } = useParentLoader() + const { data: child } = useChildLoader() + return { parent, child } + }, + template: `

{{ parent }}|{{ child }}

`, + }) + // parent component (like a parent page) uses the parent loader and renders + // the child component + const ParentComp = defineComponent({ + components: { ChildComp }, + setup() { + const { data } = useParentLoader() + return { data } + }, + template: `
{{ data }}
`, + }) + + const router = getRouter() + router.addRoute({ + name: '_test', + path: '/fetch', + component: ParentComp, + meta: { + loaders: [useParentLoader, useChildLoader], + nested: { foo: 'bar' }, + }, + }) + + const wrapper = mount(RouterViewMock, { + global: { + plugins: [ + [DataLoaderPlugin, { router }], + ...(plugins?.(customContext!) || []), + router, + ], + }, + }) + + // navigation completes while the lazy child loader is still pending, so the + // components render with the child loader's context still active + await router.push('/fetch?p=one') + await flushPromises() + // let the child loader finish + await vi.advanceTimersByTimeAsync(20) + await flushPromises() + + expect('has itself as parent').not.toHaveBeenWarned() + expect('a different parent').not.toHaveBeenWarned() + expect(parentSpy).toHaveBeenCalledTimes(1) + expect(childSpy).toHaveBeenCalledTimes(1) + expect(router.currentRoute.value.fullPath).toBe('/fetch?p=one') + expect(wrapper.get('#child').text()).toBe('parent|parent,one') + }) + it('keeps the old data until all loaders are resolved', async () => { const router = getRouter() const l1 = mockedLoader({ commit: 'after-load', key: 'l1' })