diff --git a/packages/router-core/src/router.ts b/packages/router-core/src/router.ts index 0eeb87425fe..89c975ef119 100644 --- a/packages/router-core/src/router.ts +++ b/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 = [] - let enteringMatches: Array = [] - let stayingMatches: Array = [] + + // 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 = [] + let hookEnteringMatches: Array = [] + let hookStayingMatches: Array = [] 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) => { diff --git a/packages/router-core/tests/callbacks.test.ts b/packages/router-core/tests/callbacks.test.ts index b6a5343eb66..3ad23616ba3 100644 --- a/packages/router-core/tests/callbacks.test.ts +++ b/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 }) => ({ + 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 }) + }) }) })