From 806c0d4171bf4298ba2b96f4e339e1654600cf9f Mon Sep 17 00:00:00 2001 From: Manuel Schiller <6340397+schiller-manuel@users.noreply.github.com> Date: Fri, 11 Sep 2026 21:44:40 +0200 Subject: [PATCH 1/4] perf(router-core): reuse location-independent Link destinations Most mounted Links point at a destination that does not depend on where the user currently is: an absolute `to`, literal params for every template key, a literal (or no) search, hash and state. Until now every navigation rebuilt all of them anyway, because `buildLocation` always started from the current location. `buildLocation` now tracks whether a build read the current location at all. Every such read goes through `current()` / `currentMatch()`, which set one flag; a build that finishes without it depends only on its options and the route tree. When the caller passed `_fromLocation` (what Links do) and no mask is involved, the resulting location is kept in a WeakMap keyed by the options object, and the next `buildLocation` with that same object returns it directly. `update()` and `setRoutes()` replace the map, so router option changes and HMR route-tree rebuilds invalidate everything. To make that flag meaningful the reads became demand-driven instead of unconditional: an absolute `to` resolves without a base path, params that cover the template skip the inherited-params merge, a literal search/hash/ state never touches the current values, and stringifiers fetch inherited params only when a route defines one. Structural sharing with the current search/state stays, as it only affects identity. Contract: the same options object yields the same location until router options or the route tree change. Callers own invalidation by passing a new object when their values change (the follow-up react-router commit does that with deepEqual-stabilized copies of `params`/`search`). Once whole locations are reused, the per-route SIEVE pathname cache (`_pathCache`, `InterpolationPlan`, `createPathInterpolator` and its router plumbing) is redundant and is removed; templates are interpolated directly from the route's parsed segments. Tests: path, route-tree-caches, path-decoder and the interpolation bench assert canonical pathnames through `buildLocation` instead of cache internals. Performance (Apple M4, Node 24, fresh production bundles, 3 runs each; measured on a tree that also carried the replaceEqualDeep changes now proposed separately in #8362, #8363 and #8364): - Measured alone, with the React Link still spreading a fresh options object per navigation so nothing hits the cache: links/react 341.3 -> 290 hz (-15%); paired link-perf client vs stack HEAD: encoding +30%, splats +24% slower (HEAD's pathname cache targeted exactly those), the other cases within noise. This commit is the first half of a two-part change and regresses on its own. - With the follow-up react-router commit (stable options object, so 160 of 161 builds per navigation become 13 ns cache hits; a full build costs 680-970 ns): links/react 622 hz (+82% vs stack HEAD, rme +-0.4%). Paired link-perf client vs origin/main: shared-params -34%, unique-params -36%, splats -41%, encoding -54%, active -44%, middleware -14%, relative -9%. SSR vs origin/main: -12..-32% on every measured case (encoding -23%, splats -18%); vs stack HEAD only SSR encoding is slower (+31%). - Bundle (react-router.minimal gzip, this tree): 85925 -> 85992 (+67) for this commit, 1 byte below origin/main (85993); 86038 with the Link commit. With #8362, #8363 and #8364 applied as well the tree measures 85983. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- packages/router-core/src/path.ts | 6 - packages/router-core/src/route.ts | 6 +- packages/router-core/src/router.ts | 322 ++++++++++-------- .../router-core/tests/path-decoder.test.ts | 10 +- .../tests/path-interpolation.bench.ts | 34 +- packages/router-core/tests/path.test.ts | 65 +--- .../tests/route-tree-caches.test.ts | 158 +-------- packages/router-core/tests/routerTestUtils.ts | 4 +- 8 files changed, 214 insertions(+), 391 deletions(-) diff --git a/packages/router-core/src/path.ts b/packages/router-core/src/path.ts index 0b2f8d8a60..74247cd0f2 100644 --- a/packages/router-core/src/path.ts +++ b/packages/router-core/src/path.ts @@ -208,12 +208,6 @@ export function compileDecodeCharMap( encoded.replace(regex, (match) => charMap.get(match) ?? match) } -export type InterpolationPlan = [ - paths: SieveCache, - path: string, - segments: RouteInterpolation, -] - export type InterpolationSegment = string | DynamicPathSegment export type RouteInterpolation = Array & { diff --git a/packages/router-core/src/route.ts b/packages/router-core/src/route.ts index 36bedc9a89..089369b454 100644 --- a/packages/router-core/src/route.ts +++ b/packages/router-core/src/route.ts @@ -4,7 +4,7 @@ import { notFound } from './not-found' import { redirect } from './redirect' import { rootRouteId } from './root' import type { LazyRoute } from './fileRoute' -import type { InterpolationPlan, RouteInterpolation } from './path' +import type { RouteInterpolation } from './path' import type { NotFoundError } from './not-found' import type { RedirectFnRoute } from './redirect' import type { NavigateOptions, ParsePathParams } from './link' @@ -730,8 +730,6 @@ export interface Route< /** @internal */ _branch?: ReadonlyArray /** @internal */ - _pathCache?: InterpolationPlan - /** @internal */ _interpolation?: RouteInterpolation rank: number to: TrimPathRight @@ -1723,8 +1721,6 @@ export class BaseRoute< /** @internal */ _branch?: ReadonlyArray /** @internal */ - _pathCache?: InterpolationPlan - /** @internal */ _interpolation?: RouteInterpolation constructor( options?: RouteOptions< diff --git a/packages/router-core/src/router.ts b/packages/router-core/src/router.ts index ccd0898c46..f11f3f17a1 100644 --- a/packages/router-core/src/router.ts +++ b/packages/router-core/src/router.ts @@ -14,6 +14,7 @@ import { functionalUpdate, getUrlScheme, hasKeys, + hasOwn, isDangerousProtocol, last, nullReplaceEqualDeep, @@ -25,7 +26,6 @@ import { findFlatMatch, findRouteMatch, findSingleMatch, - getParamNames, parseSegments, processRouteMasks, processRouteTree, @@ -58,7 +58,7 @@ import { } from './rewrite' import { createRouterStores } from './stores' import type { SieveCache } from './sieve-cache' -import type { InterpolationPlan } from './path' +import type { RouteInterpolation } from './path' import type { ProcessRouteTreeResult, ProcessedTree, @@ -1027,7 +1027,6 @@ type LightweightRouteMatchCacheEntry = [ type RouteTreeCaches = ProcessRouteTreeResult & { resolvePathCache: SieveCache - unmatchedPathCache: SieveCache } export type CreateRouterFn = < @@ -1169,11 +1168,13 @@ export class RouterCore< routesByPath!: RoutesByPath processedTree!: ProcessedTree resolvePathCache!: SieveCache - private unmatchedPathCache!: SieveCache private lightweightCache!: WeakMap< ParsedLocation, LightweightRouteMatchCacheEntry > + // Locations built without reading the current location, keyed by the stable + // options object a Link owns. Links pass a new object when their values change. + private staticLocations!: WeakMap isServer!: boolean readonly pathParamsDecoder?: (encoded: string) => string protocolAllowlist!: Set @@ -1249,6 +1250,7 @@ export class RouterCore< ...prevOptions, ...newOptions, } + this.staticLocations = new WeakMap() this.isServer = this.options.isServer ?? isServer ?? typeof document === 'undefined' @@ -1382,13 +1384,13 @@ export class RouterCore< return { ...result, resolvePathCache: createSieveCache(1000), - unmatchedPathCache: createSieveCache(32), } } setRoutes(caches: RouteTreeCaches) { Object.assign(this, caches) this.lightweightCache = new WeakMap() + this.staticLocations = new WeakMap() const notFoundRoute = this.options.notFoundRoute @@ -1876,63 +1878,6 @@ export class RouterCore< return result } - private interpolatePath( - path: string, - params: Record, - route?: AnyRoute, - ): string { - let plan = route ? route._pathCache : this.unmatchedPathCache.get(path) - if (!plan || plan[1 /* path */] !== path) { - const segments = - route?._interpolation ?? parseSegments(false, { fullPath: path }, 0) - plan = [createSieveCache(128), path, segments] - if (route) { - route._pathCache = plan - } else { - this.unmatchedPathCache.set(path, plan) - } - } - const paths = plan[0 /* paths */] - const keys = getParamNames(plan[2 /* segments */]) - // Single-param templates use the value itself as the Map key. Compound keys - // concatenate `:` tokens; undefined becomes - // `undefined:undefined`, whose nonnumeric prefix cannot match a string token. - let key: string | undefined = '' - for (const name of keys) { - const value = params[name] - if (typeof value !== 'string' && value !== undefined) { - return normalizeProtocolRelative( - decodePath( - interpolatePath( - path, - plan[2 /* segments */], - params, - this.pathParamsDecoder, - ), - ), - ) - } - key = keys.length === 1 ? value : key! + value?.length + ':' + value - } - const cached = paths.get(key) - if (cached) { - return cached - } - // Cache canonical pathnames, not the encoded interpolation output. - const interpolated = normalizeProtocolRelative( - decodePath( - interpolatePath( - path, - plan[2 /* segments */], - params, - this.pathParamsDecoder, - ), - ), - ) - paths.set(key, interpolated) - return interpolated - } - /** * Build the next ParsedLocation from navigation options without committing. * Resolves `to`/`from`, params/search/hash/state, applies search validation @@ -1941,6 +1886,15 @@ export class RouterCore< * @link https://tanstack.com/router/latest/docs/framework/react/api/router/RouterType#buildlocation-method */ buildLocation: BuildLocationFn = (opts) => { + const cached = this.staticLocations.get(opts) + if (cached) { + return cached + } + + // Set by `current()` whenever a build reads the current location. A + // location built without it depends only on `opts` and the route tree. + let usedCurrent = false + const build = ( dest: BuildNextOptions & { unmaskOnReload?: boolean @@ -1965,7 +1919,17 @@ export class RouterCore< // Use lightweight matching - only computes what buildLocation needs // (fullPath, search, params) without creating full match objects - const lightweightResult = this.matchRoutesLightweight(currentLocation) + const lightweight = this.matchRoutesLightweight(currentLocation) + + // Value-affecting reads of the current location go through these two. + const current = () => { + usedCurrent = true + return currentLocation + } + const currentMatch = () => { + usedCurrent = true + return lightweight + } // check that from path exists in the current route tree // do this check only on navigations during test or development @@ -1975,16 +1939,14 @@ export class RouterCore< dest._isNavigate ) { const [allFromMatches] = this.getMatchedRoutes(dest.from) + const [matchedRoutes, fullPath] = currentMatch() - const matchedFrom = findLast( - lightweightResult[0 /* matchedRoutes */], - (d) => { - return comparePaths(d.fullPath, dest.from!) - }, - ) + const matchedFrom = findLast(matchedRoutes, (d) => { + return comparePaths(d.fullPath, dest.from!) + }) const matchedCurrent = findLast(allFromMatches, (d) => { - return comparePaths(d.fullPath, lightweightResult[1 /* fullPath */]) + return comparePaths(d.fullPath, fullPath) }) // for from to be invalid it shouldn't just be unmatched to currentLocation @@ -1994,24 +1956,17 @@ export class RouterCore< } } - const defaultedFromPath = - dest.unsafeRelative === 'path' - ? currentLocation.pathname - : (dest.from ?? lightweightResult[1 /* fullPath */]) - - // From search should always use the current location - const fromSearch = lightweightResult[2 /* search */] - // Same with params. It can't hurt to provide as many as possible - const fromParams = lightweightResult[3 /* params */] - + const to = dest.to ? `${dest.to}` : '.' const nextTo = this.resolvePathWithBase( - defaultedFromPath, - dest.to ? `${dest.to}` : '.', + // Absolute destinations resolve without a base. + to[0] === '/' + ? '' + : dest.unsafeRelative === 'path' + ? current().pathname + : (dest.from ?? currentMatch()[1 /* fullPath */]), + to, ) - // Resolve the next params - let nextParams = resolveNextParams(dest.params, fromParams) - const destRoute = this.routesByPath[ trimPathRight(nextTo) as keyof typeof this.routesByPath ] as AnyRoute | undefined @@ -2036,34 +1991,62 @@ export class RouterCore< } } - // If there are any params, we need to stringify them - if (destRoutes.length && hasKeys(nextParams)) { - for (const route of destRoutes) { - const fn = - route.options.params?.stringify ?? route.options.stringifyParams - if (fn) { - if (nextParams === fromParams) { - nextParams = Object.assign(createNull(), nextParams) - } - try { - Object.assign(nextParams, fn(nextParams)) - } catch { - // Ignore errors here. When a paired parseParams is defined, - // extractStrictParams will re-throw during route matching, - // storing the error on the match and allowing the route's - // errorComponent to render. If no parseParams is defined, - // the stringify error is silently dropped. - } + // One parsed template serves both trailing-slash variants. + const interpolation = nextTo.includes('$') + ? (destRoute?._interpolation ?? + parseSegments(false, { fullPath: nextTo }, 0)) + : undefined + + let nextParams: Record | undefined + for (const route of destRoutes) { + const fn = + route.options.params?.stringify ?? route.options.stringifyParams + if (fn) { + // Stringifiers receive the merged params, so they always see inherited ones. + const fromParams = currentMatch()[3 /* params */] + nextParams ??= resolveNextParams(dest.params, fromParams) + if (!hasKeys(nextParams)) { + break + } + if (nextParams === fromParams) { + nextParams = Object.assign( + createNull() as Record, + nextParams, + ) + } + try { + Object.assign(nextParams, fn(nextParams)) + } catch { + // Ignore errors here. When a paired parseParams is defined, + // extractStrictParams will re-throw during route matching, + // storing the error on the match and allowing the route's + // errorComponent to render. If no parseParams is defined, + // the stringify error is silently dropped. } } } + nextParams ??= resolveNextParams( + dest.params, + needsInheritedParams(dest.params, interpolation) + ? currentMatch()[3 /* params */] + : EMPTY_RECORD, + ) const nextPathname = opts.leaveParams ? // Keep path params uninterpolated for matchRoute/template matching. nextTo - : nextTo.includes('$') - ? this.interpolatePath(nextTo, nextParams, destRoute) - : normalizeProtocolRelative(decodePath(nextTo)) + : normalizeProtocolRelative( + decodePath( + interpolation + ? interpolatePath( + nextTo, + interpolation, + nextParams, + this.pathParamsDecoder, + ) + : nextTo, + ), + ) if ( process.env.NODE_ENV !== 'production' && @@ -2084,36 +2067,45 @@ export class RouterCore< } // Resolve the next search - let nextSearch = fromSearch - if (opts._includeValidateSearch && this.options.search?.strict) { - const validatedSearch = {} - destRoutes.forEach((route) => { - if (route.options.validateSearch) { - try { - Object.assign( - validatedSearch, - validateSearch(route.options.validateSearch, { - ...validatedSearch, - ...nextSearch, - }), - ) - } catch { - // ignore errors here because they are already handled in matchRoutes - } - } - }) - nextSearch = validatedSearch - } - - nextSearch = applySearchMiddleware( - nextSearch, - dest, + const middlewares = getSearchMiddlewares( destRoutes, opts._includeValidateSearch, ) - - // Replace the equal deep - nextSearch = nullReplaceEqualDeep(fromSearch, nextSearch) + const fromSearch = () => { + let search = currentMatch()[2 /* search */] + if (opts._includeValidateSearch && this.options.search?.strict) { + const validatedSearch = {} + destRoutes.forEach((route) => { + if (route.options.validateSearch) { + try { + Object.assign( + validatedSearch, + validateSearch(route.options.validateSearch, { + ...validatedSearch, + ...search, + }), + ) + } catch { + // ignore errors here because they are already handled in matchRoutes + } + } + }) + search = validatedSearch + } + return search + } + // A literal search never reads the current one. + let nextSearch: Record = middlewares.length + ? applySearchMiddleware(middlewares, fromSearch(), dest) + : dest.search === true + ? fromSearch() + : typeof dest.search === 'function' + ? 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,24 +2113,24 @@ export class RouterCore< // Resolve the next hash const hash = dest.hash === true - ? currentLocation.hash - : dest.hash - ? functionalUpdate(dest.hash, currentLocation.hash) - : undefined + ? current().hash + : typeof dest.hash === 'function' + ? dest.hash(current().hash) + : dest.hash || undefined // Resolve the next hash string const hashStr = hash ? `#${hash}` : '' // Resolve the next state - let nextState = - dest.state === true - ? currentLocation.state - : dest.state - ? functionalUpdate(dest.state, currentLocation.state) - : {} - - // Replace the equal deep + 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) } @@ -2216,6 +2208,11 @@ export class RouterCore< } } + // Masked locations stay out: `opts.mask` is rebuilt from the current location. + if (!usedCurrent && opts._fromLocation && !next.maskedLocation) { + this.staticLocations.set(opts, next) + } + return next } @@ -2302,8 +2299,11 @@ export class RouterCore< } } - nextHistory.state.__hashScrollIntoViewOptions = - hashScrollIntoView ?? this.options.defaultHashScrollIntoView ?? true + nextHistory.state = { + ...nextHistory.state, + __hashScrollIntoViewOptions: + hashScrollIntoView ?? this.options.defaultHashScrollIntoView ?? true, + } this.shouldViewTransition = viewTransition @@ -2878,6 +2878,30 @@ function resolveNextParams( return Object.assign(next, base, spec) } +// Inherited params are read by param updaters and by template keys the +// destination leaves open. `interpolation` is undefined without template keys. +function needsInheritedParams( + spec: unknown, + interpolation: RouteInterpolation | undefined, +) { + if (typeof spec === 'function') { + return true + } + if (!interpolation || spec === false || spec === null) { + return false + } + return ( + spec === undefined || + spec === true || + interpolation.some( + (part) => + typeof part !== 'string' && !hasOwn.call(spec, part[1 /* key */]), + ) + ) +} + +const EMPTY_RECORD: Record = Object.freeze({}) + // Keep this separate from recursive execution to limit JIT compiler memory. function getSearchMiddlewares( destRoutes: ReadonlyArray, @@ -2943,12 +2967,10 @@ function getSearchMiddlewares( } function applySearchMiddleware( + middlewares: Array>, search: any, dest: BuildNextOptions, - destRoutes: ReadonlyArray, - includeValidateSearch: boolean | undefined, ) { - const middlewares = getSearchMiddlewares(destRoutes, includeValidateSearch) const applyNext = ( index: number, currentSearch: any, diff --git a/packages/router-core/tests/path-decoder.test.ts b/packages/router-core/tests/path-decoder.test.ts index 941626e53d..bf27b93134 100644 --- a/packages/router-core/tests/path-decoder.test.ts +++ b/packages/router-core/tests/path-decoder.test.ts @@ -34,17 +34,15 @@ function setup(allowed?: Array<'@' | '+'>) { router.history.destroy() const build = () => router.buildLocation({ to: '/items/$id', params: { id: '@+' } }).href - return { router, item, build } + return { router, build } } -test('keeps the decoder and cached paths across provider option updates', () => { +test('keeps the decoder across provider option updates', () => { const allowed: Array<'@' | '+'> = ['@'] - const { router, item, build } = setup(allowed) + const { router, build } = setup(allowed) const decoder = router.pathParamsDecoder expect(build()).toBe('/items/@%2B') - const plan = item._pathCache const compile = vi.spyOn(pathUtils, 'compileDecodeCharMap') - const interpolate = vi.spyOn(pathUtils, 'interpolatePath') for (let count = 0; count < 3; count++) { router.update({ ...router.options, @@ -52,10 +50,8 @@ test('keeps the decoder and cached paths across provider option updates', () => }) expect(build()).toBe('/items/@%2B') expect(router.pathParamsDecoder).toBe(decoder) - expect(item._pathCache).toBe(plan) } expect(compile).not.toHaveBeenCalled() - expect(interpolate).not.toHaveBeenCalled() }) test('requires a new router to apply changes to the original character array', () => { diff --git a/packages/router-core/tests/path-interpolation.bench.ts b/packages/router-core/tests/path-interpolation.bench.ts index 9f1891f151..0f6840752d 100644 --- a/packages/router-core/tests/path-interpolation.bench.ts +++ b/packages/router-core/tests/path-interpolation.bench.ts @@ -1,10 +1,14 @@ import { bench, describe, expect } from 'vitest' -import { createMemoryHistory } from '@tanstack/history' +import { + createMemoryHistory, + normalizeProtocolRelative, +} from '@tanstack/history' import { BaseRootRoute, BaseRoute } from '../src' import { compileDecodeCharMap, interpolatePath } from '../src/path' import { parseSegments } from '../src/new-process-route-tree' import { decodePath } from '../src/utils' import { createTestRouter, interpolateTestPath } from './routerTestUtils' +import type { AnyRoute } from '../src' import type { PathInterpolationTestOptions } from './routerTestUtils' const scenarios: Array<{ @@ -142,6 +146,22 @@ describe.each(scenarios)('$name', ({ inputs, register = true }) => { scrollRestoration: false, }) router.history.destroy() + // Mirrors how buildLocation turns a template into a canonical pathname. + const canonicalPathname = ( + path: string, + params: Record, + route: AnyRoute | undefined, + ) => + normalizeProtocolRelative( + decodePath( + interpolatePath( + path, + route?._interpolation ?? parseSegments(false, { fullPath: path }, 0), + params, + router.pathParamsDecoder, + ), + ), + ) const calls = inputs .filter((input) => input.path.includes('$')) .map((input) => ({ @@ -172,13 +192,13 @@ describe.each(scenarios)('$name', ({ inputs, register = true }) => { 0, ) const cachedExpected = calls.reduce((sum, call) => { - expect(router['interpolatePath'](call.path, call.params, call.route)).toBe( + expect(canonicalPathname(call.path, call.params, call.route)).toBe( call.expected, ) return sum + call.expected.length }, 0) for (const call of calls) { - expect(router['interpolatePath'](call.path, call.params, call.route)).toBe( + expect(canonicalPathname(call.path, call.params, call.route)).toBe( call.expected, ) } @@ -191,15 +211,11 @@ describe.each(scenarios)('$name', ({ inputs, register = true }) => { })) bench( - 'shared interpolation and normalization batch', + 'interpolation and normalization batch', () => { let length = 0 for (const call of calls) { - length += router['interpolatePath']( - call.path, - call.params, - call.route, - ).length + length += canonicalPathname(call.path, call.params, call.route).length } checksum = length }, diff --git a/packages/router-core/tests/path.test.ts b/packages/router-core/tests/path.test.ts index 5fb2b02628..7d6eed9de7 100644 --- a/packages/router-core/tests/path.test.ts +++ b/packages/router-core/tests/path.test.ts @@ -1,5 +1,4 @@ import { afterEach, describe, expect, it, vi } from 'vitest' -import * as pathUtils from '../src/path' import { compileDecodeCharMap, exactPathTest, @@ -25,7 +24,7 @@ import type { SegmentKind } from '../src/new-process-route-tree' afterEach(() => vi.restoreAllMocks()) describe.each([false, true])( - 'shared pathname interpolation (server: %s)', + 'pathname interpolation (server: %s)', (server) => { it.each([ { path: '/', params: {}, expected: '/' }, @@ -88,7 +87,7 @@ describe.each([false, true])( expected: '/123/123/', }, ])( - 'interpolates and caches $path with $params', + 'interpolates $path with $params', ({ path, params, expected, normalized }) => { const interpolate = createPathInterpolator({ isServer: server }) const options = { path, params, server } @@ -102,37 +101,7 @@ describe.each([false, true])( }, ) - it('shares results across equivalent params without retaining unrelated params', () => { - const interpolate = createPathInterpolator({ - isServer: server, - pathParamsAllowedCharacters: ['@'], - }) - const format = vi.spyOn(pathUtils, 'interpolatePath') - const options = { - path: '/users/$id', - params: { id: '@one', unrelated: 'first' }, - server, - } - expect(interpolate(options)).toBe('/users/@one') - expect(format).toHaveBeenCalledOnce() - - expect( - interpolate({ - ...options, - params: { id: '@one', unrelated: 'second' }, - }), - ).toBe('/users/@one') - expect(format).toHaveBeenCalledOnce() - - const anotherRouter = createPathInterpolator({ - isServer: server, - pathParamsAllowedCharacters: ['@'], - }) - expect(anotherRouter(options)).toBe('/users/@one') - expect(format).toHaveBeenCalledTimes(2) - }) - - it('normalizes non-string fallbacks without memoizing object coercion', () => { + it('formats non-string values on every call', () => { const interpolate = createPathInterpolator({ isServer: server }) const toString = vi.fn(() => 'one two') const options = { @@ -168,7 +137,7 @@ describe.each([false, true])( expect(allowAt(options)).toBe('/users/@%2B') }) - it('keeps each router template cache independent of other encodings', () => { + it('keeps each router encoding independent across templates', () => { const allowAt = createPathInterpolator({ isServer: server, pathParamsAllowedCharacters: ['@'], @@ -192,7 +161,7 @@ describe.each([false, true])( } }) - it('does not confuse parameter boundaries in shared cache keys', () => { + it('does not confuse parameter boundaries', () => { const interpolate = createPathInterpolator({ isServer: server }) const options = { path: '/$first/$second', server } const first = { first: 'a:b', second: 'c' } @@ -212,7 +181,7 @@ describe.each([false, true])( expect(interpolate({ path: '/users/$id', params })).toBe('/users/123') }) - it('tracks optional params that were absent when the template was first used', () => { + it('formats optional params from their current values', () => { const interpolate = createPathInterpolator({ isServer: server }) const path = '/posts/{-$category}/$id' const inputs = [ @@ -238,31 +207,11 @@ describe.each([false, true])( } }) - it('caches paths with omitted optional params', () => { - const interpolate = createPathInterpolator({ - isServer: server, - pathParamsAllowedCharacters: ['@'], - }) - const format = vi.spyOn(pathUtils, 'interpolatePath') - const options = { - path: '/{-$lang}/foo/$id', - params: { id: '@one' }, - server, - } - - expect(interpolate(options)).toBe('/foo/@one') - expect(interpolate({ ...options, params: { id: '@one' } })).toBe( - '/foo/@one', - ) - expect(format).toHaveBeenCalledOnce() - }) - it('uses the canonical splat value instead of its legacy alias', () => { const interpolate = createPathInterpolator({ isServer: server, pathParamsAllowedCharacters: ['@'], }) - const format = vi.spyOn(pathUtils, 'interpolatePath') const options = { path: '/files/$', params: { _splat: 'docs/@guide', '*': 'ignored' }, @@ -270,14 +219,12 @@ describe.each([false, true])( } expect(interpolate(options)).toBe('/files/docs/@guide') - format.mockClear() expect( interpolate({ ...options, params: { _splat: 'docs/@guide', '*': 'changed' }, }), ).toBe('/files/docs/@guide') - expect(format).not.toHaveBeenCalled() }) it('collects used params for splats but not missing optionals', () => { diff --git a/packages/router-core/tests/route-tree-caches.test.ts b/packages/router-core/tests/route-tree-caches.test.ts index 207784525c..302dfa62f1 100644 --- a/packages/router-core/tests/route-tree-caches.test.ts +++ b/packages/router-core/tests/route-tree-caches.test.ts @@ -1,7 +1,6 @@ import { afterEach, beforeEach, expect, test, vi } from 'vitest' import { createMemoryHistory } from '@tanstack/history' import { BaseRootRoute, BaseRoute } from '../src' -import * as pathUtils from '../src/path' import * as routeTreeUtils from '../src/new-process-route-tree' import { createRequestHandler } from '../src/ssr/createRequestHandler' import { createTestRouter } from './routerTestUtils' @@ -39,9 +38,9 @@ function history() { } test.each([false, true])( - 'caches the canonical pathname without changing hrefs (server: %s)', + 'builds canonical pathnames and encoded hrefs (server: %s)', (isServer) => { - const { routeTree, item } = createRoutes() + const { routeTree } = createRoutes() const router = createTestRouter({ routeTree, history: history(), isServer }) for (const [id, pathname, href] of [ ['one two', '/items/one two', '/items/one%20two'], @@ -59,7 +58,6 @@ test.each([false, true])( }) expect(result.pathname).toBe(pathname) expect(result.href).toBe(href) - expect(item._pathCache?.[0 /* paths */].get(id)).toBe(pathname) } } }, @@ -78,10 +76,6 @@ test('reuses server caches after request cleanup without sharing match state', a }, })(({ router }) => { disposers.push(router.history.destroy) - router.buildLocation({ - to: '/pretty/$id', - params: { id: 'one two' }, - }) return new Response( router.buildLocation({ to: '/items/$id', @@ -92,28 +86,15 @@ test('reuses server caches after request cleanup without sharing match state', a expect(await (await request()).text()).toBe('/items/one%20two') const branch = item._branch - const plan = item._pathCache const firstLocation = routers[0]!.latestLocation const lightweightResult = routers[0]!['lightweightCache'].get(firstLocation) expect(branch).toEqual([root, item]) - expect(plan).toBeDefined() expect(lightweightResult).toBeDefined() - const interpolate = vi.spyOn(pathUtils, 'interpolatePath') expect(await (await request()).text()).toBe('/items/one%20two') expect(item._branch).toBe(branch) - expect(item._pathCache).toBe(plan) - expect( - interpolate.mock.calls.filter(([path]) => path === '/items/$id'), - ).toHaveLength(0) - expect( - interpolate.mock.calls.filter(([path]) => path === '/pretty/$id'), - ).toHaveLength(0) expect(routers[0]).not.toBe(routers[1]) expect(routers[0]!.resolvePathCache).toBe(routers[1]!.resolvePathCache) - expect(routers[0]!['unmatchedPathCache']).toBe( - routers[1]!['unmatchedPathCache'], - ) expect(routers[0]!['lightweightCache']).not.toBe( routers[1]!['lightweightCache'], ) @@ -140,7 +121,6 @@ test.each([ isServer, }) expect(first.resolvePathCache).not.toBe(second.resolvePathCache) - expect(first['unmatchedPathCache']).not.toBe(second['unmatchedPathCache']) expect(first['lightweightCache']).not.toBe(second['lightweightCache']) expect(globalThis.__TSR_CACHE__).toBeUndefined() }, @@ -164,21 +144,16 @@ test('keeps different route objects independent and resets derived caches when r ).toBe('/items/one') } expect(firstTree.item._branch).not.toBe(secondTree.item._branch) - expect(firstTree.item._pathCache).not.toBe(secondTree.item._pathCache) expect(firstTree.item._interpolation).not.toBe(secondTree.item._interpolation) const previousResolve = first.resolvePathCache - const previousFallback = first['unmatchedPathCache'] const previousLightweight = first['lightweightCache'] const previousBranch = firstTree.item._branch - const previousPlan = firstTree.item._pathCache const previousInterpolation = firstTree.item._interpolation first.setRoutes(first.buildRouteTree()) expect(first.resolvePathCache).not.toBe(previousResolve) - expect(first['unmatchedPathCache']).not.toBe(previousFallback) expect(first['lightweightCache']).not.toBe(previousLightweight) expect(firstTree.item._branch).toBeUndefined() - expect(firstTree.item._pathCache).toBe(previousPlan) expect(firstTree.item._interpolation).not.toBe(previousInterpolation) expect([...firstTree.item._interpolation!]).toEqual([ ...previousInterpolation!, @@ -187,39 +162,9 @@ test('keeps different route objects independent and resets derived caches when r first.buildLocation({ to: '/items/$id', params: { id: 'one' } }).href, ).toBe('/items/one') expect(firstTree.item._branch).not.toBe(previousBranch) - expect(firstTree.item._pathCache).toBe(previousPlan) -}) - -test('keeps more than 32 registered route templates warm', () => { - const root = new BaseRootRoute({}) - const routes = Array.from( - { length: 128 }, - (_, index) => - new BaseRoute({ - getParentRoute: () => root, - path: `/section-${index}/$id`, - }), - ) - const router = createTestRouter({ - routeTree: root.addChildren(routes), - history: history(), - pathParamsAllowedCharacters: ['@'], - }) - const interpolate = vi.spyOn(pathUtils, 'interpolatePath') - for (let round = 0; round < 2; round++) { - for (let index = 0; index < routes.length; index++) { - expect( - router.buildLocation({ - to: `/section-${index}/$id`, - params: { id: '@one' }, - }).pathname, - ).toBe(`/section-${index}/@one`) - } - expect(interpolate).toHaveBeenCalledTimes(routes.length) - } }) -test('refreshes branches and path plans when an existing route is reparented', () => { +test('refreshes branches and segments when an existing route is reparented', () => { vi.stubEnv('NODE_ENV', 'development') const root = new BaseRootRoute({}) const left = new BaseRoute({ getParentRoute: () => root, path: '/left' }) @@ -238,7 +183,6 @@ test('refreshes branches and path plans when an existing route is reparented', ( .href, ).toBe('/left/child/one%20two') expect(child._branch).toEqual([root, left, child]) - const plan = child._pathCache const previousInterpolation = child._interpolation parent = right @@ -246,7 +190,6 @@ test('refreshes branches and path plans when an existing route is reparented', ( right.addChildren([child]) router.setRoutes(router.buildRouteTree()) expect(child._branch).toBeUndefined() - expect(child._pathCache).toBe(plan) expect(child._interpolation).not.toBe(previousInterpolation) expect(child.fullPath).toBe('/right/child/$id') expect(child._interpolation?.[0]).toBe('/right/child') @@ -256,7 +199,6 @@ test('refreshes branches and path plans when an existing route is reparented', ( .href, ).toBe('/right/child/one%20two') expect(child._branch).toEqual([root, right, child]) - expect(child._pathCache).not.toBe(plan) expect(parse).not.toHaveBeenCalled() }) @@ -280,29 +222,6 @@ test('uses updated callbacks with rebuilt segments in development', () => { expect(parse).not.toHaveBeenCalled() }) -test('keeps the 128-result bound within each route', () => { - const { routeTree } = createRoutes() - const router = createTestRouter({ - routeTree, - history: history(), - pathParamsAllowedCharacters: ['@'], - }) - const interpolate = vi.spyOn(pathUtils, 'interpolatePath') - for (let id = 0; id <= 128; id++) { - expect( - router.buildLocation({ - to: '/items/$id', - params: { id: `@${id}` }, - }).pathname, - ).toBe(`/items/@${id}`) - } - interpolate.mockClear() - expect( - router.buildLocation({ to: '/items/$id', params: { id: '@0' } }).pathname, - ).toBe('/items/@0') - expect(interpolate).toHaveBeenCalledOnce() -}) - test('uses fixed encodings and trailing-slash policies on independent routes', () => { const allowAt = createTestRouter({ routeTree: createRoutes().routeTree, @@ -344,50 +263,6 @@ test('uses fixed encodings and trailing-slash policies on independent routes', ( ).toBe('/items/@%2B') }) -test('parses unregistered templates once per immutable router', () => { - const parse = vi.spyOn(routeTreeUtils, 'parseSegments') - for (const allowed of [undefined, ['@'], ['+']] as const) { - const router = createTestRouter({ - routeTree: createRoutes().routeTree, - history: history(), - pathParamsAllowedCharacters: allowed, - }) - for (let id = 0; id < 140; id++) { - expect( - router.buildLocation({ - to: '/unregistered/$id', - params: { id: String(id) }, - }).pathname, - ).toBe(`/unregistered/${id}`) - } - } - expect(parse).toHaveBeenCalledTimes(3) -}) - -test('keeps client-local template caches independent of other fixed encodings', () => { - const first = createTestRouter({ - routeTree: createRoutes().routeTree, - history: history(), - isServer: false, - pathParamsAllowedCharacters: ['@'], - }) - const second = createTestRouter({ - routeTree: createRoutes().routeTree, - history: history(), - isServer: false, - pathParamsAllowedCharacters: ['+'], - }) - const destination = { to: '/unregistered/$id', params: { id: '@+' } } - expect(first.buildLocation(destination).pathname).toBe('/unregistered/@%2B') - expect( - second.buildLocation({ to: '/items/$id', params: { id: '@+' } }).pathname, - ).toBe('/items/%40+') - expect(second.buildLocation(destination).pathname).toBe('/unregistered/%40+') - const interpolate = vi.spyOn(pathUtils, 'interpolatePath') - expect(first.buildLocation(destination).pathname).toBe('/unregistered/@%2B') - expect(interpolate).not.toHaveBeenCalled() -}) - test('does not reparse a legacy fallback already registered in the tree', () => { const { routeTree, item } = createRoutes() const parse = vi.spyOn(routeTreeUtils, 'parseSegments') @@ -401,14 +276,13 @@ test('does not reparse a legacy fallback already registered in the tree', () => expect(parse).not.toHaveBeenCalled() }) -test('retains a bounded fallback for unregistered mask templates', () => { +test('formats masked destinations without registered templates', () => { const { routeTree } = createRoutes() const router = createTestRouter({ routeTree, history: history(), pathParamsAllowedCharacters: ['@'], }) - const interpolate = vi.spyOn(pathUtils, 'interpolatePath') const build = () => router.buildLocation({ to: '/items/$id', @@ -417,29 +291,5 @@ test('retains a bounded fallback for unregistered mask templates', () => { }) expect(build().maskedLocation?.pathname).toBe('/pretty/@one') - interpolate.mockClear() expect(build().maskedLocation?.pathname).toBe('/pretty/@one') - expect(interpolate).not.toHaveBeenCalled() - - const templates = Array.from( - { length: 64 }, - (_, index) => `/unregistered-${index}/$id`, - ) - for (const to of templates) { - router.buildLocation({ - to, - params: { id: '@one' }, - }) - } - const cache = router['unmatchedPathCache'] - expect( - ['/pretty/$id', ...templates].filter((path) => cache.get(path)), - ).toHaveLength(32) - const evicted = templates.find((path) => !cache.get(path))! - expect(evicted).toBeDefined() - interpolate.mockClear() - expect( - router.buildLocation({ to: evicted, params: { id: '@one' } }).pathname, - ).toBe(evicted.replace('$id', '@one')) - expect(interpolate).toHaveBeenCalledOnce() }) diff --git a/packages/router-core/tests/routerTestUtils.ts b/packages/router-core/tests/routerTestUtils.ts index 015736b13c..51a785819f 100644 --- a/packages/router-core/tests/routerTestUtils.ts +++ b/packages/router-core/tests/routerTestUtils.ts @@ -105,13 +105,15 @@ export function createTestPathInterpolator( routeTree: new BaseRootRoute({}), history: createMemoryHistory({ initialEntries: ['/'] }), scrollRestoration: false, + trailingSlash: 'preserve', ...options, }) router.history.destroy() return ( options: Pick, ): string => { - return router['interpolatePath'](options.path, options.params) + return router.buildLocation({ to: options.path, params: options.params }) + .pathname } } From d03b0c82e1ecebfc969a70930c1801c4f82af446 Mon Sep 17 00:00:00 2001 From: Manuel Schiller <6340397+schiller-manuel@users.noreply.github.com> Date: Fri, 11 Sep 2026 21:54:46 +0200 Subject: [PATCH 2/4] perf(react-router): keep one options object per Link so locations are reused `buildLocation` now returns a cached location for an options object whose previous build never read the current location (06e4504605). The React Link built a fresh `{ _fromLocation, ..._options }` on every navigation, so it never hit that cache. Each Link now owns one `dest` object, memoized on `_options`. The location selector sets `dest._fromLocation` in place (unless the caller supplied one) and passes the same object to `router.buildLocation`, so a Link whose destination does not depend on the current location costs a WeakMap lookup per navigation instead of a full build. `navigate` and `preloadRoute` still receive `_options`, so the in-place field never reaches a navigation. Because object identity is now the router's invalidation signal, `useValueStable` keeps a shallow copy of `params`/`search`/`activeOptions` rather than the caller's object. `deepEqual` therefore never short-circuits on reference equality, and a params object that was mutated in place, or is backed by accessors, yields a new reference (hence a new `dest`) on the render that observes the change. A mutation that no render observes is no longer picked up by a navigation alone; that matches how every other Link input already behaves. The server branch had its own inline copy of the active-state, class and style derivation while the client used `resolveIsActive`. Both now share `resolveIsActive` and a `resolveStateProps` helper; the results are identical (`exactPathTest` is the same trailing-slash comparison, `deepEqual` already treats two key-less search objects as equal, and the hash check yields `false` on the server because it is never hydrated there). The two `blockedLink` spreads that position state props before or after `ref`/handlers are documented in place. Tests (`link-destination.test.tsx`): the middleware test follows the HMR sequence (`route.update()` then `router.setRoutes(router.buildRouteTree())`) instead of expecting a bare `route.update()` to be observed; the test for an impure `stringifySearch` closing over a mutable variable is dropped along with that contract; the mutation and accessor tests re-render the fixture and assert the updated location survives further navigations. Performance (Apple M4, Node 24, fresh production bundles, 3 runs each; measured on a tree that also carried the replaceEqualDeep changes now proposed separately in #8362, #8363 and #8364): - links/react (200 mounted Links, 8 navigations per lap): 290 -> 622 hz against the parent commit, +82% against stack HEAD (341 hz). 160 of the 161 `buildLocation` calls per navigation become 13 ns cache hits; a full build of these Links costs 680-970 ns. - Paired link-perf client vs the parent commit: shared-params -24%, splats -35%, encoding -49%, active -36%, unique-params -25% (wide interval), middleware unchanged (never cacheable). - Combined with the parent, vs origin/main: client shared-params -34%, unique-params -36%, splats -41%, encoding -54%, active -44%, middleware -14%, relative -9%; SSR -12..-32% on every measured case. - Bundle (react-router.minimal gzip, this tree): 85992 -> 86038 (+46); +113 vs stack HEAD, +45 vs origin/main (85993). With #8362, #8363 and #8364 applied as well the tree measures 85983, 10 bytes below origin/main. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- packages/react-router/src/link.tsx | 230 +++++------------- .../tests/link-destination.test.tsx | 71 +++--- 2 files changed, 93 insertions(+), 208 deletions(-) diff --git a/packages/react-router/src/link.tsx b/packages/react-router/src/link.tsx index 7a76081742..2d63f385b2 100644 --- a/packages/react-router/src/link.tsx +++ b/packages/react-router/src/link.tsx @@ -4,10 +4,8 @@ import * as React from 'react' import { useSelector } from '@tanstack/react-store' import { deepEqual, - exactPathTest, functionalUpdate, getUrlScheme, - hasKeys, isDangerousProtocol, preloadWarning, removeTrailingSlash, @@ -42,16 +40,20 @@ type LinkState = [href: string | undefined, isActive?: boolean] // otherwise change `_options` identity on every parent render, rebuild the // store selector, and discard its memoized selection. // +// The kept value is a shallow copy, never the caller's object: the router +// reuses a built location for as long as it sees the same options object, so +// a params object mutated in place, or one backed by accessors, has to yield +// a new reference here on the render that observes the change. +// // `ignoreUndefined: false` is required: an explicit `undefined` clears an // inherited param or search key, so `{}` and `{ category: undefined }` build // different locations and must not be treated as equal here. function useValueStable(value: T): T { - const ref = React.useRef(value) - // `deepEqual` short-circuits on reference equality, so this covers both cases. + const ref = React.useRef(undefined) if (!deepEqual(ref.current, value, { ignoreUndefined: false })) { - ref.current = value + ref.current = value && typeof value === 'object' ? { ...value } : value } - return ref.current + return ref.current as T } function compareLinkState(a: LinkState, b: LinkState) { @@ -203,10 +205,6 @@ export function useLinkProps< ? router.buildLocation(options as any) : undefined - // Use publicHref - it contains the correct href for display - // When a rewrite changes the origin, publicHref is the full URL - // Otherwise it's the origin-stripped path - // This avoids constructing URL objects in the hot path const hrefOption = next ? getHrefOption(next, router, disabled) : (directExternalLink ?? undefined) @@ -216,76 +214,6 @@ export function useLinkProps< directExternalLink ?? (hrefOption && getUrlScheme(hrefOption) ? hrefOption : undefined) - const isActive = (() => { - if (!next || (!disabled && !hrefOption) || externalLink) { - return false - } - - const currentLocation = router.stores.location.get() - - const exact = activeOptions?.exact ?? false - - if (exact) { - const testExact = exactPathTest( - currentLocation.pathname, - next.pathname, - router.basepath, - ) - if (!testExact) { - return false - } - } else { - const currentPathSplit = removeTrailingSlash( - currentLocation.pathname, - router.basepath, - ) - const nextPathSplit = removeTrailingSlash( - next.pathname, - router.basepath, - ) - - const pathIsFuzzyEqual = - currentPathSplit.startsWith(nextPathSplit) && - (currentPathSplit.length === nextPathSplit.length || - currentPathSplit[nextPathSplit.length] === '/') - - if (!pathIsFuzzyEqual) { - return false - } - } - - const includeSearch = activeOptions?.includeSearch ?? true - if (includeSearch) { - if (currentLocation.search !== next.search) { - const currentSearchEmpty = - !currentLocation.search || - (typeof currentLocation.search === 'object' && - !hasKeys(currentLocation.search)) - const nextSearchEmpty = - !next.search || - (typeof next.search === 'object' && - !hasKeys(next.search as Record)) - - if (!(currentSearchEmpty && nextSearchEmpty)) { - const searchTest = deepEqual(currentLocation.search, next.search, { - partial: !exact, - ignoreUndefined: !activeOptions?.explicitUndefined, - }) - if (!searchTest) { - return false - } - } - } - } - - // Hash is not available on the server - if (activeOptions?.includeHash) { - return false - } - - return true - })() - if (externalLink) { return { ...propsSafeToSpread, @@ -299,79 +227,27 @@ export function useLinkProps< } } - const resolvedActiveProps: React.HTMLAttributes = - isActive - ? (functionalUpdate(activeProps as any, {}) ?? STATIC_ACTIVE_OBJECT) - : STATIC_EMPTY_OBJECT - - const resolvedInactiveProps: React.HTMLAttributes = - isActive - ? STATIC_EMPTY_OBJECT - : (functionalUpdate(inactiveProps, {}) ?? STATIC_EMPTY_OBJECT) - - const resolvedStyle = (() => { - const baseStyle = style - const activeStyle = resolvedActiveProps.style - const inactiveStyle = resolvedInactiveProps.style - - if (!baseStyle && !activeStyle && !inactiveStyle) { - return undefined - } - - if (baseStyle && !activeStyle && !inactiveStyle) { - return baseStyle - } - - if (!baseStyle && activeStyle && !inactiveStyle) { - return activeStyle - } - - if (!baseStyle && !activeStyle && inactiveStyle) { - return inactiveStyle - } - - return { - ...baseStyle, - ...activeStyle, - ...inactiveStyle, - } - })() - - const resolvedClassName = (() => { - const baseClassName = className - const activeClassName = resolvedActiveProps.className - const inactiveClassName = resolvedInactiveProps.className - - if (!baseClassName && !activeClassName && !inactiveClassName) { - return '' - } - - let out = '' - - if (baseClassName) { - out = baseClassName - } - - if (activeClassName) { - out = out ? `${out} ${activeClassName}` : activeClassName - } - - if (inactiveClassName) { - out = out ? `${out} ${inactiveClassName}` : inactiveClassName - } - - return out - })() - const blockedLink = !disabled && !hrefOption + // Hash is not available on the server, so it never counts as hydrated. + const isActive = + !!next && + !blockedLink && + resolveIsActive( + router.stores.location.get(), + next, + activeOptions, + router.basepath, + false, + ) + const [resolvedStateProps, resolvedClassName, resolvedStyle] = + resolveStateProps(isActive, activeProps, inactiveProps, className, style) return { ...propsSafeToSpread, - ...(blockedLink ? resolvedActiveProps : STATIC_EMPTY_OBJECT), - ...(blockedLink ? resolvedInactiveProps : STATIC_EMPTY_OBJECT), + // State props may override `ref`, but not on a blocked link (spread first). + ...(blockedLink ? resolvedStateProps : STATIC_EMPTY_OBJECT), ref: innerRef as React.ComponentPropsWithRef<'a'>['ref'], - ...(!blockedLink ? resolvedActiveProps : STATIC_EMPTY_OBJECT), - ...(!blockedLink ? resolvedInactiveProps : STATIC_EMPTY_OBJECT), + ...(!blockedLink ? resolvedStateProps : STATIC_EMPTY_OBJECT), href: hrefOption, disabled: !!linkDisabled, target, @@ -421,6 +297,9 @@ export function useLinkProps< options.unsafeRelative, ], ) + // One stable object per link lets the router reuse location-independent results. + // eslint-disable-next-line react-hooks/rules-of-hooks + const dest = React.useMemo(() => ({ ..._options }) as any, [_options]) // Derive inside the selector so `compareLinkState` can bail out. Deriving after // the subscription instead re-renders every link on every navigation, because @@ -436,10 +315,10 @@ export function useLinkProps< return [directExternalLink ?? undefined] } - const next = router.buildLocation({ - _fromLocation: location, - ..._options, - } as any) + if (!_options._fromLocation) { + dest._fromLocation = location + } + const next = router.buildLocation(dest) // Use publicHref - it contains the correct href for display // When a rewrite changes the origin, publicHref is the full URL @@ -459,7 +338,7 @@ export function useLinkProps< ), ] }, - [stableActiveOptions, disabled, isHydrated, _options, router, to], + [stableActiveOptions, disabled, isHydrated, _options, dest, router, to], ) // eslint-disable-next-line react-hooks/rules-of-hooks @@ -566,20 +445,8 @@ export function useLinkProps< const blockedLink = isActive === undefined - // Only one state contributes props, so resolve and merge it once. - const resolvedStateProps: React.HTMLAttributes = - functionalUpdate(isActive ? (activeProps as any) : inactiveProps, {}) ?? - (isActive ? STATIC_ACTIVE_OBJECT : STATIC_EMPTY_OBJECT) - - const stateClassName = resolvedStateProps.className - const resolvedClassName = className - ? stateClassName - ? `${className} ${stateClassName}` - : className - : stateClassName - const stateStyle = resolvedStateProps.style - const resolvedStyle = - style && stateStyle ? { ...style, ...stateStyle } : style || stateStyle + const [resolvedStateProps, resolvedClassName, resolvedStyle] = + resolveStateProps(isActive, activeProps, inactiveProps, className, style) // The click handler const handleClick = (e: React.MouseEvent) => { @@ -625,6 +492,7 @@ export function useLinkProps< return { ...propsSafeToSpread, + // State props may override `ref` and handlers, but not on a blocked link (spread first). ...(blockedLink ? resolvedStateProps : STATIC_EMPTY_OBJECT), ref: innerRef as React.ComponentPropsWithRef<'a'>['ref'], onClick: composeHandlers(onClick, handleClick), @@ -649,6 +517,34 @@ const STATIC_ACTIVE_OBJECT = { className: 'active' } const STATIC_DISABLED_PROPS = { role: 'link', 'aria-disabled': true } const STATIC_ACTIVE_PROPS = { 'data-status': 'active', 'aria-current': 'page' } +// Only one state contributes props; merge its class and style with the base ones. +function resolveStateProps( + isActive: boolean | undefined, + activeProps: unknown, + inactiveProps: unknown, + className: string | undefined, + style: React.CSSProperties | undefined, +): [ + stateProps: React.HTMLAttributes, + className: string | undefined, + style: React.CSSProperties | undefined, +] { + const stateProps: React.HTMLAttributes = + functionalUpdate((isActive ? activeProps : inactiveProps) as any, {}) ?? + (isActive ? STATIC_ACTIVE_OBJECT : STATIC_EMPTY_OBJECT) + const stateClassName = stateProps.className + const stateStyle = stateProps.style + return [ + stateProps, + className + ? stateClassName + ? `${className} ${stateClassName}` + : className + : stateClassName, + style && stateStyle ? { ...style, ...stateStyle } : style || stateStyle, + ] +} + const timeoutMap = new WeakMap>() const cancelPreload = (eventTarget: object) => { clearTimeout(timeoutMap.get(eventTarget)) diff --git a/packages/react-router/tests/link-destination.test.tsx b/packages/react-router/tests/link-destination.test.tsx index b81d02a706..6447e5aa06 100644 --- a/packages/react-router/tests/link-destination.test.tsx +++ b/packages/react-router/tests/link-destination.test.tsx @@ -9,7 +9,6 @@ import { createRootRoute, createRoute, createRouter, - defaultStringifySearch, retainSearchParams, } from '../src' @@ -25,21 +24,25 @@ describe('Link destination updates', () => { stringify?: (params: Record) => { id: string }, ) { const rootRoute = createRootRoute({ - component: () => ( - <> - - Target - - - - ), + component: function Root() { + const [, rerender] = React.useState(0) + return ( + <> + + + Target + + + + ) + }, }) const itemsRoute = createRoute({ getParentRoute: () => rootRoute, @@ -131,16 +134,18 @@ describe('Link destination updates', () => { expect(stringify).toHaveBeenCalled() }) - test('updates when ancestor search middleware is added and removed', async () => { + test('updates when the route tree is rebuilt after route options change', async () => { const { router, rootRoute } = setupFixedLink() render() const link = await screen.findByTestId('fixed-link') expect(link).toHaveAttribute('href', '/target/fixed#details') + // HMR updates the live route in place, then rebuilds the route tree. rootRoute.update({ search: { middlewares: [retainSearchParams(true)] }, }) + router.setRoutes(router.buildRouteTree()) await act(() => router.navigate({ to: '/items/$source', @@ -151,6 +156,7 @@ describe('Link destination updates', () => { expect(link).toHaveAttribute('href', '/target/fixed?retained=value#details') rootRoute.update({ search: undefined }) + router.setRoutes(router.buildRouteTree()) await act(() => router.navigate({ to: '/items/$source', @@ -161,26 +167,7 @@ describe('Link destination updates', () => { expect(link).toHaveAttribute('href', '/target/fixed#details') }) - test('continues evaluating a custom search serializer', async () => { - const { router } = setupFixedLink() - let language = 'en' - router.update({ - stringifySearch: (search) => - defaultStringifySearch({ ...search, language }), - }) - render() - - const link = await screen.findByTestId('fixed-link') - expect(link).toHaveAttribute('href', '/target/fixed?language=en#details') - - language = 'fr' - await act(() => - router.navigate({ to: '/items/$source', params: { source: 'two' } }), - ) - expect(link).toHaveAttribute('href', '/target/fixed?language=fr#details') - }) - - test('reads accessor-backed params after navigation', async () => { + test('reads accessor-backed params on the next render', async () => { let id = 'one' const { router } = setupFixedLink({ get id() { @@ -193,13 +180,11 @@ describe('Link destination updates', () => { expect(link).toHaveAttribute('href', '/target/one#details') id = 'two' - await act(() => - router.navigate({ to: '/items/$source', params: { source: 'two' } }), - ) + fireEvent.click(screen.getByRole('button', { name: 'Rerender' })) expect(link).toHaveAttribute('href', '/target/two#details') }) - test('updates when an existing params object changes', async () => { + test('updates when an existing params object changes before a render', async () => { const params = { id: 'one' } const { router } = setupFixedLink(params) render() @@ -208,6 +193,10 @@ describe('Link destination updates', () => { expect(link).toHaveAttribute('href', '/target/one#details') params.id = 'two' + fireEvent.click(screen.getByRole('button', { name: 'Rerender' })) + expect(link).toHaveAttribute('href', '/target/two#details') + + // The same object keeps its location across navigations until it changes again. await act(() => router.navigate({ to: '/items/$source', params: { source: 'two' } }), ) From c363888494ecc1690bc935b5476ae3ba41cf22c8 Mon Sep 17 00:00:00 2001 From: Manuel Schiller <6340397+schiller-manuel@users.noreply.github.com> Date: Fri, 11 Sep 2026 21:58:25 +0200 Subject: [PATCH 3/4] refactor: stop exporting isPlainObject and isPlainArray Nothing in the repository imports these two helpers from a package entry point; they are only called inside `utils.ts` by `deepEqual` and `replaceEqualDeep`, and `isPlainArray` is unit-tested via `../src/utils`. #8363 tightens `isPlainObject` to a constructor check that is meant for the router's own structural sharing, so the helpers are no longer exported from `@tanstack/router-core`, `@tanstack/react-router`, `@tanstack/solid-router` or `@tanstack/vue-router`. No bundle change in react-router.minimal (unused exports were already tree-shaken): 85983 before and after. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- packages/react-router/src/index.tsx | 2 -- packages/router-core/src/index.ts | 2 -- packages/solid-router/src/index.tsx | 2 -- packages/vue-router/src/index.tsx | 2 -- 4 files changed, 8 deletions(-) diff --git a/packages/react-router/src/index.tsx b/packages/react-router/src/index.tsx index 6637c7542b..387afa25bc 100644 --- a/packages/react-router/src/index.tsx +++ b/packages/react-router/src/index.tsx @@ -17,8 +17,6 @@ export { stringifySearchWith, functionalUpdate, replaceEqualDeep, - isPlainObject, - isPlainArray, deepEqual, createControlledPromise, retainSearchParams, diff --git a/packages/router-core/src/index.ts b/packages/router-core/src/index.ts index 4e37b8ed71..28d450ad24 100644 --- a/packages/router-core/src/index.ts +++ b/packages/router-core/src/index.ts @@ -314,8 +314,6 @@ export { functionalUpdate, hasKeys, replaceEqualDeep, - isPlainObject, - isPlainArray, deepEqual, createControlledPromise, isModuleNotFoundError, diff --git a/packages/solid-router/src/index.tsx b/packages/solid-router/src/index.tsx index 31be74086f..6cd6020f43 100644 --- a/packages/solid-router/src/index.tsx +++ b/packages/solid-router/src/index.tsx @@ -15,8 +15,6 @@ export { stringifySearchWith, functionalUpdate, replaceEqualDeep, - isPlainObject, - isPlainArray, deepEqual, createControlledPromise, retainSearchParams, diff --git a/packages/vue-router/src/index.tsx b/packages/vue-router/src/index.tsx index 2e14e349f0..ebc8f46aba 100644 --- a/packages/vue-router/src/index.tsx +++ b/packages/vue-router/src/index.tsx @@ -15,8 +15,6 @@ export { stringifySearchWith, functionalUpdate, replaceEqualDeep, - isPlainObject, - isPlainArray, deepEqual, createControlledPromise, retainSearchParams, From d5d73c7611ca9a715fd186ddca677c1534c14080 Mon Sep 17 00:00:00 2001 From: Manuel Schiller <6340397+schiller-manuel@users.noreply.github.com> Date: Fri, 11 Sep 2026 23:05:29 +0200 Subject: [PATCH 4/4] chore: add changesets for Link location reuse and the removed helpers --- .changeset/plain-helpers-hide.md | 8 ++++++++ .changeset/still-links-rest.md | 6 ++++++ 2 files changed, 14 insertions(+) create mode 100644 .changeset/plain-helpers-hide.md create mode 100644 .changeset/still-links-rest.md diff --git a/.changeset/plain-helpers-hide.md b/.changeset/plain-helpers-hide.md new file mode 100644 index 0000000000..78b25ed483 --- /dev/null +++ b/.changeset/plain-helpers-hide.md @@ -0,0 +1,8 @@ +--- +'@tanstack/router-core': patch +'@tanstack/react-router': patch +'@tanstack/solid-router': patch +'@tanstack/vue-router': patch +--- + +Stop exporting the internal `isPlainObject` and `isPlainArray` helpers. diff --git a/.changeset/still-links-rest.md b/.changeset/still-links-rest.md new file mode 100644 index 0000000000..770daa9ba9 --- /dev/null +++ b/.changeset/still-links-rest.md @@ -0,0 +1,6 @@ +--- +'@tanstack/router-core': patch +'@tanstack/react-router': patch +--- + +Reuse built locations for Links whose destination does not depend on the current location. `buildLocation` keeps the result per options object when the build never read the current location, and the React `Link` passes one stable options object per instance, so navigations resolve unchanged Links with a lookup instead of a full build. The per-route pathname interpolation cache this replaces is removed. Link `params`, `search` and `activeOptions` are compared by value on render; an object mutated in place is picked up on the next render rather than by a navigation alone.