fix(data-loaders): ignore leaked loader context in components

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<Data>(
         // 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<Data>(
   // @ts-expect-error: requires the internals and symbol that are added later
   const useDataLoader: // for ts
   UseDataLoaderColada_LaxData<Data> = () => {
+    // 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?

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<Data>(
 
     // 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<Data>(
   // @ts-expect-error: return type has the generics
   const useDataLoader// for ts
   : UseDataLoaderBasic_LaxData<Data> = () => {
+    // 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

packages/router/src/experimental/data-loaders/entries/index.ts

@@ -29,6 +29,8 @@ export type {
 export {
   getCurrentContext,
   setCurrentContext,
+  isInsideLoaderFn,
+  runInsideLoaderFn,
   type _PromiseMerged,
   assign,
   isSubsetOf,

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<T>(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.

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

@@ -1477,6 +1477,93 @@ export function testDefineLoader<Context = void>(
     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<string>>()
+      // 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<string>>()
+      .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: `<p id="child">{{ parent }}|{{ child }}</p>`,
+    })
+    // 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: `<div id="parent">{{ data }}<ChildComp /></div>`,
+    })
+
+    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' })