diff --git a/packages/router-core/src/new-process-route-tree.ts b/packages/router-core/src/new-process-route-tree.ts index 6978b071ce..2841c17c73 100644 --- a/packages/router-core/src/new-process-route-tree.ts +++ b/packages/router-core/src/new-process-route-tree.ts @@ -190,6 +190,7 @@ export function parseSegment( * @param route The current route to parse. * @param start The starting index for parsing within the route's full path. * @param node The current segment node in the trie to populate. + * @param sortable Dynamic sibling arrays that need sorting. Each array is recorded once when it reaches length 2. * @param onRoute Callback invoked for each route processed. */ function parseSegments( @@ -199,6 +200,7 @@ function parseSegments( start: number, node: AnySegmentNode, depth: number, + sortable?: Array>>, onRoute?: (route: TRouteLike) => void, ) { onRoute?.(route) @@ -291,6 +293,9 @@ function parseSegments( next.parent = node node.dynamic ??= [] node.dynamic.push(next) + if (node.dynamic.length === 2) { + sortable?.push(node.dynamic) + } } break } @@ -333,6 +338,9 @@ function parseSegments( next.depth = depth node.optional ??= [] node.optional.push(next) + if (node.optional.length === 2) { + sortable?.push(node.optional) + } } break } @@ -363,6 +371,9 @@ function parseSegments( next.depth = depth node.wildcard ??= [] node.wildcard.push(next) + if (node.wildcard.length === 2) { + sortable?.push(node.wildcard) + } } } node = nextNode @@ -420,6 +431,7 @@ function parseSegments( cursor, node, depth, + sortable, onRoute, ) } @@ -464,42 +476,6 @@ function sortDynamic( return 0 } -function sortTreeNodes(node: SegmentNode) { - if (node.pathless) { - for (const child of node.pathless) { - sortTreeNodes(child) - } - } - if (node.static) { - for (const child of node.static.values()) { - sortTreeNodes(child) - } - } - if (node.staticInsensitive) { - for (const child of node.staticInsensitive.values()) { - sortTreeNodes(child) - } - } - if (node.dynamic?.length) { - node.dynamic.sort(sortDynamic) - for (const child of node.dynamic) { - sortTreeNodes(child) - } - } - if (node.optional?.length) { - node.optional.sort(sortDynamic) - for (const child of node.optional) { - sortTreeNodes(child) - } - } - if (node.wildcard?.length) { - node.wildcard.sort(sortDynamic) - for (const child of node.wildcard) { - sortTreeNodes(child) - } - } -} - function createStaticNode( fullPath: string, ): StaticSegmentNode { @@ -663,10 +639,13 @@ export function processRouteMasks< ) { const segmentTree = createStaticNode('/') const data = new Uint16Array(6) + const sortable: Array>> = [] for (const route of routeList) { - parseSegments(false, data, route, 1, segmentTree, 0) + parseSegments(false, data, route, 1, segmentTree, 0, sortable) + } + for (const nodes of sortable) { + nodes.sort(sortDynamic) } - sortTreeNodes(segmentTree) processedTree.masksTree = segmentTree processedTree.flatCache = createLRUCache< string, @@ -791,32 +770,44 @@ export function processRouteTree< const data = new Uint16Array(6) const routesById = {} as Record const routesByPath = {} as Record + const sortable: Array>> = [] let index = 0 - parseSegments(caseSensitive, data, routeTree, 1, segmentTree, 0, (route) => { - initRoute?.(route, index) + parseSegments( + caseSensitive, + data, + routeTree, + 1, + segmentTree, + 0, + sortable, + (route) => { + initRoute?.(route, index) + + if (route.id in routesById) { + if (process.env.NODE_ENV !== 'production') { + throw new Error( + `Invariant failed: Duplicate routes found with id: ${String(route.id)}`, + ) + } - if (route.id in routesById) { - if (process.env.NODE_ENV !== 'production') { - throw new Error( - `Invariant failed: Duplicate routes found with id: ${String(route.id)}`, - ) + invariant() } - invariant() - } - - routesById[route.id] = route + routesById[route.id] = route - if (index !== 0 && route.path) { - const trimmedFullPath = trimPathRight(route.fullPath) - if (!routesByPath[trimmedFullPath] || route.fullPath.endsWith('/')) { - routesByPath[trimmedFullPath] = route + if (index !== 0 && route.path) { + const trimmedFullPath = trimPathRight(route.fullPath) + if (!routesByPath[trimmedFullPath] || route.fullPath.endsWith('/')) { + routesByPath[trimmedFullPath] = route + } } - } - index++ - }) - sortTreeNodes(segmentTree) + index++ + }, + ) + for (const nodes of sortable) { + nodes.sort(sortDynamic) + } const processedTree: ProcessedTree = { segmentTree, singleCache: createLRUCache>(1000), 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 d93581355e..1703c69042 100644 --- a/packages/router-core/tests/new-process-route-tree.test.ts +++ b/packages/router-core/tests/new-process-route-tree.test.ts @@ -114,6 +114,10 @@ describe('findRouteMatch', () => { const tree = makeTree(['/a/{-$b}b', '/a/{-$b}']) expect(findRouteMatch('/a/bbb', tree)?.route.id).toBe('/a/{-$b}b') }) + it('prefix+suffix optional wins when declared after plain optional', () => { + const tree = makeTree(['/a/{-$b}', '/a/b{-$b}b']) + expect(findRouteMatch('/a/bbb', tree)?.route.id).toBe('/a/b{-$b}b') + }) it('prefix+suffix wildcard wins over plain wildcard', () => { const tree = makeTree(['/a/b{$}b', '/a/$']) @@ -127,6 +131,10 @@ describe('findRouteMatch', () => { const tree = makeTree(['/a/{$}b', '/a/$']) expect(findRouteMatch('/a/bbb', tree)?.route.id).toBe('/a/{$}b') }) + it('sorts a third, more specific wildcard declared last', () => { + const tree = makeTree(['/a/$', '/a/b{$}', '/a/b{$}b']) + expect(findRouteMatch('/a/bbb', tree)?.route.id).toBe('/a/b{$}b') + }) }) describe('prefix / suffix lengths', () => { @@ -1672,4 +1680,29 @@ describe('processRouteMasks', { sequential: true }, () => { expect(res?.route.from).toBe('/a/b/{$}.txt') expect(res?.rawParams).toEqual({ '*': 'file/path', _splat: 'file/path' }) }) + it('sorts competing route masks declared least-specific first', () => { + const localTree = processRouteTree(routeTree).processedTree + processRouteMasks( + [ + { from: '/dynamic/$param', routeTree }, + { from: '/dynamic/prefix{$param}', routeTree }, + { from: '/optional/{-$param}', routeTree }, + { from: '/optional/prefix{-$param}', routeTree }, + { from: '/wildcard/$', routeTree }, + { from: '/wildcard/prefix{$}', routeTree }, + { from: '/wildcard/prefix{$}.txt', routeTree }, + ], + localTree, + ) + + expect(findFlatMatch('/dynamic/prefixvalue', localTree)?.route.from).toBe( + '/dynamic/prefix{$param}', + ) + expect(findFlatMatch('/optional/prefixvalue', localTree)?.route.from).toBe( + '/optional/prefix{-$param}', + ) + expect( + findFlatMatch('/wildcard/prefixvalue.txt', localTree)?.route.from, + ).toBe('/wildcard/prefix{$}.txt') + }) })