diff --git a/.changeset/loud-women-end.md b/.changeset/loud-women-end.md new file mode 100644 index 0000000000..4b9c4c9721 --- /dev/null +++ b/.changeset/loud-women-end.md @@ -0,0 +1,5 @@ +--- +'@tanstack/router-core': patch +--- + +wildcard nodes have the same priority rules as other nodes in route matching diff --git a/packages/router-core/src/new-process-route-tree.ts b/packages/router-core/src/new-process-route-tree.ts index 6ba6a45c87..e4c900d2ed 100644 --- a/packages/router-core/src/new-process-route-tree.ts +++ b/packages/router-core/src/new-process-route-tree.ts @@ -1023,6 +1023,7 @@ type MatchStackFrame = { * If we really really need to support more than 32 segments we can switch to using a `BigInt` here. It's about 2x slower in worst case scenarios. */ skipped: number + /** Positional bitmasks tracking which consumed URL segments matched each segment kind. */ statics: number dynamics: number optionals: number @@ -1066,13 +1067,12 @@ function getNodeMatch( index: 1, skipped: 0, depth: 1, - statics: 1, + statics: 0, dynamics: 0, optionals: 0, }, ] - let wildcardMatch: Frame | null = null let bestFuzzy: Frame | null = null let bestMatch: Frame | null = null @@ -1081,6 +1081,18 @@ function getNodeMatch( const { node, index, skipped, depth, statics, dynamics, optionals } = frame let { extract, rawParams, parsedParams } = frame + // Wildcard candidates are pushed speculatively as fallbacks in case a + // higher-priority wildcard later fails params.parse. If a better wildcard + // has already validated and become bestMatch, lower-priority wildcard + // fallbacks cannot win anymore and should not run params.parse. + if ( + node.kind === SEGMENT_TYPE_WILDCARD && + node.route && + !isFrameMoreSpecific(bestMatch, frame) + ) { + continue + } + if (node.skipOnParamError) { const result = validateMatchParams(path, parts, frame) if (!result) continue @@ -1101,7 +1113,13 @@ function getNodeMatch( const isBeyondPath = index === partsLength if (isBeyondPath) { - if (node.route && !pathIsIndex && isFrameMoreSpecific(bestMatch, frame)) { + if ( + node.route && + (!pathIsIndex || + node.kind === SEGMENT_TYPE_INDEX || + node.kind === SEGMENT_TYPE_WILDCARD) && + isFrameMoreSpecific(bestMatch, frame) + ) { bestMatch = frame } // beyond the length of the path parts, only some segment types can match @@ -1134,7 +1152,12 @@ function getNodeMatch( if (indexValid) { // perfect match, no need to continue // this is an optimization, algorithm should work correctly without this block - if (statics === partsLength && !dynamics && !optionals && !skipped) { + if ( + !dynamics && + !optionals && + !skipped && + isPerfectStaticMatch(statics, partsLength) + ) { return indexFrame } if (isFrameMoreSpecific(bestMatch, indexFrame)) { @@ -1145,8 +1168,9 @@ function getNodeMatch( } // 5. Try wildcard match - if (node.wildcard && isFrameMoreSpecific(wildcardMatch, frame)) { - for (const segment of node.wildcard) { + if (node.wildcard) { + for (let i = node.wildcard.length - 1; i >= 0; i--) { + const segment = node.wildcard[i]! const { prefix, suffix } = segment if (prefix) { if (isBeyondPath) continue @@ -1161,26 +1185,19 @@ function getNodeMatch( const casePart = segment.caseSensitive ? end : end.toLowerCase() if (casePart !== suffix) continue } - // the first wildcard match is the highest priority one - // wildcard matches skip the stack because they cannot have children - const frame = { + // wildcard matches consume the rest of the URL and cannot have children + stack.push({ node: segment, index: partsLength, skipped, - depth, + depth: depth + 1, statics, dynamics, optionals, extract, rawParams, parsedParams, - } - if (segment.skipOnParamError) { - const result = validateMatchParams(path, parts, frame) - if (!result) continue - } - wildcardMatch = frame - break + }) } } @@ -1222,7 +1239,7 @@ function getNodeMatch( depth: nextDepth, statics, dynamics, - optionals: optionals + 1, + optionals: optionals + segmentScore(partsLength, index), extract, rawParams, parsedParams, @@ -1249,7 +1266,7 @@ function getNodeMatch( skipped, depth: depth + 1, statics, - dynamics: dynamics + 1, + dynamics: dynamics + segmentScore(partsLength, index), optionals, extract, rawParams, @@ -1269,7 +1286,7 @@ function getNodeMatch( index: index + 1, skipped, depth: depth + 1, - statics: statics + 1, + statics: statics + segmentScore(partsLength, index), dynamics, optionals, extract, @@ -1288,7 +1305,7 @@ function getNodeMatch( index: index + 1, skipped, depth: depth + 1, - statics: statics + 1, + statics: statics + segmentScore(partsLength, index), dynamics, optionals, extract, @@ -1319,16 +1336,8 @@ function getNodeMatch( } } - if (bestMatch && wildcardMatch) { - return isFrameMoreSpecific(wildcardMatch, bestMatch) - ? bestMatch - : wildcardMatch - } - if (bestMatch) return bestMatch - if (wildcardMatch) return wildcardMatch - if (fuzzy && bestFuzzy) { let sliceIndex = bestFuzzy.index for (let i = 0; i < bestFuzzy.index; i++) { @@ -1343,6 +1352,19 @@ function getNodeMatch( return null } +function segmentScore(partsLength: number, index: number): number { + // The specificity scores are bitmasks over consumed URL segments. Earlier + // URL segments should dominate later ones when comparing scores, so the + // first real segment gets the highest bit and the last gets bit 0. Since + // `parts[0]` is the empty string before the leading slash, real URL segments + // are [1, partsLength), making this segment's bit `partsLength - index - 1`. + return 2 ** (partsLength - index - 1) +} + +function isPerfectStaticMatch(statics: number, partsLength: number): boolean { + return statics === 2 ** (partsLength - 1) - 1 +} + function validateMatchParams( path: string, parts: Array, diff --git a/packages/router-core/tests/new-process-route-tree.test.ts b/packages/router-core/tests/new-process-route-tree.test.ts index 936df988c1..7d13939937 100644 --- a/packages/router-core/tests/new-process-route-tree.test.ts +++ b/packages/router-core/tests/new-process-route-tree.test.ts @@ -233,6 +233,64 @@ describe('findRouteMatch', () => { const treeWithIndex = makeTree(['/a/', '/a/{-$b}']) expect(findRouteMatch('/a', treeWithIndex)?.route.id).toBe('/a/') }) + it('optional+static vs static+wildcard', () => { + const tree = makeTree(['/{-$a}/b', '/a/$']) + expect(findRouteMatch('/a/b', tree)?.route.id).toBe('/a/$') + }) + it('dynamic+static vs static+wildcard', () => { + const tree = makeTree(['/$a/b', '/a/$']) + expect(findRouteMatch('/a/b', tree)?.route.id).toBe('/a/$') + }) + it('dynamic+static+static vs static+wildcard', () => { + const tree = makeTree(['/$a/b/c', '/a/$']) + expect(findRouteMatch('/a/b/c', tree)?.route.id).toBe('/a/$') + }) + it('case-insensitive static+wildcard beats optional+static', () => { + const tree = makeTree(['/{-$a}/bar', '/foo/$']) + expect(findRouteMatch('/FOO/bar', tree)?.route.id).toBe('/foo/$') + }) + it('static+optional+static vs static+static+wildcard', () => { + const tree = makeTree(['/a/{-$b}/c', '/a/b/$']) + expect(findRouteMatch('/a/b/c', tree)?.route.id).toBe('/a/b/$') + }) + it('static+optional+static vs static+static+dynamic', () => { + const tree = makeTree(['/a/{-$b}/c/d', '/a/b/c/$d']) + expect(findRouteMatch('/a/b/c/d', tree)?.route.id).toBe('/a/b/c/$d') + }) + it('static+optional+static+static beats static+dynamic+static+optional', () => { + const tree = makeTree(['/a/$b/c/{-$d}', '/a/{-$b}/c/d']) + expect(findRouteMatch('/a/b/c/d', tree)?.route.id).toBe('/a/{-$b}/c/d') + }) + it('static+dynamic+wildcard vs static+optional+static', () => { + const tree = makeTree(['/a/$b/$', '/a/{-$b}/c']) + expect(findRouteMatch('/a/b/c', tree)?.route.id).toBe('/a/{-$b}/c') + }) + it('static+dynamic+static vs static+dynamic+wildcard', () => { + const tree = makeTree(['/a/$b/c', '/a/$b/$']) + expect(findRouteMatch('/a/b/c', tree)?.route.id).toBe('/a/$b/c') + }) + it('prefix+suffix wildcard wins over prefix wildcard and plain wildcard', () => { + const tree = makeTree(['/a/foo{$}bar', '/a/foo{$}', '/a/$']) + expect(findRouteMatch('/a/fooxbar', tree)?.route.id).toBe( + '/a/foo{$}bar', + ) + }) + it('skipped optional keeps later static priority over wildcard', () => { + const tree = makeTree(['/{-$a}/b/c', '/b/$']) + expect(findRouteMatch('/b/c', tree)?.route.id).toBe('/{-$a}/b/c') + }) + it('skipped optional aligns later static priority with wildcard siblings', () => { + const tree = makeTree(['/{-$a}/b/{-$c}/d', '/b/c/$']) + expect(findRouteMatch('/b/c/d', tree)?.route.id).toBe('/b/c/$') + }) + it('optional child wins over empty wildcard at the same node', () => { + const tree = makeTree(['/a/{-$b}', '/a/$']) + expect(findRouteMatch('/a', tree)?.route.id).toBe('/a/{-$b}') + }) + it('optional+static vs static+optional', () => { + const tree = makeTree(['/{-$a}/b', '/a/{-$b}']) + expect(findRouteMatch('/a/b', tree)?.route.id).toBe('/a/{-$b}') + }) }) }) @@ -776,6 +834,79 @@ describe('findRouteMatch', () => { const actualMatch = findRouteMatch('/dashboard', processed.processedTree) expect(actualMatch?.route.id).toBe('/dashboard/') }) + + it('uses an exact wildcard match instead of a fuzzy layout match', () => { + const tree = makeTree(['/a', '/a/$']) + const match = findRouteMatch('/a/b/c', tree, true) + expect(match?.route.id).toBe('/a/$') + expect(match?.rawParams).toEqual({ '*': 'b/c', _splat: 'b/c' }) + }) + + it('falls back to fuzzy layout matching when wildcard param parsing fails', () => { + const tree = { + id: '__root__', + fullPath: '/', + path: '/', + isRoot: true, + options: {}, + children: [ + { + id: '/a', + fullPath: '/a', + path: 'a', + }, + { + id: '/a/$', + fullPath: '/a/$', + path: 'a/$', + options: { + params: { + parse: () => { + throw new Error('skip') + }, + }, + skipRouteOnParseError: { params: true }, + }, + }, + ], + } + const { processedTree } = processRouteTree(tree) + const match = findRouteMatch('/a/b/c', processedTree, true) + expect(match?.route.id).toBe('/a') + expect(match?.rawParams).toEqual({ '**': 'b/c' }) + }) + + it('falls back to exact layout matching when empty wildcard param parsing fails', () => { + const tree = { + id: '__root__', + fullPath: '/', + path: '/', + isRoot: true, + options: {}, + children: [ + { + id: '/a', + fullPath: '/a', + path: 'a', + }, + { + id: '/a/$', + fullPath: '/a/$', + path: 'a/$', + options: { + params: { + parse: () => { + throw new Error('skip') + }, + }, + skipRouteOnParseError: { params: true }, + }, + }, + ], + } + const { processedTree } = processRouteTree(tree) + expect(findRouteMatch('/a', processedTree)?.route.id).toBe('/a') + }) }) describe('param extraction', () => { @@ -886,9 +1017,305 @@ describe('findRouteMatch', () => { expect(result?.route.id).toBe('/{-$foo}/bar/$') expect(result?.rawParams).toEqual({ '*': 'rest', _splat: 'rest' }) }) + it('validates a stacked wildcard route once', () => { + let calls = 0 + const tree = { + id: '__root__', + fullPath: '/', + path: '/', + isRoot: true, + options: {}, + children: [ + { + id: '/a/$', + fullPath: '/a/$', + path: 'a/$', + isRoot: false, + options: { + params: { + parse: (params: Record) => { + calls++ + return params + }, + }, + skipRouteOnParseError: { params: true }, + }, + }, + ], + } + const { processedTree } = processRouteTree(tree) + const result = findRouteMatch('/a/rest', processedTree) + expect(result?.route.id).toBe('/a/$') + expect(result?.rawParams).toEqual({ '*': 'rest', _splat: 'rest' }) + expect(calls).toBe(1) + }) + it('can fall back to a lower-priority wildcard when wildcard param parsing fails', () => { + const tree = { + id: '__root__', + fullPath: '/', + path: '/', + isRoot: true, + options: {}, + children: [ + { + id: '/a/foo{$}', + fullPath: '/a/foo{$}', + path: 'a/foo{$}', + isRoot: false, + options: { + params: { + parse: () => { + throw new Error('skip') + }, + }, + skipRouteOnParseError: { params: true }, + }, + }, + { + id: '/a/$', + fullPath: '/a/$', + path: 'a/$', + isRoot: false, + options: {}, + }, + ], + } + const { processedTree } = processRouteTree(tree) + const result = findRouteMatch('/a/foobar', processedTree) + expect(result?.route.id).toBe('/a/$') + expect(result?.rawParams).toEqual({ '*': 'foobar', _splat: 'foobar' }) + }) + it('falls back to the next matching wildcard by priority when the best wildcard parse fails', () => { + const tree = { + id: '__root__', + fullPath: '/', + path: '/', + isRoot: true, + options: {}, + children: [ + { + id: '/a/foo{$}bar', + fullPath: '/a/foo{$}bar', + path: 'a/foo{$}bar', + isRoot: false, + options: { + params: { + parse: () => { + throw new Error('skip') + }, + }, + skipRouteOnParseError: { params: true }, + }, + }, + { + id: '/a/foo{$}', + fullPath: '/a/foo{$}', + path: 'a/foo{$}', + isRoot: false, + options: {}, + }, + { + id: '/a/$', + fullPath: '/a/$', + path: 'a/$', + isRoot: false, + options: {}, + }, + ], + } + const { processedTree } = processRouteTree(tree) + const result = findRouteMatch('/a/fooxbar', processedTree) + expect(result?.route.id).toBe('/a/foo{$}') + expect(result?.rawParams).toEqual({ '*': 'xbar', _splat: 'xbar' }) + }) + it('does not validate a lower-priority wildcard after a better wildcard matches', () => { + let higherPriorityCalls = 0 + let lowerPriorityCalls = 0 + const tree = { + id: '__root__', + fullPath: '/', + path: '/', + isRoot: true, + options: {}, + children: [ + { + id: '/a/foo{$}', + fullPath: '/a/foo{$}', + path: 'a/foo{$}', + isRoot: false, + options: { + params: { + parse: (params: Record) => { + higherPriorityCalls++ + return params + }, + }, + skipRouteOnParseError: { params: true, priority: 1 }, + }, + }, + { + id: '/a/$', + fullPath: '/a/$', + path: 'a/$', + isRoot: false, + options: { + params: { + parse: (params: Record) => { + lowerPriorityCalls++ + return params + }, + }, + skipRouteOnParseError: { params: true }, + }, + }, + ], + } + const { processedTree } = processRouteTree(tree) + const result = findRouteMatch('/a/foobar', processedTree) + expect(result?.route.id).toBe('/a/foo{$}') + expect(result?.rawParams).toEqual({ '*': 'bar', _splat: 'bar' }) + expect(higherPriorityCalls).toBe(1) + expect(lowerPriorityCalls).toBe(0) + }) + it('does not validate wildcard fallback after a static sibling matches', () => { + let calls = 0 + const tree = { + id: '__root__', + fullPath: '/', + path: '/', + isRoot: true, + options: {}, + children: [ + { + id: '/a/b', + fullPath: '/a/b', + path: 'a/b', + isRoot: false, + options: {}, + }, + { + id: '/a/$', + fullPath: '/a/$', + path: 'a/$', + isRoot: false, + options: { + params: { + parse: (params: Record) => { + calls++ + return params + }, + }, + skipRouteOnParseError: { params: true }, + }, + }, + ], + } + const { processedTree } = processRouteTree(tree) + const result = findRouteMatch('/a/b', processedTree) + expect(result?.route.id).toBe('/a/b') + expect(calls).toBe(0) + }) + it('does not validate empty wildcard fallback after an index sibling matches', () => { + let calls = 0 + const tree = { + id: '__root__', + fullPath: '/', + path: '/', + isRoot: true, + options: {}, + children: [ + { + id: '/a/', + fullPath: '/a/', + path: 'a/', + isRoot: false, + options: {}, + }, + { + id: '/a/$', + fullPath: '/a/$', + path: 'a/$', + isRoot: false, + options: { + params: { + parse: (params: Record) => { + calls++ + return params + }, + }, + skipRouteOnParseError: { params: true }, + }, + }, + ], + } + const { processedTree } = processRouteTree(tree) + const result = findRouteMatch('/a', processedTree) + expect(result?.route.id).toBe('/a/') + expect(calls).toBe(0) + }) + it('preserves parsed params from a stacked wildcard route', () => { + const tree = { + id: '__root__', + fullPath: '/', + path: '/', + isRoot: true, + options: {}, + children: [ + { + id: '/a/$', + fullPath: '/a/$', + path: 'a/$', + isRoot: false, + options: { + params: { + parse: (params: Record) => ({ + length: params._splat!.length, + }), + }, + skipRouteOnParseError: { params: true }, + }, + }, + ], + } + const { processedTree } = processRouteTree(tree) + const result = findRouteMatch('/a/foo/bar', processedTree) + expect(result?.route.id).toBe('/a/$') + expect(result?.rawParams).toEqual({ '*': 'foo/bar', _splat: 'foo/bar' }) + expect(result?.parsedParams).toEqual({ length: 7 }) + }) }) }) describe('pathless routes', () => { + it('pathless nodes do not shift positional priority scores', () => { + const tree = { + id: '__root__', + fullPath: '/', + path: '/', + isRoot: true, + children: [ + { + id: '/_layout', + fullPath: '/', + children: [ + { + id: '/_layout/a/b', + fullPath: '/a/b', + path: 'a/b', + }, + ], + }, + { + id: '/a/$', + fullPath: '/a/$', + path: 'a/$', + }, + ], + } + const { processedTree } = processRouteTree(tree) + expect(findRouteMatch('/a/b', processedTree)?.route.id).toBe( + '/_layout/a/b', + ) + }) it('builds segment tree correctly', () => { const tree = { path: '/',