fix(router-core): use routeId for lifecycle hooks to fix onStay when loaderDeps change (#6769)

Author: sleitor

packages/router-core/src/router.ts

@@ -2406,9 +2406,16 @@ export class RouterCore<
 
                   // Commit the pending matches. If a previous match was
                   // removed, place it in the cachedMatches
+                  //
+                  // exitingMatches uses match.id (routeId + params + loaderDeps) so
+                  // navigating /foo?page=1 → /foo?page=2 correctly caches the page=1 entry.
                   let exitingMatches: Array<AnyRouteMatch> = []
-                  let enteringMatches: Array<AnyRouteMatch> = []
-                  let stayingMatches: Array<AnyRouteMatch> = []
+
+                  // Lifecycle-hook identity uses routeId only so that navigating between
+                  // different params/deps of the same route fires onStay (not onLeave+onEnter).
+                  let hookExitingMatches: Array<AnyRouteMatch> = []
+                  let hookEnteringMatches: Array<AnyRouteMatch> = []
+                  let hookStayingMatches: Array<AnyRouteMatch> = []
 
                   batch(() => {
                     this.__store.setState((s) => {
@@ -2418,12 +2425,22 @@ export class RouterCore<
                       exitingMatches = previousMatches.filter(
                         (match) => !newMatches.some((d) => d.id === match.id),
                       )
-                      enteringMatches = newMatches.filter(
+
+                      // Lifecycle-hook identity: routeId only (route presence in tree)
+                      hookExitingMatches = previousMatches.filter(
+                        (match) =>
+                          !newMatches.some((d) => d.routeId === match.routeId),
+                      )
+                      hookEnteringMatches = newMatches.filter(
                         (match) =>
-                          !previousMatches.some((d) => d.id === match.id),
+                          !previousMatches.some(
+                            (d) => d.routeId === match.routeId,
+                          ),
                       )
-                      stayingMatches = newMatches.filter((match) =>
-                        previousMatches.some((d) => d.id === match.id),
+                      hookStayingMatches = newMatches.filter((match) =>
+                        previousMatches.some(
+                          (d) => d.routeId === match.routeId,
+                        ),
                       )
 
                       return {
@@ -2455,9 +2472,9 @@ export class RouterCore<
                   //
                   ;(
                     [
-                      [exitingMatches, 'onLeave'],
-                      [enteringMatches, 'onEnter'],
-                      [stayingMatches, 'onStay'],
+                      [hookExitingMatches, 'onLeave'],
+                      [hookEnteringMatches, 'onEnter'],
+                      [hookStayingMatches, 'onStay'],
                     ] as const
                   ).forEach(([matches, hook]) => {
                     matches.forEach((match) => {

packages/router-core/tests/callbacks.test.ts

@@ -1,4 +1,4 @@
-import { describe, expect, it, vi } from 'vitest'
+import { describe, expect, it, test, vi } from 'vitest'
 import { createMemoryHistory } from '@tanstack/history'
 import { BaseRootRoute, BaseRoute, RouterCore } from '../src'
 
@@ -39,6 +39,35 @@ describe('callbacks', () => {
 
     return router
   }
+
+  const createFooRouter = (
+    opts: {
+      onEnter?: () => void
+      onLeave?: () => void
+      onStay?: () => void
+      loader?: () => unknown
+      staleTime?: number
+    } = {},
+  ) => {
+    const rootRoute = new BaseRootRoute({})
+    const fooRoute = new BaseRoute({
+      getParentRoute: () => rootRoute,
+      path: '/foo',
+      loaderDeps: ({ search }: { search: Record<string, unknown> }) => ({
+        page: search['page'],
+      }),
+      onEnter: opts.onEnter,
+      onLeave: opts.onLeave,
+      onStay: opts.onStay,
+      loader: opts.loader,
+      staleTime: opts.staleTime,
+      gcTime: opts.staleTime,
+    })
+    return new RouterCore({
+      routeTree: rootRoute.addChildren([fooRoute]),
+      history: createMemoryHistory(),
+    })
+  }
   describe('onEnter', () => {
     it('runs on navigate to a new route', async () => {
       const onEnter = vi.fn()
@@ -102,5 +131,158 @@ describe('callbacks', () => {
         expect.objectContaining({ id: '/foo/foo', search: { foo: 'quux' } }),
       )
     })
+
+    it('runs instead of onLeave/onEnter when loaderDeps change from search param updates', async () => {
+      const onEnter = vi.fn()
+      const onLeave = vi.fn()
+      const onStay = vi.fn()
+      const router = createFooRouter({ onEnter, onLeave, onStay })
+
+      // Navigate to foo — onEnter should fire
+      await router.navigate({ to: '/foo', search: { page: '1' } })
+      expect(onEnter).toHaveBeenCalledTimes(1)
+      expect(onLeave).toHaveBeenCalledTimes(0)
+      expect(onStay).toHaveBeenCalledTimes(0)
+
+      // Update search param that's in loaderDeps — onStay should fire, not onLeave+onEnter
+      await router.navigate({ to: '/foo', search: { page: '2' } })
+      expect(onEnter).toHaveBeenCalledTimes(1) // no new onEnter
+      expect(onLeave).toHaveBeenCalledTimes(0) // no onLeave
+      expect(onStay).toHaveBeenCalledTimes(1) // onStay fires
+
+      // Update again — onStay fires again
+      await router.navigate({ to: '/foo', search: { page: '3' } })
+      expect(onEnter).toHaveBeenCalledTimes(1)
+      expect(onLeave).toHaveBeenCalledTimes(0)
+      expect(onStay).toHaveBeenCalledTimes(2)
+    })
+  })
+
+  // Regression tests: switching lifecycle hooks to use routeId must NOT break
+  // match-level caching, which still relies on match.id (routeId + params + loaderDeps).
+  describe('same-route match caching', () => {
+    const setupWithPathParams = ({
+      loader,
+      staleTime,
+    }: {
+      loader?: () => unknown
+      staleTime?: number
+    }) => {
+      const rootRoute = new BaseRootRoute({})
+      const postRoute = new BaseRoute({
+        getParentRoute: () => rootRoute,
+        path: '/posts/$postId',
+        loader,
+        staleTime,
+        gcTime: staleTime,
+      })
+      const routeTree = rootRoute.addChildren([postRoute])
+      return new RouterCore({
+        routeTree,
+        history: createMemoryHistory(),
+      })
+    }
+
+    test('keeps previous loaderDeps variant cached and reuses it within staleTime', async () => {
+      const loader = vi.fn()
+      const router = createFooRouter({ loader, staleTime: 60_000 })
+
+      await router.navigate({ to: '/foo', search: { page: '1' } })
+      const page1MatchId = router.state.matches.find(
+        (d) => d.routeId === '/foo',
+      )?.id
+      expect(page1MatchId).toBeDefined()
+
+      await router.navigate({ to: '/foo', search: { page: '2' } })
+      const page2MatchId = router.state.matches.find(
+        (d) => d.routeId === '/foo',
+      )?.id
+      expect(page2MatchId).toBeDefined()
+      expect(page2MatchId).not.toBe(page1MatchId)
+      expect(
+        router.state.cachedMatches.some((d) => d.id === page1MatchId),
+      ).toBe(true)
+
+      await router.navigate({ to: '/foo', search: { page: '1' } })
+      expect(loader).toHaveBeenCalledTimes(2)
+      expect(router.state.matches.some((d) => d.id === page1MatchId)).toBe(true)
+    })
+
+    test('keeps previous params variant cached and reuses it within staleTime', async () => {
+      const loader = vi.fn()
+      const router = setupWithPathParams({ loader, staleTime: 60_000 })
+
+      await router.navigate({ to: '/posts/$postId', params: { postId: '1' } })
+      const post1MatchId = router.state.matches.find(
+        (d) => d.routeId === '/posts/$postId',
+      )?.id
+      expect(post1MatchId).toBeDefined()
+
+      await router.navigate({ to: '/posts/$postId', params: { postId: '2' } })
+      const post2MatchId = router.state.matches.find(
+        (d) => d.routeId === '/posts/$postId',
+      )?.id
+      expect(post2MatchId).toBeDefined()
+      expect(post2MatchId).not.toBe(post1MatchId)
+      expect(
+        router.state.cachedMatches.some((d) => d.id === post1MatchId),
+      ).toBe(true)
+
+      await router.navigate({ to: '/posts/$postId', params: { postId: '1' } })
+      expect(loader).toHaveBeenCalledTimes(2)
+      expect(router.state.matches.some((d) => d.id === post1MatchId)).toBe(true)
+    })
+  })
+
+  // Verify that router-level subscription events still fire correctly.
+  // These are used by integrations like Sentry's TanStack Router instrumentation
+  // (https://github.com/getsentry/sentry-javascript/blob/develop/packages/react/src/tanstackrouter.ts)
+  // to track page transitions.
+  describe('onBeforeNavigate event', () => {
+    it('fires when navigating to a new route', async () => {
+      const router = setup({})
+      const onBeforeNavigate = vi.fn()
+      router.subscribe('onBeforeNavigate', onBeforeNavigate)
+
+      await router.navigate({ to: '/foo' })
+      expect(onBeforeNavigate).toHaveBeenCalledTimes(1)
+      expect(onBeforeNavigate).toHaveBeenCalledWith(
+        expect.objectContaining({
+          type: 'onBeforeNavigate',
+          pathChanged: true,
+        }),
+      )
+    })
+
+    it('fires on every navigation including same-route loaderDeps changes', async () => {
+      const router = createFooRouter({})
+      const onBeforeNavigate = vi.fn()
+      router.subscribe('onBeforeNavigate', onBeforeNavigate)
+
+      await router.navigate({ to: '/foo', search: { page: '1' } })
+      expect(onBeforeNavigate).toHaveBeenCalledTimes(1)
+
+      // loaderDeps change — same route, different params
+      await router.navigate({ to: '/foo', search: { page: '2' } })
+      expect(onBeforeNavigate).toHaveBeenCalledTimes(2)
+    })
+
+    it('includes toLocation and pathChanged in the event', async () => {
+      const router = setup({})
+      const events: Array<{ to: string; pathChanged: boolean }> = []
+      router.subscribe('onBeforeNavigate', (e) => {
+        events.push({
+          to: e.toLocation.pathname,
+          pathChanged: e.pathChanged,
+        })
+      })
+
+      await router.navigate({ to: '/foo' })
+      await router.navigate({ to: '/bar' })
+
+      expect(events).toHaveLength(2)
+      expect(events[0]).toMatchObject({ to: '/foo', pathChanged: true })
+      expect(events[1]).toMatchObject({ to: '/bar', pathChanged: true })
+    })
   })
 })