diff --git a/.changeset/quiet-zeros-stay.md b/.changeset/quiet-zeros-stay.md new file mode 100644 index 0000000000..13ec72d8d3 --- /dev/null +++ b/.changeset/quiet-zeros-stay.md @@ -0,0 +1,5 @@ +--- +'@tanstack/router-core': patch +--- + +Treat `0` and `false` as provided `_splat` values when interpolating paths. Only `undefined`, `null`, and `''` now omit a splat segment, matching how other path params are stringified. diff --git a/benchmarks/client-nav/README.md b/benchmarks/client-nav/README.md index 76e3d3e238..c8ebbf06bc 100644 --- a/benchmarks/client-nav/README.md +++ b/benchmarks/client-nav/README.md @@ -123,7 +123,7 @@ construction. `scenarios/mount` includes construction but also rendering and loading. To isolate initialization, use the core construction benchmark: ```bash -TSR_LINK_PERF=1 CI=1 NX_DAEMON=false pnpm nx run @tanstack/router-core:test:unit --outputStyle=stream --skipRemoteCache -- bench tests/route-tree-construction.bench.ts --run --testNamePattern=1000.routes --outputJson /tmp/route-tree-construction.json +CI=1 NX_DAEMON=false pnpm nx run @tanstack/router-core:test:unit --outputStyle=stream --skipRemoteCache -- bench tests/route-tree-construction.bench.ts --run --testNamePattern=1000.routes --outputJson /tmp/route-tree-construction.json ``` It covers 100, 1,000, and 10,000 static, dynamic, nested, or mixed routes. @@ -138,16 +138,16 @@ router or repeatedly initialized tree is not a construction baseline. `link-performance/` contains additional client-navigation and SSR workloads for focused Link work. They are **not included** in the regular client-nav/SSR -aggregate projects or their CodSpeed build dependencies. Benchmark discovery -also requires `TSR_LINK_PERF=1`; without it, no extended benchmark files or app -bundles are imported. +aggregate projects or their CodSpeed build dependencies; they only run through +the dedicated `@benchmarks/react-link-performance` targets below. Use `-t` to +narrow a run to specific cases. ```bash -TSR_LINK_PERF=1 CI=1 NX_DAEMON=false pnpm nx run @benchmarks/react-link-performance:test:perf:client --outputStyle=stream --skipRemoteCache -- --run -TSR_LINK_PERF=1 CI=1 NX_DAEMON=false pnpm nx run @benchmarks/react-link-performance:test:perf:ssr --outputStyle=stream --skipRemoteCache -- --run +CI=1 NX_DAEMON=false pnpm nx run @benchmarks/react-link-performance:test:perf:client --outputStyle=stream --skipRemoteCache -- --run +CI=1 NX_DAEMON=false pnpm nx run @benchmarks/react-link-performance:test:perf:ssr --outputStyle=stream --skipRemoteCache -- --run # Select a feature and save the normal Vitest JSON report. -TSR_LINK_PERF=1 CI=1 NX_DAEMON=false pnpm nx run @benchmarks/react-link-performance:test:perf:client --outputStyle=stream --skipRemoteCache -- --run -t "updater|optional|splat" --outputJson /tmp/link-perf.json +CI=1 NX_DAEMON=false pnpm nx run @benchmarks/react-link-performance:test:perf:client --outputStyle=stream --skipRemoteCache -- --run -t "updater|optional|splat" --outputJson /tmp/link-perf.json ``` The cases cover repeated versus unique destination params, updater functions, @@ -203,12 +203,12 @@ harness loader does not transpile their emitted JavaScript a second time. ```bash # Build the baseline using these same benchmark sources in its own checkout. # --baseline points to that checkout's link-performance/dist directory. -TSR_LINK_PERF=1 pnpm nx run @benchmarks/react-link-performance:test:perf:stable -- \ +pnpm nx run @benchmarks/react-link-performance:test:perf:stable -- \ --baseline /path/to/baseline/benchmarks/client-nav/link-performance/dist \ --outputJson /tmp/paired-links.json # Narrow a comparison, or increase independent process repetitions. -TSR_LINK_PERF=1 pnpm nx run @benchmarks/react-link-performance:test:perf:stable -- \ +pnpm nx run @benchmarks/react-link-performance:test:perf:stable -- \ --baseline /path/to/baseline/benchmarks/client-nav/link-performance/dist \ --mode ssr -t "middleware|unique-params" --repeats 6 \ --outputJson /tmp/paired-links-ssr.json diff --git a/benchmarks/client-nav/link-performance/config.test.ts b/benchmarks/client-nav/link-performance/config.test.ts index 3f2624677f..1a468a3169 100644 --- a/benchmarks/client-nav/link-performance/config.test.ts +++ b/benchmarks/client-nav/link-performance/config.test.ts @@ -3,55 +3,40 @@ import { createLinkPerformanceConfig } from './config' afterEach(() => vi.unstubAllEnvs()) -describe.each(['client', 'ssr'] as const)( - '%s opt-in configuration', - (target) => { - test.each([undefined, '', '0', 'false'])( - 'does not discover benchmarks with TSR_LINK_PERF=%s', - (value) => { - vi.stubEnv('TSR_LINK_PERF', value) - const config = createLinkPerformanceConfig(target) - expect(config.test?.benchmark?.include).toEqual([]) - expect(config.test?.passWithNoTests).toBe(true) - }, - ) - - test('discovers only the requested suite when explicitly enabled', () => { - vi.stubEnv('TSR_LINK_PERF', '1') - const config = createLinkPerformanceConfig(target) - expect(config.test?.benchmark?.include).toEqual([`${target}.bench.ts`]) - expect(config.test?.passWithNoTests).toBe(false) - }) +describe.each(['client', 'ssr'] as const)('%s configuration', (target) => { + test('discovers only the requested suite', () => { + const config = createLinkPerformanceConfig(target) + expect(config.test?.benchmark?.include).toEqual([`${target}.bench.ts`]) + }) - test('builds production code for the correct environment', () => { - const config = createLinkPerformanceConfig(target) - expect(config.define?.['process.env.NODE_ENV']).toBe('"production"') - expect(config.resolve?.conditions).toContain( - target === 'ssr' ? 'node' : 'browser', - ) - expect(config.build?.ssr).toBe(target === 'ssr') - expect(config.build?.outDir).toBe(`./dist/${target}`) - expect(config.build?.rolldownOptions?.platform).toBe('node') - expect(config.build?.rolldownOptions?.external).toEqual([ - 'node:module', - 'module', - /^react(?:\/|$)/, - /^react-dom(?:\/|$)/, - ]) - }) + test('builds production code for the correct environment', () => { + const config = createLinkPerformanceConfig(target) + expect(config.define?.['process.env.NODE_ENV']).toBe('"production"') + expect(config.resolve?.conditions).toContain( + target === 'ssr' ? 'node' : 'browser', + ) + expect(config.build?.ssr).toBe(target === 'ssr') + expect(config.build?.outDir).toBe(`./dist/${target}`) + expect(config.build?.rolldownOptions?.platform).toBe('node') + expect(config.build?.rolldownOptions?.external).toEqual([ + 'node:module', + 'module', + /^react(?:\/|$)/, + /^react-dom(?:\/|$)/, + ]) + }) - test('does not retransform built bundles in the test environment', () => { - vi.stubEnv('VITEST', 'true') - const config = createLinkPerformanceConfig(target) - expect(config.ssr?.noExternal).toBeUndefined() - expect(config.test?.server?.deps?.external).toEqual([ - /\/link-performance\/dist\//, - ]) - }) + test('does not retransform built bundles in the test environment', () => { + vi.stubEnv('VITEST', 'true') + const config = createLinkPerformanceConfig(target) + expect(config.ssr?.noExternal).toBeUndefined() + expect(config.test?.server?.deps?.external).toEqual([ + /\/link-performance\/dist\//, + ]) + }) - test('bundles router dependencies when building app snapshots', () => { - vi.stubEnv('VITEST', undefined) - expect(createLinkPerformanceConfig(target).ssr?.noExternal).toBe(true) - }) - }, -) + test('bundles router dependencies when building app snapshots', () => { + vi.stubEnv('VITEST', undefined) + expect(createLinkPerformanceConfig(target).ssr?.noExternal).toBe(true) + }) +}) diff --git a/benchmarks/client-nav/link-performance/config.ts b/benchmarks/client-nav/link-performance/config.ts index ef5e4f29ca..416b027057 100644 --- a/benchmarks/client-nav/link-performance/config.ts +++ b/benchmarks/client-nav/link-performance/config.ts @@ -9,7 +9,6 @@ export function createLinkPerformanceConfig( target: 'client' | 'ssr', ): UserConfig { const server = target === 'ssr' - const enabled = process.env.TSR_LINK_PERF === '1' return { root, @@ -62,9 +61,8 @@ export function createLinkPerformanceConfig( }, }, include: [], - passWithNoTests: !enabled, benchmark: { - include: enabled ? [`${target}.bench.ts`] : [], + include: [`${target}.bench.ts`], }, }, } diff --git a/benchmarks/client-nav/link-performance/stable-runner.ts b/benchmarks/client-nav/link-performance/stable-runner.ts index 83f63ff6d2..59ccfa3cb4 100644 --- a/benchmarks/client-nav/link-performance/stable-runner.ts +++ b/benchmarks/client-nav/link-performance/stable-runner.ts @@ -159,10 +159,6 @@ async function sampleReplica( } async function main() { - if (process.env.TSR_LINK_PERF !== '1') { - console.log('Link performance sampling is disabled; set TSR_LINK_PERF=1.') - return - } const { values } = parseArgs({ options: { baseline: { type: 'string' }, diff --git a/benchmarks/ssr/README.md b/benchmarks/ssr/README.md index 1f1c232922..3d90af1598 100644 --- a/benchmarks/ssr/README.md +++ b/benchmarks/ssr/README.md @@ -108,9 +108,8 @@ rewrites, and active props. Each timed batch creates four fresh routers and renders their Links to HTML; it does not measure Start HTTP/streaming overhead. ```bash -TSR_LINK_PERF=1 CI=1 NX_DAEMON=false pnpm nx run @benchmarks/react-link-performance:test:perf:ssr --outputStyle=stream --skipRemoteCache -- --run +CI=1 NX_DAEMON=false pnpm nx run @benchmarks/react-link-performance:test:perf:ssr --outputStyle=stream --skipRemoteCache -- --run ``` These cases are excluded from this directory's aggregate projects and normal -CodSpeed dependency graph. The flag must be explicitly enabled when invoking -the dedicated target. +CodSpeed dependency graph; they only run through the dedicated target above. diff --git a/packages/router-core/src/path.ts b/packages/router-core/src/path.ts index 74247cd0f2..88837db2b1 100644 --- a/packages/router-core/src/path.ts +++ b/packages/router-core/src/path.ts @@ -218,6 +218,11 @@ export function getRouteSegments(route: AnyRoute) { return route._interpolation } +/** A splat is missing when it has no value; `0` and `false` are stringified like any other param. */ +function isMissingSplat(value: unknown): boolean { + return value == null || value === '' +} + /** Devtools checks navigation availability separately from the hot formatter. */ export function hasMissingPathParams( segments: RouteInterpolation, @@ -229,7 +234,7 @@ export function hasMissingPathParams( } const [kind, key] = part return kind === SEGMENT_TYPE_WILDCARD - ? !params[key] + ? isMissingSplat(params[key]) : kind === SEGMENT_TYPE_PARAM && !(key in params) }) } @@ -290,7 +295,7 @@ export function interpolatePath( usedParams['*'] = paramValue } } - if (splat && !paramValue) { + if (splat && isMissingSplat(paramValue)) { // A missing wildcard keeps its affixes, but omits a bare segment. if (prefix === '/' && !suffix) { continue diff --git a/packages/router-core/tests/lightweight-location.bench.ts b/packages/router-core/tests/lightweight-location.bench.ts index a9189801c8..2470f40cc1 100644 --- a/packages/router-core/tests/lightweight-location.bench.ts +++ b/packages/router-core/tests/lightweight-location.bench.ts @@ -3,62 +3,57 @@ import { createMemoryHistory } from '@tanstack/history' import { BaseRootRoute, BaseRoute } from '../src' import { createTestRouter } from './routerTestUtils' -if (process.env.TSR_LINK_PERF === '1') { - describe.each([false, true])( - 'cold lightweight locations (server: %s)', - (isServer) => { - for (const count of [1, 8, 32]) { - const root = new BaseRootRoute({}) - const segments = Array.from( - { length: count }, - (_, index) => `$p${index}`, - ) - const source = new BaseRoute({ - getParentRoute: () => root, - path: `/source/${segments.join('/')}`, - }) - const target = new BaseRoute({ - getParentRoute: () => root, - path: '/target', - }) - const history = createMemoryHistory({ initialEntries: ['/'] }) - const router = createTestRouter({ - routeTree: root.addChildren([source, target]), - history, - isServer, - scrollRestoration: false, - }) - history.destroy() - const pathname = `/source/${segments.map((_, index) => String(index)).join('/')}` - const location = { - ...router.latestLocation, - pathname, - href: pathname, - publicHref: pathname, - } - let checksum = 0 - const iterations = 256 - function run() { - checksum = 0 - for (let index = 0; index < iterations; index++) { - checksum += router.buildLocation({ - to: '/target', - params: true, - _fromLocation: { ...location }, - }).pathname.length - } +describe.each([false, true])( + 'cold lightweight locations (server: %s)', + (isServer) => { + for (const count of [1, 8, 32]) { + const root = new BaseRootRoute({}) + const segments = Array.from({ length: count }, (_, index) => `$p${index}`) + const source = new BaseRoute({ + getParentRoute: () => root, + path: `/source/${segments.join('/')}`, + }) + const target = new BaseRoute({ + getParentRoute: () => root, + path: '/target', + }) + const history = createMemoryHistory({ initialEntries: ['/'] }) + const router = createTestRouter({ + routeTree: root.addChildren([source, target]), + history, + isServer, + scrollRestoration: false, + }) + history.destroy() + const pathname = `/source/${segments.map((_, index) => String(index)).join('/')}` + const location = { + ...router.latestLocation, + pathname, + href: pathname, + publicHref: pathname, + } + let checksum = 0 + const iterations = 256 + function run() { + checksum = 0 + for (let index = 0; index < iterations; index++) { + checksum += router.buildLocation({ + to: '/target', + params: true, + _fromLocation: { ...location }, + }).pathname.length } - run() - expect(checksum).toBe(iterations * '/target'.length) - bench(`${count} source params`, run, { - time: 1000, - warmupTime: 200, - throws: true, - teardown: () => { - expect(checksum).toBe(iterations * '/target'.length) - }, - }) } - }, - ) -} + run() + expect(checksum).toBe(iterations * '/target'.length) + bench(`${count} source params`, run, { + time: 1000, + warmupTime: 200, + throws: true, + teardown: () => { + expect(checksum).toBe(iterations * '/target'.length) + }, + }) + } + }, +) diff --git a/packages/router-core/tests/matching-interpolation.bench.ts b/packages/router-core/tests/matching-interpolation.bench.ts index 8932897ec2..f86cd0b87e 100644 --- a/packages/router-core/tests/matching-interpolation.bench.ts +++ b/packages/router-core/tests/matching-interpolation.bench.ts @@ -39,85 +39,82 @@ const cases = [ }, ] -// Opt in explicitly; these extra cases are not part of default CI benchmarks. -if (process.env.TSR_LINK_PERF === '1') { - for (const server of [false, true]) { - for (const primeLinks of [false, true]) { - describe(`matching interpolation (server: ${server}, Link-primed: ${primeLinks})`, () => { - for (const scenario of cases) { - const root = new BaseRootRoute({}) - let parent: AnyRoute = root - for (const path of scenario.segments) { - const parentRoute = parent - const route = new BaseRoute({ - getParentRoute: () => parentRoute, - path, - }) - parent.addChildren([route]) - parent = route - } - const history = createMemoryHistory({ initialEntries: ['/'] }) - const router = createTestRouter({ - routeTree: root, - history, - isServer: server, - scrollRestoration: false, +for (const server of [false, true]) { + for (const primeLinks of [false, true]) { + describe(`matching interpolation (server: ${server}, Link-primed: ${primeLinks})`, () => { + for (const scenario of cases) { + const root = new BaseRootRoute({}) + let parent: AnyRoute = root + for (const path of scenario.segments) { + const parentRoute = parent + const route = new BaseRoute({ + getParentRoute: () => parentRoute, + path, }) - history.destroy() - const options = { _controller: new AbortController() } + parent.addChildren([route]) + parent = route + } + const history = createMemoryHistory({ initialEntries: ['/'] }) + const router = createTestRouter({ + routeTree: root, + history, + isServer: server, + scrollRestoration: false, + }) + history.destroy() + const options = { _controller: new AbortController() } - for (const path of scenario.paths) { - router.getMatchedRoutes(path) - } - if (primeLinks) { - const paths = scenario.misses - ? scenario.paths - .slice(0, 32) - .map((path) => path.replace('item%20', 'cached%20')) - : scenario.paths - for (const path of paths) { - const [routes, params] = router.getMatchedRoutes(path) - for (const route of routes) { - const to: string = route.fullPath - if (to.includes('$')) { - router.buildLocation({ to, params }) - } + for (const path of scenario.paths) { + router.getMatchedRoutes(path) + } + if (primeLinks) { + const paths = scenario.misses + ? scenario.paths + .slice(0, 32) + .map((path) => path.replace('item%20', 'cached%20')) + : scenario.paths + for (const path of paths) { + const [routes, params] = router.getMatchedRoutes(path) + for (const route of routes) { + const to: string = route.fullPath + if (to.includes('$')) { + router.buildLocation({ to, params }) } } } + } - let expected = 0 - for (const path of scenario.paths) { - const matches = router.matchRoutes(path, {}, options) - expect(matches.at(-1)?.routeId).toBe(parent.id) - for (const match of matches) { - expect(match.paramsError).toBeUndefined() - expected += - match.id.length + Object.keys(match._strictParams).length - } + let expected = 0 + for (const path of scenario.paths) { + const matches = router.matchRoutes(path, {}, options) + expect(matches.at(-1)?.routeId).toBe(parent.id) + for (const match of matches) { + expect(match.paramsError).toBeUndefined() + expected += + match.id.length + Object.keys(match._strictParams).length } - let checksum = 0 - bench( - scenario.name, - () => { - let length = 0 - for (const path of scenario.paths) { - for (const match of router.matchRoutes(path, {}, options)) { - length += - match.id.length + Object.keys(match._strictParams).length - } - } - checksum = length - }, - { - time: 1000, - warmupTime: 300, - throws: true, - teardown: () => expect(checksum).toBe(expected), - }, - ) } - }) - } + let checksum = 0 + bench( + scenario.name, + () => { + let length = 0 + for (const path of scenario.paths) { + for (const match of router.matchRoutes(path, {}, options)) { + length += + match.id.length + Object.keys(match._strictParams).length + } + } + checksum = length + }, + { + time: 1000, + warmupTime: 300, + throws: true, + teardown: () => expect(checksum).toBe(expected), + }, + ) + } + }) } } diff --git a/packages/router-core/tests/optional-path-params-clean.test.ts b/packages/router-core/tests/optional-path-params-clean.test.ts index 2e2db43496..36ed96f227 100644 --- a/packages/router-core/tests/optional-path-params-clean.test.ts +++ b/packages/router-core/tests/optional-path-params-clean.test.ts @@ -1,6 +1,6 @@ import { describe, expect, it } from 'vitest' +import { interpolatePath } from '../src/path' import { - interpolateTestPath as interpolatePath, parseTestPathname as parsePathname, processTestRouteTree as processRouteTree, } from './routerTestUtils' @@ -9,6 +9,7 @@ import { SEGMENT_TYPE_PATHNAME, SEGMENT_TYPE_WILDCARD, findSingleMatch, + parseSegments, } from '../src/new-process-route-tree' describe('Optional Path Parameters - Clean Comprehensive Tests', () => { @@ -51,44 +52,40 @@ describe('Optional Path Parameters - Clean Comprehensive Tests', () => { describe('interpolatePath', () => { it('should interpolate optional dynamic params when present', () => { - const result = interpolatePath('/posts/{-$category}', { - category: 'tech', - }) - expect(result).toBe('/posts/tech') + const path = '/posts/{-$category}' + const segments = parseSegments(false, { fullPath: path }, 0) + expect(interpolatePath(path, segments, { category: 'tech' })).toBe( + '/posts/tech', + ) }) it('should omit optional dynamic params when missing', () => { - const result = interpolatePath('/posts/{-$category}', {}) - expect(result).toBe('/posts') + const path = '/posts/{-$category}' + const segments = parseSegments(false, { fullPath: path }, 0) + expect(interpolatePath(path, segments, {})).toBe('/posts') }) it('should handle multiple optional dynamic params', () => { - const result1 = interpolatePath('/posts/{-$category}/{-$slug}', { - category: 'tech', - slug: 'hello', - }) - expect(result1).toBe('/posts/tech/hello') - - const result2 = interpolatePath('/posts/{-$category}/{-$slug}', { - category: 'tech', - }) - expect(result2).toBe('/posts/tech') - - const result3 = interpolatePath('/posts/{-$category}/{-$slug}', {}) - expect(result3).toBe('/posts') + const path = '/posts/{-$category}/{-$slug}' + const segments = parseSegments(false, { fullPath: path }, 0) + expect( + interpolatePath(path, segments, { category: 'tech', slug: 'hello' }), + ).toBe('/posts/tech/hello') + expect(interpolatePath(path, segments, { category: 'tech' })).toBe( + '/posts/tech', + ) + expect(interpolatePath(path, segments, {})).toBe('/posts') }) it('should handle mixed required and optional dynamic params', () => { - const result = interpolatePath('/posts/{-$category}/user/$id', { - category: 'tech', - id: '123', - }) - expect(result).toBe('/posts/tech/user/123') - - const result2 = interpolatePath('/posts/{-$category}/user/$id', { - id: '123', - }) - expect(result2).toBe('/posts/user/123') + const path = '/posts/{-$category}/user/$id' + const segments = parseSegments(false, { fullPath: path }, 0) + expect( + interpolatePath(path, segments, { category: 'tech', id: '123' }), + ).toBe('/posts/tech/user/123') + expect(interpolatePath(path, segments, { id: '123' })).toBe( + '/posts/user/123', + ) }) }) @@ -161,41 +158,46 @@ describe('Optional Path Parameters - Clean Comprehensive Tests', () => { describe('Edge Cases', () => { it('should handle optional params with wildcards', () => { - const result = interpolatePath('/docs/{-$version}/$', { - version: 'v1', - _splat: 'guide/intro', - }) - expect(result).toBe('/docs/v1/guide/intro') - - const result2 = interpolatePath('/docs/{-$version}/$', { - _splat: 'guide/intro', - }) - expect(result2).toBe('/docs/guide/intro') + const path = '/docs/{-$version}/$' + const segments = parseSegments(false, { fullPath: path }, 0) + expect( + interpolatePath(path, segments, { + version: 'v1', + _splat: 'guide/intro', + }), + ).toBe('/docs/v1/guide/intro') + expect(interpolatePath(path, segments, { _splat: 'guide/intro' })).toBe( + '/docs/guide/intro', + ) }) it('should work with complex patterns', () => { const pattern = '/app/{-$env}/api/{-$version}/users/$id/{-$tab}' + const segments = parseSegments(false, { fullPath: pattern }, 0) // All params provided - const result1 = interpolatePath(pattern, { - env: 'prod', - version: 'v2', - id: '123', - tab: 'settings', - }) - expect(result1).toBe('/app/prod/api/v2/users/123/settings') + expect( + interpolatePath(pattern, segments, { + env: 'prod', + version: 'v2', + id: '123', + tab: 'settings', + }), + ).toBe('/app/prod/api/v2/users/123/settings') // Only required param - const result2 = interpolatePath(pattern, { id: '123' }) - expect(result2).toBe('/app/api/users/123') + expect(interpolatePath(pattern, segments, { id: '123' })).toBe( + '/app/api/users/123', + ) // Mix of optional and required - const result3 = interpolatePath(pattern, { - env: 'dev', - id: '456', - tab: 'profile', - }) - expect(result3).toBe('/app/dev/api/users/456/profile') + expect( + interpolatePath(pattern, segments, { + env: 'dev', + id: '456', + tab: 'profile', + }), + ).toBe('/app/dev/api/users/456/profile') }) }) }) diff --git a/packages/router-core/tests/optional-path-params.test.ts b/packages/router-core/tests/optional-path-params.test.ts index d9f669f7ea..4d3c2ee76e 100644 --- a/packages/router-core/tests/optional-path-params.test.ts +++ b/packages/router-core/tests/optional-path-params.test.ts @@ -1,6 +1,6 @@ import { describe, expect, it } from 'vitest' +import { interpolatePath } from '../src/path' import { - interpolateTestPath as interpolatePath, parseTestPathname as parsePathname, processTestRouteTree as processRouteTree, } from './routerTestUtils' @@ -10,6 +10,7 @@ import { SEGMENT_TYPE_PATHNAME, SEGMENT_TYPE_WILDCARD, findSingleMatch, + parseSegments, } from '../src/new-process-route-tree' import type { SegmentKind } from '../src/new-process-route-tree' @@ -343,7 +344,8 @@ describe('Optional Path Parameters', () => { result: '/posts/42', }, ])('$name', ({ path, params, result }) => { - expect(interpolatePath(path, params)).toBe(result) + const segments = parseSegments(false, { fullPath: path }, 0) + expect(interpolatePath(path, segments, params)).toBe(result) }) }) @@ -472,8 +474,9 @@ describe('Optional Path Parameters', () => { it('should handle optional parameters with validation', () => { // This test will be expanded when we implement params.parse for optional params const path = '/posts/{-$category}' + const segments = parseSegments(false, { fullPath: path }, 0) const params = { category: 'tech' } - expect(interpolatePath(path, params)).toBe('/posts/tech') + expect(interpolatePath(path, segments, params)).toBe('/posts/tech') }) it('should handle multiple consecutive optional parameters correctly', () => { diff --git a/packages/router-core/tests/original-param-names.test.ts b/packages/router-core/tests/original-param-names.test.ts index e94e0ff412..aca4c61ae4 100644 --- a/packages/router-core/tests/original-param-names.test.ts +++ b/packages/router-core/tests/original-param-names.test.ts @@ -13,8 +13,12 @@ test('keeps original names when different templates share a trie prefix', () => isRoot: true, fullPath: '/', children: [ - { id: '/first', path: '$id/first', fullPath: '/$id/first' }, - { id: '/second', path: '{$name}/second', fullPath: '/{$name}/second' }, + { id: '/$id/first', path: '$id/first', fullPath: '/$id/first' }, + { + id: '/{$name}/second', + path: '{$name}/second', + fullPath: '/{$name}/second', + }, ], }).processedTree expect(findRouteMatch('/one/first', tree)?.rawParams).toEqual({ id: 'one' }) @@ -23,20 +27,20 @@ test('keeps original names when different templates share a trie prefix', () => }) }) -test('keeps a terminal route names when a later alias adds a parser', () => { +test("keeps a terminal route's names when a later alias adds a parser", () => { const tree = processRouteTree({ id: '__root__', isRoot: true, fullPath: '/', children: [ - { id: '/first', path: '$id/detail', fullPath: '/$id/detail' }, + { id: '/$id/detail', path: '$id/detail', fullPath: '/$id/detail' }, { - id: '/alias', + id: '/$name', path: '$name', fullPath: '/$name', children: [ { - id: '/alias/detail', + id: '/$name/detail', path: 'detail', fullPath: '/$name/detail', options: { @@ -48,7 +52,7 @@ test('keeps a terminal route names when a later alias adds a parser', () => { ], }).processedTree const match = findRouteMatch('/abc/detail', tree) - expect(match?.route.id).toBe('/first') + expect(match?.route.id).toBe('/$id/detail') expect(match?.rawParams).toEqual({ id: 'abc' }) }) @@ -76,7 +80,7 @@ test('resumes through a route-less parse gate with skipped optional names', () = }, children: [ { - id: '/leaf', + id: '/$id/_layout/{-$lang}/$slug', path: '{-$lang}/$slug', fullPath: '/$id/{-$lang}/$slug', }, diff --git a/packages/router-core/tests/path-decoder.bench.ts b/packages/router-core/tests/path-decoder.bench.ts index 05971c30f6..23060b33be 100644 --- a/packages/router-core/tests/path-decoder.bench.ts +++ b/packages/router-core/tests/path-decoder.bench.ts @@ -3,61 +3,59 @@ import { createMemoryHistory } from '@tanstack/history' import { BaseRootRoute, BaseRoute } from '../src' import { createTestRouter } from './routerTestUtils' -if (process.env.TSR_LINK_PERF === '1') { - describe.each(['stable', 'new-router', 'no-update'] as const)( - 'path decoder: %s configuration', - (mode) => { - const makeRouter = (allowed: ReadonlyArray<'@' | '+'>) => { - const root = new BaseRootRoute({}) - const item = new BaseRoute({ - getParentRoute: () => root, - path: '/items/$id', - }) - const router = createTestRouter({ - routeTree: root.addChildren([item]), - history: createMemoryHistory({ initialEntries: ['/'] }), - pathParamsAllowedCharacters: allowed, - scrollRestoration: false, +describe.each(['stable', 'new-router', 'no-update'] as const)( + 'path decoder: %s configuration', + (mode) => { + const makeRouter = (allowed: ReadonlyArray<'@' | '+'>) => { + const root = new BaseRootRoute({}) + const item = new BaseRoute({ + getParentRoute: () => root, + path: '/items/$id', + }) + const router = createTestRouter({ + routeTree: root.addChildren([item]), + history: createMemoryHistory({ initialEntries: ['/'] }), + pathParamsAllowedCharacters: allowed, + scrollRestoration: false, + }) + router.history.destroy() + return router + } + let router = makeRouter(['@']) + const inputs = Array.from({ length: 200 }, (_, index) => ({ + to: '/items/$id', + params: { id: `item-${index % 40}@+` }, + })) + let count = 0 + let lastHref = '' + const run = () => { + count++ + if (mode === 'new-router') { + router = makeRouter(count % 2 === 0 ? ['+'] : ['@']) + } else if (mode === 'stable') { + router.update({ + ...router.options, + context: { count }, }) - router.history.destroy() - return router } - let router = makeRouter(['@']) - const inputs = Array.from({ length: 200 }, (_, index) => ({ - to: '/items/$id', - params: { id: `item-${index % 40}@+` }, - })) - let count = 0 - let lastHref = '' - const run = () => { - count++ - if (mode === 'new-router') { - router = makeRouter(count % 2 === 0 ? ['+'] : ['@']) - } else if (mode === 'stable') { - router.update({ - ...router.options, - context: { count }, - }) - } - for (const input of inputs) { - lastHref = router.buildLocation(input).href - } + for (const input of inputs) { + lastHref = router.buildLocation(input).href } - const verify = () => { - expect(lastHref).toBe( - mode === 'new-router' && count % 2 === 0 - ? '/items/item-39%40+' - : '/items/item-39@%2B', - ) - } - run() - verify() - bench('update and build 200 locations', run, { - time: 1500, - warmupTime: 500, - throws: true, - teardown: verify, - }) - }, - ) -} + } + const verify = () => { + expect(lastHref).toBe( + mode === 'new-router' && count % 2 === 0 + ? '/items/item-39%40+' + : '/items/item-39@%2B', + ) + } + run() + verify() + bench('update and build 200 locations', run, { + time: 1500, + warmupTime: 500, + throws: true, + teardown: verify, + }) + }, +) diff --git a/packages/router-core/tests/path-interpolation.bench.ts b/packages/router-core/tests/path-interpolation.bench.ts index 0f6840752d..53498b5198 100644 --- a/packages/router-core/tests/path-interpolation.bench.ts +++ b/packages/router-core/tests/path-interpolation.bench.ts @@ -7,7 +7,7 @@ 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 { createTestRouter } from './routerTestUtils' import type { AnyRoute } from '../src' import type { PathInterpolationTestOptions } from './routerTestUtils' @@ -146,6 +146,15 @@ describe.each(scenarios)('$name', ({ inputs, register = true }) => { scrollRestoration: false, }) router.history.destroy() + // Reference values come from a fresh standalone parse, independent of any + // route-owned segments under test. + const referencePathname = (input: PathInterpolationTestOptions) => + interpolatePath( + input.path, + parseSegments(false, { fullPath: input.path }, 0), + input.params, + input.decoder, + ) // Mirrors how buildLocation turns a template into a canonical pathname. const canonicalPathname = ( path: string, @@ -168,27 +177,11 @@ describe.each(scenarios)('$name', ({ inputs, register = true }) => { path: input.path, params: input.params, route: register ? routes.get(input.path) : undefined, - expected: decodePath( - interpolateTestPath( - input.path, - input.params, - input.decoder, - undefined, - undefined, - ), - ), + expected: decodePath(referencePathname(input)), })) let checksum = 0 const expected = inputs.reduce( - (sum, input) => - sum + - interpolateTestPath( - input.path, - input.params, - input.decoder, - undefined, - undefined, - ).length, + (sum, input) => sum + referencePathname(input).length, 0, ) const cachedExpected = calls.reduce((sum, call) => { diff --git a/packages/router-core/tests/path.test.ts b/packages/router-core/tests/path.test.ts index 7d6eed9de7..8dc964bcd6 100644 --- a/packages/router-core/tests/path.test.ts +++ b/packages/router-core/tests/path.test.ts @@ -2,6 +2,8 @@ import { afterEach, describe, expect, it, vi } from 'vitest' import { compileDecodeCharMap, exactPathTest, + hasMissingPathParams, + interpolatePath, removeTrailingSlash, resolvePath, trimPathLeft, @@ -11,11 +13,12 @@ import { SEGMENT_TYPE_PATHNAME, SEGMENT_TYPE_WILDCARD, findSingleMatch, + getParamNames, + parseSegments, } from '../src/new-process-route-tree' import { createSieveCache } from '../src/sieve-cache' import { createTestPathInterpolator as createPathInterpolator, - interpolateTestPath as interpolatePath, parseTestPathname as parsePathname, processTestRouteTree as processRouteTree, } from './routerTestUtils' @@ -81,6 +84,12 @@ describe.each([false, true])( expected: '/files/prefixa/bsuffix', }, { path: '/files/$', params: { _splat: '' }, expected: '/files' }, + { path: '/files/$', params: { _splat: 0 }, expected: '/files/0' }, + { + path: '/files/prefix{$}.txt', + params: { _splat: 0 }, + expected: '/files/prefix0.txt', + }, { path: '/$id/$id/', params: { id: '123' }, @@ -91,9 +100,8 @@ describe.each([false, true])( ({ path, params, expected, normalized }) => { const interpolate = createPathInterpolator({ isServer: server }) const options = { path, params, server } - expect( - interpolatePath(path, params, undefined, undefined, undefined), - ).toBe(expected) + const segments = parseSegments(false, { fullPath: path }, 0) + expect(interpolatePath(path, segments, params)).toBe(expected) expect(interpolate(options)).toBe(normalized ?? expected) expect(interpolate({ ...options, params: { ...params } })).toBe( normalized ?? expected, @@ -231,18 +239,18 @@ describe.each([false, true])( const usedParams: Record = Object.create(null) interpolatePath( '/posts/{-$category}', + parseSegments(false, { fullPath: '/posts/{-$category}' }, 0), {}, undefined, usedParams, - undefined, ) expect(usedParams).toEqual({}) interpolatePath( '/files/$', + parseSegments(false, { fullPath: '/files/$' }, 0), { _splat: 'docs/guide' }, undefined, usedParams, - undefined, ) expect(usedParams).toEqual({ _splat: 'docs/guide', @@ -278,25 +286,15 @@ describe.each([false, true])( 'keeps mixed segment metadata in one pass for $params', ({ params, path, used, missing }) => { const template = '/root/prefix{$id}suffix/{-$language}/files/{$}.txt' + const segments = parseSegments(false, { fullPath: template }, 0) const usedParams: Record = Object.create(null) - const keys: Array = [] - const metadata = { isMissingParams: false } expect( - interpolatePath( - template, - params, - undefined, - usedParams, - keys, - metadata, - ), + interpolatePath(template, segments, params, undefined, usedParams), ).toBe(path) - expect(keys).toEqual(['id', 'language', '_splat']) + expect(getParamNames(segments)).toEqual(['id', 'language', '_splat']) expect(usedParams).toEqual(used) - expect(metadata.isMissingParams).toBe(missing) - expect( - interpolatePath(template, params, undefined, undefined, undefined), - ).toBe(path) + expect(hasMissingPathParams(segments, params)).toBe(missing) + expect(interpolatePath(template, segments, params)).toBe(path) }, ) @@ -339,20 +337,13 @@ describe.each([false, true])( ])( 'preserves missing-param metadata for $path with $params', ({ path, params, pathname, usedParams, missing }) => { + const segments = parseSegments(false, { fullPath: path }, 0) const collectedParams: Record = Object.create(null) - const metadata = { isMissingParams: false } expect( - interpolatePath( - path, - params, - undefined, - collectedParams, - undefined, - metadata, - ), + interpolatePath(path, segments, params, undefined, collectedParams), ).toBe(pathname) expect(collectedParams).toEqual(usedParams) - expect(metadata.isMissingParams).toBe(missing) + expect(hasMissingPathParams(segments, params)).toBe(missing) }, ) @@ -662,465 +653,447 @@ describe('resolvePath', () => { }) }) -describe.each([{ server: true }, { server: false }])( - 'interpolatePath (server: $server)', - ({ server }) => { - it.each(['value', '', undefined])( - 'stops interpolation at a bare splat with value %s', - (_splat) => { - const params = { - _splat, - get ignored() { - throw new Error('A bare splat consumes the rest of the template') - }, - } - const used: Record = Object.create(null) - const keys: Array = [] - expect( - interpolatePath('/files/$/$ignored', params, undefined, used, keys), - ).toBe(_splat ? '/files/value' : '/files') - expect(keys).toEqual(['_splat']) - expect(used).toEqual({ _splat, '*': _splat }) - }, - ) - - it.each([ - { path: '/', expected: '/' }, - { path: '/about/', expected: '/about/' }, - ])('preserves static paths: $path', ({ path, expected }) => { +describe('interpolatePath', () => { + it.each(['value', '', undefined])( + 'stops interpolation at a bare splat with value %s', + (_splat) => { const params = { - get unused() { - throw new Error('Static paths must not read params') + _splat, + get ignored() { + throw new Error('A bare splat consumes the rest of the template') }, } - const decoder = vi.fn() - const usedParams: Record = Object.create(null) - const keys: Array = [] - const metadata = { isMissingParams: false } + const segments = parseSegments( + false, + { fullPath: '/files/$/$ignored' }, + 0, + ) + const used: Record = Object.create(null) expect( - interpolatePath(path, params, decoder, usedParams, keys, metadata), - ).toBe(expected) - expect(usedParams).toEqual({}) - expect(keys).toEqual([]) - expect(metadata.isMissingParams).toBe(false) - expect(decoder).not.toHaveBeenCalled() - }) + interpolatePath('/files/$/$ignored', segments, params, undefined, used), + ).toBe(_splat ? '/files/value' : '/files') + expect(getParamNames(segments)).toEqual(['_splat']) + expect(used).toEqual({ _splat, '*': _splat }) + }, + ) + + it.each([ + { path: '/', expected: '/' }, + { path: '/about/', expected: '/about/' }, + ])('preserves static paths: $path', ({ path, expected }) => { + const params = { + get unused() { + throw new Error('Static paths must not read params') + }, + } + const segments = parseSegments(false, { fullPath: path }, 0) + const decoder = vi.fn() + const usedParams: Record = Object.create(null) + expect(interpolatePath(path, segments, params, decoder, usedParams)).toBe( + expected, + ) + expect(usedParams).toEqual({}) + expect(getParamNames(segments)).toEqual([]) + expect(hasMissingPathParams(segments, params)).toBe(false) + expect(decoder).not.toHaveBeenCalled() + }) + + it.each([ + { + path: '/users/$id', + params: {}, + expected: '/users/undefined', + missing: true, + }, + { + path: '/users/$id', + params: { id: 'one' }, + expected: '/users/one', + missing: false, + }, + { + path: '/posts/{-$category}', + params: {}, + expected: '/posts', + missing: false, + }, + { path: '/files/$', params: {}, expected: '/files', missing: true }, + ])( + 'collects only missing status for $path', + ({ path, params, expected, missing }) => { + const segments = parseSegments(false, { fullPath: path }, 0) + expect(interpolatePath(path, segments, params)).toBe(expected) + expect(hasMissingPathParams(segments, params)).toBe(missing) + }, + ) + describe('regular usage', () => { it.each([ { + name: 'should interpolate the path', path: '/users/$id', - params: {}, - expected: '/users/undefined', - missing: true, + params: { id: '123' }, + result: '/users/123', }, { + name: 'should interpolate the path', path: '/users/$id', - params: { id: 'one' }, - expected: '/users/one', - missing: false, + params: { id: '123_' }, + result: '/users/123_', }, { - path: '/posts/{-$category}', + name: 'should interpolate the path with multiple params', + path: '/users/$id/$name', + params: { id: '123', name: 'tanner' }, + result: '/users/123/tanner', + }, + { + name: 'should interpolate the path with multiple params', + path: '/users/$id/$name', + params: { id: '123_', name: 'tanner' }, + result: '/users/123_/tanner', + }, + { + name: 'should interpolate the path with extra params', + path: '/users/$id', + params: { id: '123', name: 'tanner' }, + result: '/users/123', + }, + { + name: 'should interpolate the path with missing params', + path: '/users/$id/$name', + params: { id: '123' }, + result: '/users/123/undefined', + }, + { + name: 'should interpolate the path with missing params and extra params', + path: '/users/$id', + params: { name: 'john' }, + result: '/users/undefined', + }, + { + name: 'should interpolate the path with the param being a number', + path: '/users/$id', + params: { id: 123 }, + result: '/users/123', + }, + { + name: 'should interpolate the path with the param being a falsey number', + path: '/users/$id', + params: { id: 0 }, + result: '/users/0', + }, + { + name: 'should interpolate the path with the splat param being a falsey number', + path: '/users/$', + params: { _splat: 0 }, + result: '/users/0', + }, + { + name: 'should interpolate the path with URI component encoding', + path: '/users/$id', + params: { id: '?#@john+smith' }, + result: '/users/%3F%23%40john%2Bsmith', + }, + { + name: 'should interpolate the path without URI encoding characters in decodeCharMap', + path: '/users/$id', + params: { id: '?#@john+smith' }, + result: '/users/%3F%23@john+smith', + decoder: compileDecodeCharMap(['@', '+']), + }, + { + name: 'should interpolate the path with the splat param at the end', + path: '/users/$', + params: { _splat: '123' }, + result: '/users/123', + }, + { + name: 'should interpolate the path with a single named path param and the splat param at the end', + path: '/users/$username/$', + params: { username: 'seancassiere', _splat: '123' }, + result: '/users/seancassiere/123', + }, + { + name: 'should interpolate the path with 2 named path params with the splat param at the end', + path: '/users/$username/$id/$', + params: { username: 'seancassiere', id: '123', _splat: '456' }, + result: '/users/seancassiere/123/456', + }, + { + name: 'should interpolate the path with multiple named path params with the splat param at the end', + path: '/$username/settings/$repo/$id/$', + params: { + username: 'sean-cassiere', + repo: 'my-repo', + id: '123', + _splat: '456', + }, + result: '/sean-cassiere/settings/my-repo/123/456', + }, + { + name: 'should interpolate the path with the splat param containing slashes', + path: '/users/$', + params: { _splat: 'sean/cassiere' }, + result: '/users/sean/cassiere', + }, + ])('$name', ({ path, params, decoder, result }) => { + const segments = parseSegments(false, { fullPath: path }, 0) + expect(interpolatePath(path, segments, params, decoder)).toBe(result) + }) + }) + + describe('preserve trailing slash', () => { + it.each([ + { + path: '/', params: {}, - expected: '/posts', - missing: false, + result: '/', + }, + { + path: '/a/b/', + params: {}, + result: '/a/b/', + }, + { + path: '/a/$id/', + params: { id: '123' }, + result: '/a/123/', + }, + { + path: '/a/{-$id}/', + params: { id: '123' }, + result: '/a/123/', }, - { path: '/files/$', params: {}, expected: '/files', missing: true }, ])( - 'collects only missing status for $path', - ({ path, params, expected, missing }) => { - const metadata = { isMissingParams: false } - expect( - interpolatePath( - path, - params, - undefined, - undefined, - undefined, - metadata, - ), - ).toBe(expected) - expect(metadata.isMissingParams).toBe(missing) + 'should preserve trailing slash for $path', + ({ path, params, result }) => { + const segments = parseSegments(false, { fullPath: path }, 0) + expect(interpolatePath(path, segments, params)).toBe(result) }, ) + }) - describe('regular usage', () => { - it.each([ - { - name: 'should interpolate the path', - path: '/users/$id', - params: { id: '123' }, - result: '/users/123', - }, - { - name: 'should interpolate the path', - path: '/users/$id', - params: { id: '123_' }, - result: '/users/123_', - }, - { - name: 'should interpolate the path with multiple params', - path: '/users/$id/$name', - params: { id: '123', name: 'tanner' }, - result: '/users/123/tanner', - }, - { - name: 'should interpolate the path with multiple params', - path: '/users/$id/$name', - params: { id: '123_', name: 'tanner' }, - result: '/users/123_/tanner', - }, - { - name: 'should interpolate the path with extra params', - path: '/users/$id', - params: { id: '123', name: 'tanner' }, - result: '/users/123', - }, - { - name: 'should interpolate the path with missing params', - path: '/users/$id/$name', - params: { id: '123' }, - result: '/users/123/undefined', - }, - { - name: 'should interpolate the path with missing params and extra params', - path: '/users/$id', - params: { name: 'john' }, - result: '/users/undefined', - }, - { - name: 'should interpolate the path with the param being a number', - path: '/users/$id', - params: { id: 123 }, - result: '/users/123', - }, - { - name: 'should interpolate the path with the param being a falsey number', - path: '/users/$id', - params: { id: 0 }, - result: '/users/0', - }, - { - name: 'should interpolate the path with URI component encoding', - path: '/users/$id', - params: { id: '?#@john+smith' }, - result: '/users/%3F%23%40john%2Bsmith', - }, - { - name: 'should interpolate the path without URI encoding characters in decodeCharMap', - path: '/users/$id', - params: { id: '?#@john+smith' }, - result: '/users/%3F%23@john+smith', - decoder: compileDecodeCharMap(['@', '+']), - }, - { - name: 'should interpolate the path with the splat param at the end', - path: '/users/$', - params: { _splat: '123' }, - result: '/users/123', - }, - { - name: 'should interpolate the path with a single named path param and the splat param at the end', - path: '/users/$username/$', - params: { username: 'seancassiere', _splat: '123' }, - result: '/users/seancassiere/123', - }, - { - name: 'should interpolate the path with 2 named path params with the splat param at the end', - path: '/users/$username/$id/$', - params: { username: 'seancassiere', id: '123', _splat: '456' }, - result: '/users/seancassiere/123/456', - }, - { - name: 'should interpolate the path with multiple named path params with the splat param at the end', - path: '/$username/settings/$repo/$id/$', - params: { - username: 'sean-cassiere', - repo: 'my-repo', - id: '123', - _splat: '456', - }, - result: '/sean-cassiere/settings/my-repo/123/456', - }, - { - name: 'should interpolate the path with the splat param containing slashes', - path: '/users/$', - params: { _splat: 'sean/cassiere' }, - result: '/users/sean/cassiere', - }, - ])('$name', ({ path, params, decoder, result }) => { - expect( - interpolatePath(path, params, decoder, undefined, undefined), - ).toBe(result) - }) - }) - - describe('preserve trailing slash', () => { - it.each([ - { - path: '/', - params: {}, - result: '/', - }, - { - path: '/a/b/', - params: {}, - result: '/a/b/', - }, - { - path: '/a/$id/', - params: { id: '123' }, - result: '/a/123/', - }, - { - path: '/a/{-$id}/', - params: { id: '123' }, - result: '/a/123/', - }, - ])( - 'should preserve trailing slash for $path', - ({ path, params, result }) => { - expect( - interpolatePath(path, params, undefined, undefined, undefined), - ).toBe(result) - }, - ) + describe('wildcard (prefix + suffix)', () => { + it.each([ + { + name: 'regular', + to: '/$', + params: { _splat: 'bar/foo/me' }, + result: '/bar/foo/me', + }, + { + name: 'regular curly braces', + to: '/{$}', + params: { _splat: 'bar/foo/me' }, + result: '/bar/foo/me', + }, + { + name: 'with prefix', + to: '/prefix{$}', + params: { _splat: 'bar' }, + result: '/prefixbar', + }, + { + name: 'with suffix', + to: '/{$}-suffix', + params: { _splat: 'bar' }, + result: '/bar-suffix', + }, + { + name: 'with prefix + suffix', + to: '/prefix{$}-suffix', + params: { _splat: 'bar' }, + result: '/prefixbar-suffix', + }, + ])('$name', ({ to, params, result }) => { + const segments = parseSegments(false, { fullPath: to }, 0) + expect(interpolatePath(to, segments, params)).toBe(result) }) + }) - describe('wildcard (prefix + suffix)', () => { - it.each([ - { - name: 'regular', - to: '/$', - params: { _splat: 'bar/foo/me' }, - result: '/bar/foo/me', - }, - { - name: 'regular curly braces', - to: '/{$}', - params: { _splat: 'bar/foo/me' }, - result: '/bar/foo/me', - }, - { - name: 'with prefix', - to: '/prefix{$}', - params: { _splat: 'bar' }, - result: '/prefixbar', - }, - { - name: 'with suffix', - to: '/{$}-suffix', - params: { _splat: 'bar' }, - result: '/bar-suffix', - }, - { - name: 'with prefix + suffix', - to: '/prefix{$}-suffix', - params: { _splat: 'bar' }, - result: '/prefixbar-suffix', - }, - ])('$name', ({ to, params, result }) => { - expect( - interpolatePath(to, params, undefined, undefined, undefined), - ).toBe(result) - }) + describe('splat params with special characters', () => { + it.each([ + { + name: 'should encode spaces in splat param', + path: '/$', + params: { _splat: 'file name.pdf' }, + result: '/file%20name.pdf', + }, + { + name: 'should preserve parentheses in splat param (RFC 3986 unreserved)', + path: '/$', + params: { _splat: 'file(1).pdf' }, + result: '/file(1).pdf', + }, + { + name: 'should encode brackets in splat param', + path: '/$', + params: { _splat: 'file[1].pdf' }, + result: '/file%5B1%5D.pdf', + }, + { + name: 'should encode spaces in nested splat param paths', + path: '/$', + params: { _splat: 'folder/sub folder/file name.pdf' }, + result: '/folder/sub%20folder/file%20name.pdf', + }, + { + name: 'should encode spaces and brackets but preserve parentheses', + path: '/$', + params: { _splat: 'docs/file (copy) [2].pdf' }, + result: '/docs/file%20(copy)%20%5B2%5D.pdf', + }, + { + name: 'should encode hash in splat param', + path: '/$', + params: { _splat: 'page#section' }, + result: '/page%23section', + }, + { + name: 'should handle splat param with prefix and special characters', + path: '/files/prefix{$}', + params: { _splat: 'my file.pdf' }, + result: '/files/prefixmy%20file.pdf', + }, + { + name: 'should encode plus signs in splat param', + path: '/$', + params: { _splat: 'file+name.pdf' }, + result: '/file%2Bname.pdf', + }, + { + name: 'should encode equals signs in splat param', + path: '/$', + params: { _splat: 'query=value' }, + result: '/query%3Dvalue', + }, + ])('$name', ({ path, params, result }) => { + const segments = parseSegments(false, { fullPath: path }, 0) + expect(interpolatePath(path, segments, params)).toBe(result) }) + }) - describe('splat params with special characters', () => { - it.each([ - { - name: 'should encode spaces in splat param', - path: '/$', - params: { _splat: 'file name.pdf' }, - result: '/file%20name.pdf', - }, - { - name: 'should preserve parentheses in splat param (RFC 3986 unreserved)', - path: '/$', - params: { _splat: 'file(1).pdf' }, - result: '/file(1).pdf', - }, - { - name: 'should encode brackets in splat param', - path: '/$', - params: { _splat: 'file[1].pdf' }, - result: '/file%5B1%5D.pdf', - }, - { - name: 'should encode spaces in nested splat param paths', - path: '/$', - params: { _splat: 'folder/sub folder/file name.pdf' }, - result: '/folder/sub%20folder/file%20name.pdf', - }, - { - name: 'should encode spaces and brackets but preserve parentheses', - path: '/$', - params: { _splat: 'docs/file (copy) [2].pdf' }, - result: '/docs/file%20(copy)%20%5B2%5D.pdf', - }, - { - name: 'should encode hash in splat param', - path: '/$', - params: { _splat: 'page#section' }, - result: '/page%23section', - }, - { - name: 'should handle splat param with prefix and special characters', - path: '/files/prefix{$}', - params: { _splat: 'my file.pdf' }, - result: '/files/prefixmy%20file.pdf', - }, - { - name: 'should encode plus signs in splat param', - path: '/$', - params: { _splat: 'file+name.pdf' }, - result: '/file%2Bname.pdf', - }, - { - name: 'should encode equals signs in splat param', - path: '/$', - params: { _splat: 'query=value' }, - result: '/query%3Dvalue', - }, - ])('$name', ({ path, params, result }) => { - expect( - interpolatePath(path, params, undefined, undefined, undefined), - ).toBe(result) - }) + describe('named params (prefix + suffix)', () => { + it.each([ + { + name: 'regular', + to: '/$foo', + params: { foo: 'bar' }, + result: '/bar', + }, + { + name: 'regular curly braces', + to: '/{$foo}', + params: { foo: 'bar' }, + result: '/bar', + }, + { + name: 'with prefix', + to: '/prefix{$bar}', + params: { bar: 'baz' }, + result: '/prefixbaz', + }, + { + name: 'with suffix', + to: '/{$foo}.suffix', + params: { foo: 'bar' }, + result: '/bar.suffix', + }, + { + name: 'with suffix', + to: '/{$foo}.suffix', + params: { foo: 'bar_' }, + result: '/bar_.suffix', + }, + { + name: 'with prefix and suffix', + to: '/prefix{$param}.suffix', + params: { param: 'foobar' }, + result: '/prefixfoobar.suffix', + }, + ])('$name', ({ to, params, result }) => { + const segments = parseSegments(false, { fullPath: to }, 0) + expect(interpolatePath(to, segments, params)).toBe(result) }) + }) - describe('named params (prefix + suffix)', () => { - it.each([ - { - name: 'regular', - to: '/$foo', - params: { foo: 'bar' }, - result: '/bar', - }, - { - name: 'regular curly braces', - to: '/{$foo}', - params: { foo: 'bar' }, - result: '/bar', - }, - { - name: 'with prefix', - to: '/prefix{$bar}', - params: { bar: 'baz' }, - result: '/prefixbaz', - }, - { - name: 'with suffix', - to: '/{$foo}.suffix', - params: { foo: 'bar' }, - result: '/bar.suffix', - }, - { - name: 'with suffix', - to: '/{$foo}.suffix', - params: { foo: 'bar_' }, - result: '/bar_.suffix', + describe('should handle missing _splat parameter for', () => { + it.each([ + { + name: 'basic splat route', + path: '/hello/$', + params: {}, + expectedResult: '/hello', + }, + { + name: 'splat route with prefix', + path: '/hello/prefix{$}', + params: {}, + expectedResult: '/hello/prefix', + }, + { + name: 'splat route with suffix', + path: '/hello/{$}suffix', + params: {}, + expectedResult: '/hello/suffix', + }, + { + name: 'splat route with prefix and suffix', + path: '/hello/prefix{$}suffix', + params: {}, + expectedResult: '/hello/prefixsuffix', + }, + { + name: 'splat route with empty splat', + path: '/hello/$', + params: { + _splat: '', }, - { - name: 'with prefix and suffix', - to: '/prefix{$param}.suffix', - params: { param: 'foobar' }, - result: '/prefixfoobar.suffix', + expectedResult: '/hello', + }, + { + name: 'splat route with undefined splat', + path: '/hello/$', + params: { + _splat: undefined, }, - ])('$name', ({ to, params, result }) => { - expect( - interpolatePath(to, params, undefined, undefined, undefined), - ).toBe(result) - }) + expectedResult: '/hello', + }, + ])('$name', ({ path, params, expectedResult }) => { + const segments = parseSegments(false, { fullPath: path }, 0) + expect(interpolatePath(path, segments, params)).toBe(expectedResult) + expect(hasMissingPathParams(segments, params)).toBe(true) }) + }) - describe('should handle missing _splat parameter for', () => { - it.each([ - { - name: 'basic splat route', - path: '/hello/$', - params: {}, - expectedResult: '/hello', - }, - { - name: 'splat route with prefix', - path: '/hello/prefix{$}', - params: {}, - expectedResult: '/hello/prefix', - }, - { - name: 'splat route with suffix', - path: '/hello/{$}suffix', - params: {}, - expectedResult: '/hello/suffix', - }, - { - name: 'splat route with prefix and suffix', - path: '/hello/prefix{$}suffix', - params: {}, - expectedResult: '/hello/prefixsuffix', - }, - { - name: 'splat route with empty splat', - path: '/hello/$', - params: { - _splat: '', - }, - expectedResult: '/hello', - }, - { - name: 'splat route with undefined splat', - path: '/hello/$', - params: { - _splat: undefined, - }, - expectedResult: '/hello', - }, - ])('$name', ({ path, params, expectedResult }) => { - const metadata = { isMissingParams: false } - const result = interpolatePath( - path, - params, - undefined, - undefined, - undefined, - metadata, + describe('resolvePath + interpolatePath', () => { + it.each(['never', 'preserve', 'always'] as const)( + 'trailing slash: %s', + (trailingSlash) => { + const tail = trailingSlash === 'always' ? '/' : '' + const defaultedFromPath = '/' + const fromPath = resolvePath({ + base: defaultedFromPath, + to: '.', + trailingSlash, + }) + const nextTo = resolvePath({ + base: fromPath, + to: '/splat/$', + trailingSlash, + }) + const nextParams = { _splat: '' } + const interpolatedNextTo = interpolatePath( + nextTo, + parseSegments(false, { fullPath: nextTo }, 0), + nextParams, ) - expect(result).toBe(expectedResult) - expect(metadata.isMissingParams).toBe(true) - }) - }) - - describe('resolvePath + interpolatePath', () => { - it.each(['never', 'preserve', 'always'] as const)( - 'trailing slash: %s', - (trailingSlash) => { - const tail = trailingSlash === 'always' ? '/' : '' - const defaultedFromPath = '/' - const fromPath = resolvePath({ - base: defaultedFromPath, - to: '.', - trailingSlash, - }) - const nextTo = resolvePath({ - base: fromPath, - to: '/splat/$', - trailingSlash, - }) - const nextParams = { _splat: '' } - const interpolatedNextTo = interpolatePath( - nextTo, - nextParams, - undefined, - undefined, - undefined, - ) - expect(interpolatedNextTo).toBe(`/splat${tail}`) - }, - ) - }) - }, -) + expect(interpolatedNextTo).toBe(`/splat${tail}`) + }, + ) + }) +}) describe('matchPathname', () => { const { processedTree } = processRouteTree({ diff --git a/packages/router-core/tests/route-interpolation.test.ts b/packages/router-core/tests/route-interpolation.test.ts index 3f8428dfd9..8743f8bc04 100644 --- a/packages/router-core/tests/route-interpolation.test.ts +++ b/packages/router-core/tests/route-interpolation.test.ts @@ -2,12 +2,13 @@ import { afterEach, describe, expect, test, vi } from 'vitest' import { createMemoryHistory } from '@tanstack/history' import { BaseRootRoute, BaseRoute } from '../src' import { + cleanPath, compileDecodeCharMap, hasMissingPathParams, interpolatePath, } from '../src/path' import * as routeTreeUtils from '../src/new-process-route-tree' -import { createTestRouter, interpolateTestPath } from './routerTestUtils' +import { createTestRouter } from './routerTestUtils' afterEach(() => vi.restoreAllMocks()) @@ -19,8 +20,10 @@ test.each([ { path: '/{-$id}', params: {}, missing: false }, { path: '/pre{-$id}suffix', params: { id: null }, missing: false }, { path: '/files/$', params: {}, missing: true }, + { path: '/files/$', params: { _splat: null }, missing: true }, { path: '/files/$', params: { _splat: '' }, missing: true }, - { path: '/files/{$}.txt', params: { _splat: 0 }, missing: true }, + { path: '/files/{$}.txt', params: { _splat: 0 }, missing: false }, + { path: '/files/{$}.txt', params: { _splat: false }, missing: false }, { path: '/files/$/ignored', params: { _splat: 'a/b' }, missing: false }, ])( 'preserves navigation availability for $path: $params', @@ -31,6 +34,11 @@ test.each([ ) test('encodes splats equivalently without interpreting literal encoded separators', () => { + const segments = routeTreeUtils.parseSegments( + false, + { fullPath: '/files/$' }, + 0, + ) for (const allowed of [[], ['@', '+'], ['%'], ['/']]) { const decoder = compileDecodeCharMap(allowed) for (const value of [ @@ -43,9 +51,9 @@ test('encodes splats equivalently without interpreting literal encoded separator .split('/') .map((part) => decoder(encodeURIComponent(part))) .join('/') - expect(interpolateTestPath('/files/$', { _splat: value }, decoder)).toBe( - `/files/${expected}`, - ) + expect( + interpolatePath('/files/$', segments, { _splat: value }, decoder), + ).toBe(`/files/${expected}`) } } }) @@ -77,6 +85,14 @@ describe.each([false, true])( scrollRestoration: false, }) history.destroy() + // The reference is the standalone parse buildLocation uses for templates + // without a processed route. + const template = routeTreeUtils.parseSegments( + false, + { fullPath: cleanPath(path) }, + 0, + ) + const decoder = compileDecodeCharMap(['@', '+']) const inputs: Array> = [ {}, { id: '', lang: null, _splat: '' }, @@ -87,19 +103,12 @@ describe.each([false, true])( for (const params of [...inputs, ...inputs]) { const expectedUsed = Object.create(null) const actualUsed = Object.create(null) - const expectedKeys: Array = [] - const actualKeys = (route._interpolation ?? []).flatMap((part) => - typeof part === 'string' ? [] : [part[1 /* key */]], - ) - const expectedMeta = { isMissingParams: false } - const decoder = compileDecodeCharMap(['@', '+']) - const expected = interpolateTestPath( + const expected = interpolatePath( path, + template, params, decoder, expectedUsed, - expectedKeys, - expectedMeta, ) expect( route._interpolation @@ -113,12 +122,16 @@ describe.each([false, true])( : path, ).toBe(expected) expect(actualUsed).toEqual(expectedUsed) - expect(actualKeys).toEqual(expectedKeys) + expect( + route._interpolation + ? routeTreeUtils.getParamNames(route._interpolation) + : [], + ).toEqual(routeTreeUtils.getParamNames(template)) expect( route._interpolation ? hasMissingPathParams(route._interpolation, params) : false, - ).toBe(expectedMeta.isMissingParams) + ).toBe(hasMissingPathParams(template, params)) } }) @@ -255,7 +268,13 @@ test.each([false, true])( const params = { _splat: 'a/b', lang: 'en' } expect( interpolatePath(child.fullPath, child._interpolation!, params), - ).toBe(interpolateTestPath(child.fullPath, params)) + ).toBe( + interpolatePath( + child.fullPath, + routeTreeUtils.parseSegments(false, { fullPath: child.fullPath }, 0), + params, + ), + ) } }, ) diff --git a/packages/router-core/tests/route-tree-construction.bench.ts b/packages/router-core/tests/route-tree-construction.bench.ts index 56324db9aa..6d564d9e84 100644 --- a/packages/router-core/tests/route-tree-construction.bench.ts +++ b/packages/router-core/tests/route-tree-construction.bench.ts @@ -40,61 +40,59 @@ function createTree(count: number, shape: Shape) { } // Keep first-use interpolation and object allocation out of the processing-only case. -if (process.env.TSR_LINK_PERF === '1') { - for (const count of [100, 1000, 10000]) { - describe.each(['static', 'dynamic', 'nested', 'mixed'])( - `route-tree construction (${count} routes, %s)`, - (shape) => { - let tree = createTree(count, shape) - let result = processRouteTree(tree) - const expectedPath = - shape === 'static' - ? '/static-0/about' - : shape === 'dynamic' - ? '/section-0/one' - : shape === 'nested' - ? '/orgs/group-0/one/items/section-0/two' - : '/mixed-0/one/items/two' - const verify = () => { - expect(Object.keys(result.routesById)).toHaveLength(count + 1) - expect( - findRouteMatch(expectedPath, result.processedTree), - ).not.toBeNull() - } - verify() +for (const count of [100, 1000, 10000]) { + describe.each(['static', 'dynamic', 'nested', 'mixed'])( + `route-tree construction (${count} routes, %s)`, + (shape) => { + let tree = createTree(count, shape) + let result = processRouteTree(tree) + const expectedPath = + shape === 'static' + ? '/static-0/about' + : shape === 'dynamic' + ? '/section-0/one' + : shape === 'nested' + ? '/orgs/group-0/one/items/section-0/two' + : '/mixed-0/one/items/two' + const verify = () => { + expect(Object.keys(result.routesById)).toHaveLength(count + 1) + expect( + findRouteMatch(expectedPath, result.processedTree), + ).not.toBeNull() + } + verify() - bench( - 'processRouteTree + route.init on fresh objects', - () => { - result = processRouteTree(tree) - }, - { - setup: (task) => { - // Vitest forwards Bench options, not per-iteration Task options. - task.opts.beforeEach = () => { - tree = createTree(count, shape) - } - }, - teardown: verify, - time: 1500, - warmupTime: 500, - throws: true, + bench( + 'processRouteTree + route.init on fresh objects', + () => { + result = processRouteTree(tree) + }, + { + setup: (task) => { + // Vitest forwards Bench options, not per-iteration Task options. + task.opts.beforeEach = () => { + tree = createTree(count, shape) + } }, - ) + teardown: verify, + time: 1500, + warmupTime: 500, + throws: true, + }, + ) - bench( - 'route objects + processRouteTree + route.init', - () => { - result = processRouteTree(createTree(count, shape)) - }, - { - teardown: verify, - time: 1500, - warmupTime: 500, - throws: true, - }, - ) - }, - ) - } + bench( + 'route objects + processRouteTree + route.init', + () => { + result = processRouteTree(createTree(count, shape)) + }, + { + teardown: verify, + time: 1500, + warmupTime: 500, + throws: true, + }, + ) + }, + ) } diff --git a/packages/router-core/tests/routerTestUtils.ts b/packages/router-core/tests/routerTestUtils.ts index 51a785819f..490eacdb33 100644 --- a/packages/router-core/tests/routerTestUtils.ts +++ b/packages/router-core/tests/routerTestUtils.ts @@ -8,14 +8,13 @@ import { createNonReactiveReadonlyStore, } from '../src' import { createRequestHandler } from '../src/ssr/createRequestHandler' -import { cleanPath, hasMissingPathParams, interpolatePath } from '../src/path' import { SEGMENT_TYPE_PATHNAME, SEGMENT_TYPE_WILDCARD, parseSegment, - parseSegments, processRouteTree, } from '../src/new-process-route-tree' +import type { interpolatePath } from '../src/path' import type { SegmentKind } from '../src/new-process-route-tree' import type { RouterHistory } from '@tanstack/history' import type { @@ -117,29 +116,6 @@ export function createTestPathInterpolator( } } -export function interpolateTestPath( - path: string, - params: Record, - decoder?: (encoded: string) => string, - usedParams?: Record, - keys?: Array, - metadata?: { isMissingParams: boolean }, -) { - const segments = parseSegments(false, { fullPath: cleanPath(path) }, 0) - if (keys) { - for (const segment of segments) { - if (typeof segment !== 'string') { - keys.push(segment[1 /* key */]) - } - } - } - const pathname = interpolatePath(path, segments, params, decoder, usedParams) - if (metadata && hasMissingPathParams(segments, params)) { - metadata.isMissingParams = true - } - return pathname -} - export function parseTestPathname(to: string | undefined) { const path = to ?? '' const segments: Array<{ diff --git a/packages/solid-router/src/link.tsx b/packages/solid-router/src/link.tsx index ab5efb1d4d..aeaf6a4a20 100644 --- a/packages/solid-router/src/link.tsx +++ b/packages/solid-router/src/link.tsx @@ -276,8 +276,8 @@ export function useLinkProps< href: base.href, disabled: base.disabled, target: base.target, - ...(style && hasKeys(style) ? { style } : undefined), - ...(className ? { class: className } : undefined), + ...(style && hasKeys(style) && { style }), + ...(className && { class: className }), ...(active && STATIC_ACTIVE_ATTRIBUTES), } as ResolvedLinkStateProps }