From e14d907c00d142a0cead0d28f6c7822f4192975a Mon Sep 17 00:00:00 2001 From: Manuel Schiller <6340397+schiller-manuel@users.noreply.github.com> Date: Sat, 12 Sep 2026 01:03:17 +0200 Subject: [PATCH] perf(router-core): stop structurally sharing search and state in buildLocation `buildLocation` ran `nullReplaceEqualDeep` over the built search and `replaceEqualDeep` over the built state against the current location on every build. That work only affected object identity, and nothing reads that identity from a built location: - Links select href and isActive from the built location; active state is computed by value and ignores history state. - `commitLocation` already compares state by value (`deepEqual` over `_getUserHistoryState`), so pre-shared children were at most a shortcut. - The identity consumers actually rely on (selecting `location.state.user` or a nested search value without rerendering) comes from `parseLocation`, which stabilizes the committed location against the previous one. That sharing stays untouched, as do the `matchRoutes` sites for search, params and loaderDeps. On the server `replaceEqualDeep` was already a passthrough, so client and server builds now produce the same shapes. Behavior changes: - A literal `search` or `state` is returned as the caller's object. It is never written to: `commitLocation` and history both copy before adding `__hashScrollIntoViewOptions`, `__TSR_key`, `key` and the index. - Because `replaceEqualDeep` returned the *current* search when contents were equal, `buildLocation` used to re-serialize the current key order. At `?a=1&b=2`, `buildLocation({ search: { b: 2, a: 1 } })` produced `?a=1&b=2`, so navigating there was a same-location no-op. It now produces `?b=2&a=1` and pushes a new history entry. - A destination without `search` yields the frozen `EMPTY_RECORD` instead of a fresh or shared object. Nothing downstream mutates the built search; all consumers copy. The `usedCurrent` tracking is unchanged: only reads through `current()` / `currentMatch()` mark a build as location-dependent, so literal search and state keep hitting the per-options cache. Tests: the build-time sharing assertion (`explicit state structurally shares unchanged nested values`) is replaced by a describe block that pins the new contract: built search/state are the caller's objects, equal nested references are preserved after navigation via parseLocation, `navigate` does not mutate a plain or frozen caller state, and the key-order case is covered at both the buildLocation and the navigation level. Measurements (react-router.minimal, this tree): gzip 86026 -> 86004 (-22), initial -23, raw -41, brotli +65. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .changeset/calm-locations-unshared.md | 5 + packages/router-core/src/router.ts | 35 ++-- .../router-core/tests/build-location.test.ts | 178 +++++++++++++++--- 3 files changed, 170 insertions(+), 48 deletions(-) create mode 100644 .changeset/calm-locations-unshared.md diff --git a/.changeset/calm-locations-unshared.md b/.changeset/calm-locations-unshared.md new file mode 100644 index 0000000000..1d3b74c795 --- /dev/null +++ b/.changeset/calm-locations-unshared.md @@ -0,0 +1,5 @@ +--- +'@tanstack/router-core': patch +--- + +`buildLocation` no longer structurally shares the built `search` and `state` with the current location. The observable `location.search` and `location.state` still preserve equal nested references across navigations, because `parseLocation` stabilizes them once a location is committed. A search whose contents equal the current search but list its keys in a different order now serializes in the requested order, so navigating to it creates a new history entry instead of being treated as the same location. A `state` object passed to `navigate` or `buildLocation` is never mutated. diff --git a/packages/router-core/src/router.ts b/packages/router-core/src/router.ts index f11f3f17a1..19ea2b8aed 100644 --- a/packages/router-core/src/router.ts +++ b/packages/router-core/src/router.ts @@ -1881,7 +1881,9 @@ export class RouterCore< /** * Build the next ParsedLocation from navigation options without committing. * Resolves `to`/`from`, params/search/hash/state, applies search validation - * and middlewares, and returns a stable, stringified location object. + * and middlewares, and returns a stringified location object. The built + * `search` and `state` are not structurally shared with the current + * location; `parseLocation` stabilizes them once the location is committed. * * @link https://tanstack.com/router/latest/docs/framework/react/api/router/RouterType#buildlocation-method */ @@ -2094,8 +2096,10 @@ export class RouterCore< } return search } - // A literal search never reads the current one. - let nextSearch: Record = middlewares.length + // A literal search never reads the current one. The result is not + // structurally shared with the current search: `parseLocation` keeps + // equal nested values stable once the location is committed. + const nextSearch: Record = middlewares.length ? applySearchMiddleware(middlewares, fromSearch(), dest) : dest.search === true ? fromSearch() @@ -2103,10 +2107,6 @@ export class RouterCore< ? dest.search(fromSearch()) : (dest.search as Record) || EMPTY_RECORD - // Structural sharing only affects identity, so it does not make the - // location depend on the current one. - nextSearch = nullReplaceEqualDeep(lightweight[2 /* search */], nextSearch) - // Stringify the next search const searchStr = this.options.stringifySearch(nextSearch) @@ -2121,18 +2121,15 @@ export class RouterCore< // Resolve the next hash string const hashStr = hash ? `#${hash}` : '' - // Resolve the next state - let nextState: HistoryState = EMPTY_RECORD - if (dest.state) { - nextState = - dest.state === true - ? current().state - : typeof dest.state === 'function' - ? dest.state(current().state) - : dest.state - // Identity-only, as above. - nextState = replaceEqualDeep(currentLocation.state, nextState) - } + // Resolve the next state. A literal state never reads the current one + // and, like the search, is not shared with it here. + const nextState: HistoryState = !dest.state + ? EMPTY_RECORD + : dest.state === true + ? current().state + : typeof dest.state === 'function' + ? dest.state(current().state) + : dest.state // Create the full path of the location const fullPath = `${nextPathname}${searchStr}${hashStr}` diff --git a/packages/router-core/tests/build-location.test.ts b/packages/router-core/tests/build-location.test.ts index e9c291e31e..99bbbc60cf 100644 --- a/packages/router-core/tests/build-location.test.ts +++ b/packages/router-core/tests/build-location.test.ts @@ -1299,68 +1299,188 @@ describe('buildLocation - state', () => { expect(location.state).not.toBe(emptyState) }) - test('explicit state structurally shares unchanged nested values', async () => { + test('state can contain complex nested objects', async () => { const rootRoute = new BaseRootRoute({}) const postsRoute = new BaseRoute({ getParentRoute: () => rootRoute, path: '/posts', }) - const history = createMemoryHistory({ initialEntries: ['/posts'] }) - history.replace('/posts', { + + const routeTree = rootRoute.addChildren([postsRoute]) + + const router = createTestRouter({ + routeTree, + history: createMemoryHistory({ initialEntries: ['/posts'] }), + }) + + await router.load() + + const complexState = { user: { id: 1, name: 'Test' }, - count: 1, + items: [1, 2, 3], + nested: { deep: { value: true } }, + } + + const location = router.buildLocation({ + to: '/posts', + state: complexState as any, }) - const router = createTestRouter({ + + expect(location.state).toEqual(complexState) + }) +}) + +describe('buildLocation - no structural sharing with the current location', () => { + function createPostsRouter( + history = createMemoryHistory({ initialEntries: ['/posts'] }), + ) { + const rootRoute = new BaseRootRoute({}) + const postsRoute = new BaseRoute({ + getParentRoute: () => rootRoute, + path: '/posts', + }) + return createTestRouter({ routeTree: rootRoute.addChildren([postsRoute]), history, }) + } + + test('explicit state is returned as-is and equal nested values are shared only after navigation', async () => { + const history = createMemoryHistory({ initialEntries: ['/posts'] }) + history.replace('/posts', { + user: { id: 1, name: 'Test' }, + count: 1, + }) + const router = createPostsRouter(history) await router.load() - const currentState = router.state.location.state as any + const previousState = router.state.location.state as any + const nextState = { + user: { id: 1, name: 'Test' }, + count: 2, + } const location = router.buildLocation({ to: '/posts', - state: { - user: { id: 1, name: 'Test' }, - count: 2, - } as any, + state: nextState as any, }) - expect(location.state).toEqual({ + // The built location carries the caller's object untouched. + expect(location.state).toBe(nextState) + expect((location.state as any).user).not.toBe(previousState.user) + + await router.navigate({ to: '/posts', state: nextState as any }) + + // parseLocation still stabilizes the committed state against the + // previous one, which is what location selectors rely on. + const committedState = router.state.location.state as any + expect(committedState).toMatchObject({ user: { id: 1, name: 'Test' }, count: 2, }) - expect((location.state as any).user).toBe(currentState.user) - expect(location.state).not.toBe(currentState) + expect(committedState).not.toBe(previousState) + expect(committedState.user).toBe(previousState.user) + expect(nextState.user).not.toBe(previousState.user) }) - test('state can contain complex nested objects', async () => { - const rootRoute = new BaseRootRoute({}) - const postsRoute = new BaseRoute({ - getParentRoute: () => rootRoute, - path: '/posts', + test('explicit search is returned as-is and equal nested values are shared only after navigation', async () => { + const router = createPostsRouter() + await router.load() + + await router.navigate({ + to: '/posts', + search: { page: 1, filter: { tags: ['a'] } } as any, }) + const previousSearch = router.state.location.search as any + expect(previousSearch).toEqual({ page: 1, filter: { tags: ['a'] } }) - const routeTree = rootRoute.addChildren([postsRoute]) + const nextSearch = { page: 2, filter: { tags: ['a'] } } + const location = router.buildLocation({ + to: '/posts', + search: nextSearch as any, + }) - const router = createTestRouter({ - routeTree, - history: createMemoryHistory({ initialEntries: ['/posts'] }), + expect(location.search).toBe(nextSearch) + expect((location.search as any).filter).not.toBe(previousSearch.filter) + + await router.navigate({ to: '/posts', search: nextSearch as any }) + + const committedSearch = router.state.location.search as any + expect(committedSearch).toEqual({ page: 2, filter: { tags: ['a'] } }) + expect(committedSearch).not.toBe(previousSearch) + expect(committedSearch.filter).toBe(previousSearch.filter) + expect(nextSearch.filter).not.toBe(previousSearch.filter) + }) + + test('navigate does not mutate a caller-supplied state object', async () => { + const history = createMemoryHistory({ initialEntries: ['/posts'] }) + const router = createPostsRouter(history) + await router.load() + + const state = { user: { id: 1 } } + await router.navigate({ + to: '/posts', + state: state as any, + hashScrollIntoView: true, }) + expect(state).toEqual({ user: { id: 1 } }) + expect(Object.keys(state)).toEqual(['user']) + const committedState = router.state.location.state as any + expect(committedState).not.toBe(state) + expect(committedState.user).toBe(state.user) + expect(committedState.__hashScrollIntoViewOptions).toBe(true) + expect(committedState.__TSR_key).toBeTypeOf('string') + expect(committedState.key).toBe(committedState.__TSR_key) + + // A frozen state (for example produced by an immutable store) commits + // without any write hitting it: in strict mode such a write would throw. + const frozenState = Object.freeze({ user: Object.freeze({ id: 2 }) }) + await router.navigate({ to: '/posts', state: frozenState as any }) + + expect(router.state.location.state).toMatchObject({ user: { id: 2 } }) + expect(router.state.location.state).not.toBe(frozenState) + expect(history.length).toBe(3) + }) + + test('an equal search in a different key order serializes in the requested order', async () => { + const router = createPostsRouter( + createMemoryHistory({ initialEntries: ['/posts?a=1&b=2'] }), + ) await router.load() - const complexState = { - user: { id: 1, name: 'Test' }, - items: [1, 2, 3], - nested: { deep: { value: true } }, - } + expect(router.state.location.href).toBe('/posts?a=1&b=2') const location = router.buildLocation({ to: '/posts', - state: complexState as any, + search: { b: 2, a: 1 } as any, }) - expect(location.state).toEqual(complexState) + expect(location.search).toEqual({ a: 1, b: 2 }) + expect(Object.keys(location.search)).toEqual(['b', 'a']) + expect(location.searchStr).toBe('?b=2&a=1') + expect(location.href).toBe('/posts?b=2&a=1') + }) + + test('navigating to an equal search in a different key order pushes a new history entry', async () => { + const history = createMemoryHistory({ initialEntries: ['/posts?a=1&b=2'] }) + const router = createPostsRouter(history) + await router.load() + + expect(history.length).toBe(1) + + // Same contents and order: nothing to commit. + await router.navigate({ to: '/posts', search: { a: 1, b: 2 } as any }) + + expect(history.length).toBe(1) + expect(router.state.location.href).toBe('/posts?a=1&b=2') + + // Same contents, different order: the URL changes, so history grows. + await router.navigate({ to: '/posts', search: { b: 2, a: 1 } as any }) + + expect(history.length).toBe(2) + expect(history.location.href).toBe('/posts?b=2&a=1') + expect(router.state.location.href).toBe('/posts?b=2&a=1') + expect(router.state.location.search).toEqual({ a: 1, b: 2 }) }) })