refactor(router-core): Process routeTree into segment tree instead of flatRoutes [WIP] #5722
Conversation
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThis PR replaces legacy path parsing/matching and flat route lists with a segment-tree based route processor, adds a new route-tree implementation and tests, removes several path-related public exports, updates RouterCore to use a ProcessedTree, and marks MatchRouteOptions.caseSensitive as deprecated in docs and JSDoc. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Router
participant Old as "Old Processor\n(process-route-tree)"
participant New as "New Processor\n(new-process-route-tree)"
rect rgb(240,228,228)
Note over Old: Legacy flow (removed)
User->>Router: buildRouteTree(routeTree)
Router->>Old: processRouteTree(routeTree)
Old->>Old: sort & flatten routes
Old-->>Router: flatRoutes
User->>Router: getMatchedRoutes(pathname, routePathname)
Router->>Old: matchPathname / matchByPath
Old-->>Router: matchedRoutes
end
rect rgb(228,240,232)
Note over New: New flow (added)
User->>Router: buildRouteTree(routeTree)
Router->>New: processRouteTree(routeTree, caseSensitive)
New->>New: parseSegments -> build segment tree
New-->>Router: processedTree
User->>Router: getMatchedRoutes(pathname)
Router->>New: findRouteMatch(pathname, processedTree)
New->>New: DFS traversal, extract params
New-->>Router: matchedRoutes + foundRoute + params
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60–90 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
Comment |
|
View your CI Pipeline Execution ↗ for commit 320cdbd
☁️ Nx Cloud last updated this comment at |
| function sortDynamic(a: { prefix?: string, suffix?: string }, b: { prefix?: string, suffix?: string }) { | ||
| if (a.prefix && b.prefix && a.prefix !== b.prefix) { | ||
| if (a.prefix.startsWith(b.prefix)) return -1 | ||
| if (b.prefix.startsWith(a.prefix)) return 1 | ||
| } | ||
| if (a.suffix && b.suffix && a.suffix !== b.suffix) { | ||
| if (a.suffix.endsWith(b.suffix)) return -1 | ||
| if (b.suffix.endsWith(a.suffix)) return 1 | ||
| } | ||
| if (a.prefix && !b.prefix) return -1 | ||
| if (!a.prefix && b.prefix) return 1 | ||
| if (a.suffix && !b.suffix) return -1 | ||
| if (!a.suffix && b.suffix) return 1 | ||
| return 0 | ||
| } |
There was a problem hiding this comment.
should rank case-sensitive routes w/ prefix/suffix higher than case-insensitive ones
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (4)
packages/router-core/src/new-process-route-tree.ts (3)
170-170: Critical: Fix depth calculation when reusing existing nodes.Lines 170, 185, 222, 260, and 288 use
++depthto setnext.depth, which mutates the outerdepthvariable. When the loop encounters an existing child node,depthis incremented but never synchronized with the child's actual depth. This causes subsequent siblings to receive incorrect depth values, breakingbuildBranchandextractParamsfor routes that share prefixes (e.g.,/foo/barfollowed by/foo/baz).Apply this diff to compute depth from the parent and resync for both new and existing nodes:
if (caseSensitive) { const existingNode = node.static?.get(value) if (existingNode) { nextNode = existingNode } else { node.static ??= new Map() const next = createStaticNode<TRouteLike>( route.fullPath ?? route.from, ) next.parent = node - next.depth = ++depth + next.depth = node.depth + 1 nextNode = next node.static.set(value, next) } } else { const name = value.toLowerCase() const existingNode = node.staticInsensitive?.get(name) if (existingNode) { nextNode = existingNode } else { node.staticInsensitive ??= new Map() const next = createStaticNode<TRouteLike>( route.fullPath ?? route.from, ) next.parent = node - next.depth = ++depth + next.depth = node.depth + 1 nextNode = next node.staticInsensitive.set(name, next) } }Apply similar fixes to dynamic/optional/wildcard node creation:
) nextNode = next - next.depth = ++depth + next.depth = node.depth + 1 next.parent = nodeAnd resync depth after advancing to the next node:
node = nextNode + depth = node.depth }Also applies to: 185-185, 222-222, 260-260, 288-288
546-546: Critical: Pass caseSensitive parameter to parseSegments.Line 546 hardcodes
caseSensitiveas the first argument but then callsparseSegments(caseSensitive, ...). However, the temporary tree is always built with case-insensitive semantics because the hardcoded value should match the caller'scaseSensitiveparameter. Currently, static segments are lowercased regardless of the caller's flag, breakingrouter.matchRoute({ caseSensitive: true }).The code currently passes
caseSensitivecorrectly on line 546. If tests are failing, verify the logic insideparseSegmentsproperly honors the flag.
895-898: Critical: Fix wildcard suffix validation to use the correct segment.Lines 895-898 check the suffix for wildcards, but line 896 constructs
endfromparts.slice(index).join('/')which represents the rest of the path, not the current segment. For wildcards in the middle of a path, this reads the wrong portion and breaks suffix matching.Apply this diff:
if (suffix) { - const end = parts.slice(index).join('/').slice(-suffix.length) - const casePart = segment.caseSensitive ? end : end.toLowerCase() - if (casePart !== suffix) continue + const lastPart = parts[parts.length - 1]! + const casePart = segment.caseSensitive ? lastPart : lastPart.toLowerCase() + if (!casePart.endsWith(suffix)) continue }packages/router-core/src/path.ts (1)
194-201: Fix undefined prefix/suffix in optional param serialization.Line 200 uses template literals with
prefixandsuffixvariables that may beundefined. When either is undefined, the resulting string will contain the literal text"undefined", corrupting the path template (e.g.,"undefined{-$id}undefined").Apply this diff to normalize undefined values to empty strings:
} else { // SEGMENT_TYPE_OPTIONAL_PARAM - joined += `${prefix}{-$${value}}${suffix}` + const safePrefix = prefix ?? '' + const safeSuffix = suffix ?? '' + joined += `${safePrefix}{-$${value}}${safeSuffix}` }
🧹 Nitpick comments (5)
packages/router-core/src/new-process-route-tree.ts (1)
311-328: Consider adding alphabetical fallback to sortDynamic.The
sortDynamicfunction prioritizes prefix/suffix length but doesn't provide a stable fallback when segments are identical. This can lead to non-deterministic ordering across environments or V8 versions.Add a final alphabetical comparison:
if (a.suffix && !b.suffix) return -1 if (!a.suffix && b.suffix) return 1 + // Fallback to alphabetical order for stability + const aStr = (a.prefix ?? '') + (a.suffix ?? '') + const bStr = (b.prefix ?? '') + (b.suffix ?? '') + return aStr.localeCompare(bStr) - return 0 }packages/router-core/tests/optional-path-params-clean.test.ts (1)
3-10: Fix import order and type import style.Lines 7 and 9 violate ESLint rules for import ordering and type import style. These are minor style issues but should be fixed for consistency.
Apply this diff:
import { findSingleMatch, parseSegment, processRouteTree, - SEGMENT_TYPE_OPTIONAL_PARAM, SEGMENT_TYPE_PATHNAME, - type SegmentKind, + SEGMENT_TYPE_OPTIONAL_PARAM, } from '../src/new-process-route-tree' +import type { SegmentKind } from '../src/new-process-route-tree'packages/router-core/tests/new-process-route-tree.test.ts (1)
4-4: Remove unused import or implement processFlatRouteList tests.Line 4 imports
processFlatRouteListbut it's only used in adescribe.todoblock (lines 317-329). Either implement the tests or remove the import to clean up the code.If tests aren't ready, remove the import:
import { findRouteMatch, - processFlatRouteList, processRouteTree, } from '../src/new-process-route-tree'Also applies to: 317-329
packages/router-core/tests/path.test.ts (1)
9-18: Fix import issues: remove unused import and fix style.Line 13 imports
SEGMENT_TYPE_OPTIONAL_PARAMbut it's never used. Lines 13 and 17 also violate import ordering and type import style rules.Apply this diff:
import { findSingleMatch, parseSegment, processRouteTree, - SEGMENT_TYPE_OPTIONAL_PARAM, SEGMENT_TYPE_PARAM, SEGMENT_TYPE_PATHNAME, SEGMENT_TYPE_WILDCARD, - type SegmentKind, } from '../src/new-process-route-tree' +import type { SegmentKind } from '../src/new-process-route-tree'packages/router-core/tests/optional-path-params.test.ts (1)
3-12: Fix import order and type import style.Lines 7 and 11 violate ESLint rules for import ordering and type import style, similar to other test files.
Apply this diff:
import { findSingleMatch, parseSegment, processRouteTree, - SEGMENT_TYPE_OPTIONAL_PARAM, SEGMENT_TYPE_PARAM, SEGMENT_TYPE_PATHNAME, SEGMENT_TYPE_WILDCARD, - type SegmentKind, + SEGMENT_TYPE_OPTIONAL_PARAM, } from '../src/new-process-route-tree' +import type { SegmentKind } from '../src/new-process-route-tree'
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
packages/router-core/src/new-process-route-tree.ts(1 hunks)packages/router-core/src/path.ts(5 hunks)packages/router-core/src/router.ts(10 hunks)packages/router-core/tests/match-by-path.test.ts(1 hunks)packages/router-core/tests/new-process-route-tree.test.ts(1 hunks)packages/router-core/tests/optional-path-params-clean.test.ts(2 hunks)packages/router-core/tests/optional-path-params.test.ts(5 hunks)packages/router-core/tests/path.test.ts(17 hunks)packages/router-devtools-core/src/BaseTanStackRouterDevtoolsPanel.tsx(0 hunks)packages/router-plugin/src/core/route-hmr-statement.ts(3 hunks)
💤 Files with no reviewable changes (1)
- packages/router-devtools-core/src/BaseTanStackRouterDevtoolsPanel.tsx
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use TypeScript in strict mode with extensive type safety across the codebase
Files:
packages/router-core/tests/optional-path-params-clean.test.tspackages/router-plugin/src/core/route-hmr-statement.tspackages/router-core/tests/match-by-path.test.tspackages/router-core/src/path.tspackages/router-core/src/router.tspackages/router-core/tests/optional-path-params.test.tspackages/router-core/tests/new-process-route-tree.test.tspackages/router-core/src/new-process-route-tree.tspackages/router-core/tests/path.test.ts
packages/router-core/**
📄 CodeRabbit inference engine (AGENTS.md)
Keep framework-agnostic core router logic in packages/router-core/
Files:
packages/router-core/tests/optional-path-params-clean.test.tspackages/router-core/tests/match-by-path.test.tspackages/router-core/src/path.tspackages/router-core/src/router.tspackages/router-core/tests/optional-path-params.test.tspackages/router-core/tests/new-process-route-tree.test.tspackages/router-core/src/new-process-route-tree.tspackages/router-core/tests/path.test.ts
packages/{router-cli,router-generator,router-plugin,virtual-file-routes}/**
📄 CodeRabbit inference engine (AGENTS.md)
Keep CLI, generators, bundler plugins, and virtual file routing utilities in their dedicated tooling package directories
Files:
packages/router-plugin/src/core/route-hmr-statement.ts
packages/router-plugin/**
📄 CodeRabbit inference engine (AGENTS.md)
Use unplugin for universal bundler plugins in the router-plugin package
Files:
packages/router-plugin/src/core/route-hmr-statement.ts
🧠 Learnings (12)
📓 Common learnings
Learnt from: CR
Repo: TanStack/router PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-09-23T17:36:12.598Z
Learning: Applies to packages/router-core/** : Keep framework-agnostic core router logic in packages/router-core/
Learnt from: nlynzaad
Repo: TanStack/router PR: 5182
File: e2e/react-router/basic-file-based/tests/non-nested-paths.spec.ts:167-172
Timestamp: 2025-09-22T00:56:53.426Z
Learning: In TanStack Router, underscores are intentionally stripped from route segments during path parsing, but preserved in base path segments. This is the expected behavior implemented in PR #5182.
Learnt from: nlynzaad
Repo: TanStack/router PR: 5182
File: e2e/react-router/basic-file-based/src/routes/non-nested/named/$baz_.bar.tsx:3-5
Timestamp: 2025-09-22T00:56:49.237Z
Learning: In TanStack Router, underscores are intentionally stripped from route segments (e.g., `$baz_` becomes `baz` in generated types) but should be preserved in base path segments. This is the correct behavior as of the fix in PR #5182.
Learnt from: nlynzaad
Repo: TanStack/router PR: 5402
File: packages/router-generator/tests/generator/no-formatted-route-tree/routeTree.nonnested.snapshot.ts:19-21
Timestamp: 2025-10-08T08:11:47.088Z
Learning: Test snapshot files in the router-generator tests directory (e.g., files matching the pattern `packages/router-generator/tests/generator/**/routeTree*.snapshot.ts` or `routeTree*.snapshot.js`) should not be modified or have issues flagged, as they are fixtures used to verify the generator's output and are intentionally preserved as-is.
📚 Learning: 2025-10-08T08:11:47.088Z
Learnt from: nlynzaad
Repo: TanStack/router PR: 5402
File: packages/router-generator/tests/generator/no-formatted-route-tree/routeTree.nonnested.snapshot.ts:19-21
Timestamp: 2025-10-08T08:11:47.088Z
Learning: Test snapshot files in the router-generator tests directory (e.g., files matching the pattern `packages/router-generator/tests/generator/**/routeTree*.snapshot.ts` or `routeTree*.snapshot.js`) should not be modified or have issues flagged, as they are fixtures used to verify the generator's output and are intentionally preserved as-is.
Applied to files:
packages/router-core/tests/optional-path-params-clean.test.tspackages/router-plugin/src/core/route-hmr-statement.tspackages/router-core/tests/match-by-path.test.tspackages/router-core/src/path.tspackages/router-core/src/router.tspackages/router-core/tests/optional-path-params.test.tspackages/router-core/tests/new-process-route-tree.test.tspackages/router-core/src/new-process-route-tree.tspackages/router-core/tests/path.test.ts
📚 Learning: 2025-10-14T18:59:33.990Z
Learnt from: FatahChan
Repo: TanStack/router PR: 5475
File: e2e/react-start/basic-prerendering/src/routes/redirect/$target/via-beforeLoad.tsx:8-0
Timestamp: 2025-10-14T18:59:33.990Z
Learning: In TanStack Router e2e test files, when a route parameter is validated at the route level (e.g., using zod in validateSearch or param validation), switch statements on that parameter do not require a default case, as the validation ensures only expected values will reach the switch.
Applied to files:
packages/router-core/tests/optional-path-params-clean.test.tspackages/router-core/tests/match-by-path.test.tspackages/router-core/tests/optional-path-params.test.tspackages/router-core/tests/new-process-route-tree.test.tspackages/router-core/tests/path.test.ts
📚 Learning: 2025-09-23T17:36:12.598Z
Learnt from: CR
Repo: TanStack/router PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-09-23T17:36:12.598Z
Learning: Applies to packages/router-core/** : Keep framework-agnostic core router logic in packages/router-core/
Applied to files:
packages/router-core/tests/optional-path-params-clean.test.tspackages/router-plugin/src/core/route-hmr-statement.tspackages/router-core/tests/match-by-path.test.tspackages/router-core/src/path.tspackages/router-core/src/router.tspackages/router-core/tests/optional-path-params.test.tspackages/router-core/tests/new-process-route-tree.test.tspackages/router-core/src/new-process-route-tree.tspackages/router-core/tests/path.test.ts
📚 Learning: 2025-10-01T18:30:26.591Z
Learnt from: schiller-manuel
Repo: TanStack/router PR: 5330
File: packages/router-core/src/router.ts:2231-2245
Timestamp: 2025-10-01T18:30:26.591Z
Learning: In `packages/router-core/src/router.ts`, the `resolveRedirect` method intentionally strips the router's origin from redirect URLs when they match (e.g., `https://foo.com/bar` → `/bar` for same-origin redirects) while preserving the full URL for cross-origin redirects. This logic should not be removed or simplified to use `location.publicHref` directly.
Applied to files:
packages/router-core/tests/optional-path-params-clean.test.tspackages/router-plugin/src/core/route-hmr-statement.tspackages/router-core/src/path.tspackages/router-core/src/router.tspackages/router-core/tests/optional-path-params.test.tspackages/router-core/src/new-process-route-tree.ts
📚 Learning: 2025-09-28T21:41:45.233Z
Learnt from: nlynzaad
Repo: TanStack/router PR: 5284
File: e2e/react-start/basic/server.js:50-0
Timestamp: 2025-09-28T21:41:45.233Z
Learning: In Express v5, catch-all routes must use named wildcards. Use `/*splat` to match everything except root path, or `/{*splat}` (with braces) to match including root path. The old `*` syntax is not allowed and will cause "Missing parameter name" errors. This breaking change requires explicit naming of wildcard parameters.
Applied to files:
packages/router-core/tests/optional-path-params-clean.test.tspackages/router-core/tests/match-by-path.test.tspackages/router-core/src/path.tspackages/router-core/tests/optional-path-params.test.tspackages/router-core/tests/new-process-route-tree.test.tspackages/router-core/src/new-process-route-tree.tspackages/router-core/tests/path.test.ts
📚 Learning: 2025-10-01T18:31:35.420Z
Learnt from: schiller-manuel
Repo: TanStack/router PR: 5330
File: e2e/react-start/custom-basepath/src/routeTree.gen.ts:58-61
Timestamp: 2025-10-01T18:31:35.420Z
Learning: Do not review files named `routeTree.gen.ts` in TanStack Router repositories, as these are autogenerated files that should not be manually modified.
Applied to files:
packages/router-plugin/src/core/route-hmr-statement.tspackages/router-core/tests/match-by-path.test.tspackages/router-core/src/router.tspackages/router-core/tests/new-process-route-tree.test.tspackages/router-core/src/new-process-route-tree.tspackages/router-core/tests/path.test.ts
📚 Learning: 2025-09-23T17:36:12.598Z
Learnt from: CR
Repo: TanStack/router PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-09-23T17:36:12.598Z
Learning: Applies to packages/router-plugin/** : Use unplugin for universal bundler plugins in the router-plugin package
Applied to files:
packages/router-plugin/src/core/route-hmr-statement.ts
📚 Learning: 2025-09-23T17:36:12.598Z
Learnt from: CR
Repo: TanStack/router PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-09-23T17:36:12.598Z
Learning: Applies to packages/{router-cli,router-generator,router-plugin,virtual-file-routes}/** : Keep CLI, generators, bundler plugins, and virtual file routing utilities in their dedicated tooling package directories
Applied to files:
packages/router-plugin/src/core/route-hmr-statement.tspackages/router-core/src/path.tspackages/router-core/src/router.tspackages/router-core/tests/new-process-route-tree.test.tspackages/router-core/src/new-process-route-tree.tspackages/router-core/tests/path.test.ts
📚 Learning: 2025-09-23T17:36:12.598Z
Learnt from: CR
Repo: TanStack/router PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-09-23T17:36:12.598Z
Learning: Applies to packages/{router-devtools,*-router-devtools}/** : Keep router devtools packages in packages/router-devtools/ and packages/*-router-devtools/
Applied to files:
packages/router-plugin/src/core/route-hmr-statement.tspackages/router-core/src/new-process-route-tree.ts
📚 Learning: 2025-09-22T00:56:49.237Z
Learnt from: nlynzaad
Repo: TanStack/router PR: 5182
File: e2e/react-router/basic-file-based/src/routes/non-nested/named/$baz_.bar.tsx:3-5
Timestamp: 2025-09-22T00:56:49.237Z
Learning: In TanStack Router, underscores are intentionally stripped from route segments (e.g., `$baz_` becomes `baz` in generated types) but should be preserved in base path segments. This is the correct behavior as of the fix in PR #5182.
Applied to files:
packages/router-core/src/path.tspackages/router-core/src/new-process-route-tree.tspackages/router-core/tests/path.test.ts
📚 Learning: 2025-09-22T00:56:53.426Z
Learnt from: nlynzaad
Repo: TanStack/router PR: 5182
File: e2e/react-router/basic-file-based/tests/non-nested-paths.spec.ts:167-172
Timestamp: 2025-09-22T00:56:53.426Z
Learning: In TanStack Router, underscores are intentionally stripped from route segments during path parsing, but preserved in base path segments. This is the expected behavior implemented in PR #5182.
Applied to files:
packages/router-core/src/path.tspackages/router-core/src/new-process-route-tree.tspackages/router-core/tests/path.test.ts
🪛 ESLint
packages/router-core/tests/optional-path-params-clean.test.ts
[error] 7-7: Member 'SEGMENT_TYPE_OPTIONAL_PARAM' of the import declaration should be sorted alphabetically.
(sort-imports)
[error] 9-9: Prefer using a top-level type-only import instead of inline type specifiers.
(import/consistent-type-specifier-style)
packages/router-core/tests/optional-path-params.test.ts
[error] 7-7: Member 'SEGMENT_TYPE_OPTIONAL_PARAM' of the import declaration should be sorted alphabetically.
(sort-imports)
[error] 11-11: Prefer using a top-level type-only import instead of inline type specifiers.
(import/consistent-type-specifier-style)
packages/router-core/tests/new-process-route-tree.test.ts
[error] 4-4: 'processFlatRouteList' is defined but never used.
(unused-imports/no-unused-imports)
packages/router-core/tests/path.test.ts
[error] 13-13: Member 'SEGMENT_TYPE_OPTIONAL_PARAM' of the import declaration should be sorted alphabetically.
(sort-imports)
[error] 13-13: 'SEGMENT_TYPE_OPTIONAL_PARAM' is defined but never used.
(unused-imports/no-unused-imports)
[error] 17-17: Prefer using a top-level type-only import instead of inline type specifiers.
(import/consistent-type-specifier-style)
🪛 GitHub Actions: pr
packages/router-core/tests/match-by-path.test.ts
[error] 42-42: Multiple test failures reported in match-by-path tests: expected undefined vs {} or mismatched shape in various matchByPath assertions.
packages/router-core/src/router.ts
[warning] 2094-2097: Unused eslint-disable directive (no problems were reported from '@typescript-eslint/require-await'); Async arrow function has no 'await' expression (require-await).
🔇 Additional comments (17)
packages/router-core/src/new-process-route-tree.ts (1)
502-502: Verify depth initialization consistency across processing functions.Line 502 initializes depth at
0when callingparseSegments, which is consistent withprocessRouteTree(line 596). Confirm this is correct for flat route lists and thatbuildBranchworks as expected when mixing flat and tree routes.If depth 0 is correct for the root, this is fine. Otherwise, verify against tests.
packages/router-plugin/src/core/route-hmr-statement.ts (1)
25-28: Document limitation: HMR does not handle new route additions.The TODO on line 27 indicates that adding a new route requires rebuilding the entire tree, which the current HMR logic doesn't support. This means HMR will only work for updates to existing routes, not for adding/removing routes from the tree.
Consider documenting this limitation in a comment or the PR description so users are aware that adding new routes during development will require a full reload.
packages/router-core/src/path.ts (1)
233-350: Impressive refactor: cursor-based interpolation is much cleaner.The new cursor-based approach to path interpolation is a significant improvement over the old implementation. It's more direct, easier to follow, and handles all segment types (pathname, param, wildcard, optional) in a unified way. The elimination of the intermediate segment array and direct string assembly should also improve performance.
packages/router-core/tests/match-by-path.test.ts (1)
12-24: Fix matchByPath wrapper to return undefined for non-matches.The wrapper returns
match ? match.params : undefined(line 23), but then line 24 is missing in the displayed code. Based on the pipeline failures expectingundefinedbut receiving{}, verify that the wrapper correctly returnsundefinedfor non-matching paths rather than defaulting to an empty object.Ensure the logic matches test expectations:
const matchByPath = ( from: string, options: { to: string; caseSensitive?: boolean; fuzzy?: boolean }, ) => { const match = findSingleMatch( options.to, options.caseSensitive ?? false, options.fuzzy ?? false, from, processedTree, ) return match ? match.params : undefined // Don't default to {} }⛔ Skipped due to learnings
Learnt from: nlynzaad Repo: TanStack/router PR: 5402 File: packages/router-generator/tests/generator/no-formatted-route-tree/routeTree.nonnested.snapshot.ts:19-21 Timestamp: 2025-10-08T08:11:47.088Z Learning: Test snapshot files in the router-generator tests directory (e.g., files matching the pattern `packages/router-generator/tests/generator/**/routeTree*.snapshot.ts` or `routeTree*.snapshot.js`) should not be modified or have issues flagged, as they are fixtures used to verify the generator's output and are intentionally preserved as-is.Learnt from: FatahChan Repo: TanStack/router PR: 5475 File: e2e/react-start/basic-prerendering/src/routes/redirect/$target/via-beforeLoad.tsx:8-0 Timestamp: 2025-10-14T18:59:33.990Z Learning: In TanStack Router e2e test files, when a route parameter is validated at the route level (e.g., using zod in validateSearch or param validation), switch statements on that parameter do not require a default case, as the validation ensures only expected values will reach the switch.packages/router-core/src/router.ts (13)
12-17: LGTM: Import changes align with the new route-tree architecture.The new imports from
./new-process-route-treeprovide the necessary utilities for the processedTree-based matching pipeline.
25-25: LGTM: LRU cache import supports performance optimization.This import enables the caching mechanism for path resolution mentioned in the PR benchmarks (~2.7× faster resolvePath).
38-38: LGTM: Type import necessary for the new architecture.
698-702: Simplified signature aligns with the new architecture.The
GetMatchRoutesFnsignature now accepts only a pathname and returns a complete match result object. This simplification is appropriate given that the processedTree encapsulates the matching logic.
905-905: Verify theanytype parameters in ProcessedTree.The
ProcessedTree<TRouteTree, any, any>usesanyfor two generic parameters. Please ensure these cannot be more specifically typed for better type safety, or confirm thatanyis intentional for this context.Based on coding guidelines for TypeScript strict mode.
1097-1109: LGTM: buildRouteTree correctly migrates to processedTree architecture.The method now properly destructures the new return value from
processRouteTreeand stores theprocessedTreeon the router instance.
1207-1208: LGTM: LRU cache initialization for path resolution performance.A cache size of 1000 entries is reasonable and aligns with the PR's performance improvements (~2.7× faster resolvePath).
1215-1215: LGTM: Cache correctly threaded through to resolvePath.
1540-1546: LGTM: getMatchedRoutes simplified to delegate to the new matching logic.The method now correctly passes
processedTreeto the standalone helper function and accepts only the pathname parameter.
1607-1607: LGTM: Correctly uses the new getMatchedRoutes signature.
1777-1793: Route masking correctly migrated to findFlatMatch.The masking logic now uses
findFlatMatchwith theprocessedTree. The flow properly extracts params and route properties from the match result.
2503-2528: matchRoute correctly uses findSingleMatch with processedTree.The method now delegates to
findSingleMatchand properly accesses theparamsproperty from the returned match. The logic for parameter and search comparison is preserved.
2621-2651: The review comment's premise is incorrect.The third parameter to
findRouteMatchisfuzzy(controls fuzzy/partial matching), notcaseSensitive. Case sensitivity is already handled during route tree construction viaprocessRouteTree(routeTree, this.options.caseSensitive, ...)at router initialization (line 1099). TheprocessedTreepassed togetMatchedRoutesalready encodes case sensitivity rules from route definitions, sofindRouteMatchdoes not need a separatecaseSensitiveparameter.Likely an incorrect or invalid review comment.
Warning
untested, DO NOT MERGE
Design exploration of a rewrite of
processRouteTreeto yield a trie-like tree of segments. This should replacepackages/router-core/src/process-route-tree.tspackages/router-core/src/path.tsGoals:
Non-goals:
to-do:
/)Bench:
useMatchRoute)Notes:
useMatchRoute) does not care about therouteTree. It will match 2 arbitrary strings. This can lead to unexpected results, and is probably not the behavior we want in the long term.For now, this PR implements
findSingleMatchto preserve that behavior, and this method is slower than the current implementation. However, when we switch to matching against the routeTree (usingfindRouteMatchinstead), it will be faster.Summary by CodeRabbit
Deprecations
Breaking Changes
New Features
Tests / Quality