diff --git a/packages/router-core/src/router.ts b/packages/router-core/src/router.ts index 965fefda339..93b98131c69 100644 --- a/packages/router-core/src/router.ts +++ b/packages/router-core/src/router.ts @@ -1248,11 +1248,11 @@ export class RouterCore< } emit: EmitFn = (routerEvent) => { - this.subscribers.forEach((listener) => { + for (const listener of this.subscribers) { if (listener.eventType === routerEvent.type) { listener.fn(routerEvent) } - }) + } } /** @@ -1765,24 +1765,24 @@ export class RouterCore< } cancelMatches = () => { - this.stores.pendingMatchesId.state.forEach((matchId) => { + for (const matchId of this.stores.pendingMatchesId.state) { this.cancelMatch(matchId) - }) + } - this.stores.matchesId.state.forEach((matchId) => { + for (const matchId of this.stores.matchesId.state) { if (this.stores.pendingMatchStoresById.has(matchId)) { - return + continue } const match = this.stores.activeMatchStoresById.get(matchId)?.state if (!match) { - return + continue } if (match.status === 'pending' || match.isFetching === 'loader') { this.cancelMatch(matchId) } - }) + } } /** @@ -1915,7 +1915,7 @@ export class RouterCore< let nextSearch = fromSearch if (opts._includeValidateSearch && this.options.search?.strict) { const validatedSearch = {} - destRoutes.forEach((route) => { + for (const route of destRoutes) { if (route.options.validateSearch) { try { Object.assign( @@ -1929,7 +1929,7 @@ export class RouterCore< // ignore errors here because they are already handled in matchRoutes } } - }) + } nextSearch = validatedSearch } @@ -2093,13 +2093,13 @@ export class RouterCore< '__TSR_index', '__hashScrollIntoViewOptions', ] as const - ignoredProps.forEach((prop) => { + for (const prop of ignoredProps) { ;(next.state as any)[prop] = this.latestLocation.state[prop] - }) + } const isEqual = deepEqual(next.state, this.latestLocation.state) - ignoredProps.forEach((prop) => { + for (const prop of ignoredProps) { delete next.state[prop] - }) + } return isEqual } @@ -2418,54 +2418,42 @@ export class RouterCore< // exitingMatches uses match.id (routeId + params + loaderDeps) so // navigating /foo?page=1 → /foo?page=2 correctly caches the page=1 entry. let exitingMatches: Array | null = null - - // 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 | null = null - let hookEnteringMatches: Array | null = null - let hookStayingMatches: Array | null = null + let pendingMatches: Array = [] + let currentMatches: Array = [] + let pendingRouteIds: Set | null = null + let activeRouteIds: Set | null = null this.batch(() => { - const pendingMatches = - this.stores.pendingMatchesSnapshot.state - const mountPending = pendingMatches.length - const currentMatches = - this.stores.activeMatchesSnapshot.state - - exitingMatches = mountPending - ? currentMatches.filter( - (match) => - !this.stores.pendingMatchStoresById.has(match.id), - ) - : null - - // Lifecycle-hook identity: routeId only (route presence in tree) - // Build routeId sets from pools to avoid derived stores. - const pendingRouteIds = new Set() - for (const s of this.stores.pendingMatchStoresById.values()) { - if (s.routeId) pendingRouteIds.add(s.routeId) - } - const activeRouteIds = new Set() - for (const s of this.stores.activeMatchStoresById.values()) { - if (s.routeId) activeRouteIds.add(s.routeId) + pendingMatches = this.stores.pendingMatchesSnapshot.state + currentMatches = this.stores.activeMatchesSnapshot.state + + if (pendingMatches.length) { + activeRouteIds = new Set() + for (const match of currentMatches) { + activeRouteIds.add(match.routeId) + } + + pendingRouteIds = new Set() + exitingMatches = [] + + for (const match of pendingMatches) { + pendingRouteIds.add(match.routeId) + } + + for (const match of currentMatches) { + if ( + !this.stores.pendingMatchStoresById.has(match.id) && + match.status !== 'error' && + match.status !== 'notFound' && + match.status !== 'redirected' + ) { + exitingMatches.push(match) + } + } + } else { + exitingMatches = null } - hookExitingMatches = mountPending - ? currentMatches.filter( - (match) => !pendingRouteIds.has(match.routeId), - ) - : null - hookEnteringMatches = mountPending - ? pendingMatches.filter( - (match) => !activeRouteIds.has(match.routeId), - ) - : null - hookStayingMatches = mountPending - ? pendingMatches.filter((match) => - activeRouteIds.has(match.routeId), - ) - : currentMatches - this.stores.isLoading.setState(() => false) this.stores.loadedAt.setState(() => Date.now()) /** @@ -2474,31 +2462,47 @@ export class RouterCore< * deliberately excluded from `cachedMatches` so that subsequent invalidations * or reloads re-run their loaders instead of reusing the failed/not-found data. */ - if (mountPending) { + if (pendingMatches.length) { this.stores.setActiveMatches(pendingMatches) this.stores.setPendingMatches([]) this.stores.setCachedMatches([ ...this.stores.cachedMatchesSnapshot.state, - ...exitingMatches!.filter( - (d) => - d.status !== 'error' && - d.status !== 'notFound' && - d.status !== 'redirected', - ), + ...exitingMatches!, ]) this.clearExpiredCache() } }) - // - for (const [matches, hook] of [ - [hookExitingMatches, 'onLeave'], - [hookEnteringMatches, 'onEnter'], - [hookStayingMatches, 'onStay'], - ] as const) { - if (!matches) continue - for (const match of matches as Array) { - this.looseRoutesById[match.routeId]!.options[hook]?.( + // Lifecycle-hook identity uses routeId only so that navigating between + // different params/deps of the same route fires onStay (not onLeave+onEnter). + const nextPendingRouteIds = + pendingRouteIds as Set | null + const nextActiveRouteIds = + activeRouteIds as Set | null + + if (nextPendingRouteIds && nextActiveRouteIds) { + for (const match of currentMatches) { + if (!nextPendingRouteIds.has(match.routeId)) { + this.looseRoutesById[match.routeId]!.options.onLeave?.( + match, + ) + } + } + + for (const match of pendingMatches) { + if (nextActiveRouteIds.has(match.routeId)) { + this.looseRoutesById[match.routeId]!.options.onStay?.( + match, + ) + } else { + this.looseRoutesById[match.routeId]!.options.onEnter?.( + match, + ) + } + } + } else { + for (const match of currentMatches) { + this.looseRoutesById[match.routeId]!.options.onStay?.( match, ) } @@ -2525,9 +2529,7 @@ export class RouterCore< ? redirect.status : notFound ? 404 - : this.stores.activeMatchesSnapshot.state.some( - (d) => d.status === 'error', - ) + : this.hasErrorMatch() ? 500 : 200 @@ -2561,9 +2563,7 @@ export class RouterCore< let newStatusCode: number | undefined = undefined if (this.hasNotFoundMatch()) { newStatusCode = 404 - } else if ( - this.stores.activeMatchesSnapshot.state.some((d) => d.status === 'error') - ) { + } else if (this.hasErrorMatch()) { newStatusCode = 500 } if (newStatusCode !== undefined) { @@ -2930,10 +2930,24 @@ export class RouterCore< serverSsr?: ServerSsr + hasErrorMatch = () => { + for (const match of this.stores.activeMatchesSnapshot.state) { + if (match.status === 'error') { + return true + } + } + + return false + } + hasNotFoundMatch = () => { - return this.stores.activeMatchesSnapshot.state.some( - (d) => d.status === 'notFound' || d.globalNotFound, - ) + for (const match of this.stores.activeMatchesSnapshot.state) { + if (match.status === 'notFound' || match.globalNotFound) { + return true + } + } + + return false } } diff --git a/packages/router-core/tests/callbacks.test.ts b/packages/router-core/tests/callbacks.test.ts index 9016caced03..3a17600f349 100644 --- a/packages/router-core/tests/callbacks.test.ts +++ b/packages/router-core/tests/callbacks.test.ts @@ -1,6 +1,6 @@ import { describe, expect, it, test, vi } from 'vitest' import { createMemoryHistory } from '@tanstack/history' -import { BaseRootRoute, BaseRoute } from '../src' +import { BaseRootRoute, BaseRoute, createControlledPromise } from '../src' import { createTestRouter } from './routerTestUtils' describe('callbacks', () => { @@ -157,6 +157,105 @@ describe('callbacks', () => { expect(onLeave).toHaveBeenCalledTimes(0) expect(onStay).toHaveBeenCalledTimes(2) }) + + it('treats a navigation that changes the match instance and match id for the same routeId as stay, not leave plus enter', async () => { + const onEnter = vi.fn() + const onLeave = vi.fn() + const onStay = vi.fn() + const loaderPromises: Array< + ReturnType> + > = [] + + const rootRoute = new BaseRootRoute({}) + const fooRoute = new BaseRoute({ + getParentRoute: () => rootRoute, + path: '/foo', + loaderDeps: ({ search }: { search: Record }) => ({ + page: search['page'], + }), + onEnter, + onLeave, + onStay, + loader: () => { + const promise = createControlledPromise() + loaderPromises.push(promise) + return promise + }, + }) + + const router = createTestRouter({ + routeTree: rootRoute.addChildren([fooRoute]), + history: createMemoryHistory(), + }) + + const initialNavigation = router.navigate({ + to: '/foo', + search: { page: '1' }, + }) + + expect(loaderPromises).toHaveLength(1) + loaderPromises[0]!.resolve() + await initialNavigation + + onEnter.mockClear() + onLeave.mockClear() + onStay.mockClear() + + const currentMatches = router.stores.activeMatchesSnapshot.state + const currentFooMatch = currentMatches.find( + (match) => match.routeId === fooRoute.id, + ) + + expect(currentFooMatch).toBeDefined() + expect(router.looseRoutesById[fooRoute.id]!.options.onStay).toBe(onStay) + expect(router.looseRoutesById[fooRoute.id]!.options.onLeave).toBe(onLeave) + expect(router.looseRoutesById[fooRoute.id]!.options.onEnter).toBe(onEnter) + + const secondNavigation = router.navigate({ + to: '/foo', + search: { page: '2' }, + }) + + expect(loaderPromises).toHaveLength(2) + + let pendingMatches = router.stores.pendingMatchesSnapshot.state + for (let index = 0; index < 10 && pendingMatches.length === 0; index++) { + await Promise.resolve() + pendingMatches = router.stores.pendingMatchesSnapshot.state + } + + const pendingFooMatch = pendingMatches.find( + (match) => match.routeId === fooRoute.id, + ) + + expect(pendingFooMatch).toBeDefined() + expect(pendingFooMatch).not.toBe(currentFooMatch) + expect(pendingFooMatch!.id).not.toBe(currentFooMatch!.id) + expect(pendingFooMatch!.routeId).toBe(currentFooMatch!.routeId) + + const pendingRouteIds = new Set( + pendingMatches.map((match) => match.routeId), + ) + const activeRouteIds = new Set( + currentMatches.map((match) => match.routeId), + ) + + expect(pendingRouteIds.has(fooRoute.id)).toBe(true) + expect(activeRouteIds.has(fooRoute.id)).toBe(true) + + loaderPromises[1]!.resolve() + await secondNavigation + + expect(onStay).toHaveBeenCalledTimes(1) + expect(onStay).toHaveBeenCalledWith( + expect.objectContaining({ + id: pendingFooMatch!.id, + routeId: fooRoute.id, + }), + ) + expect(onLeave).toHaveBeenCalledTimes(0) + expect(onEnter).toHaveBeenCalledTimes(0) + }) }) // Regression tests: switching lifecycle hooks to use routeId must NOT break