refactor(router-core): processRouteTree in its own file #5128
Conversation
WalkthroughThe route-tree processing logic was extracted from router.ts into a new module process-route-tree.ts, with RouteLike moved to route.ts. Public exports in index.ts now re-export processRouteTree and ProcessRouteTreeResult from the new module. Tests were updated to import from the new paths and use RouteLike. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor App
participant Router as RouterCore
participant Proc as process-route-tree.ts
App->>Router: buildRouteTree(routeTree)
Router->>Proc: processRouteTree({ routeTree, initRoute? })
activate Proc
Proc->>Proc: Traverse tree (initRoute?, parent links)
Proc->>Proc: Build routesById / routesByPath
Proc->>Proc: Rank and sort flatRoutes
Proc-->>Router: { routesById, routesByPath, flatRoutes }
deactivate Proc
Router-->>App: Build result continues with processed routes
sequenceDiagram
autonumber
participant Proc as process-route-tree
participant Node as Route node
participant Maps as {byId, byPath}
participant Rank as sortRoutes
Proc->>Node: visit(root)
loop DFS over children
Proc->>Node: initRoute?(node, index)
Proc->>Maps: record id/path
end
Proc->>Rank: compute scores per route
Rank-->>Proc: sorted flatRoutes
Proc-->>Proc: return { routesById, routesByPath, flatRoutes }
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Suggested reviewers
Pre-merge checks (2 passed, 1 warning)❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
Poem
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. ✨ Finishing touches
🧪 Generate unit tests
Comment |
|
View your CI Pipeline Execution ↗ for commit a2a07b4
☁️ Nx Cloud last updated this comment at |
More templates
@tanstack/arktype-adapter
@tanstack/directive-functions-plugin
@tanstack/eslint-plugin-router
@tanstack/history
@tanstack/react-router
@tanstack/react-router-devtools
@tanstack/react-router-ssr-query
@tanstack/react-start
@tanstack/react-start-client
@tanstack/react-start-plugin
@tanstack/react-start-server
@tanstack/router-cli
@tanstack/router-core
@tanstack/router-devtools
@tanstack/router-devtools-core
@tanstack/router-generator
@tanstack/router-plugin
@tanstack/router-ssr-query-core
@tanstack/router-utils
@tanstack/router-vite-plugin
@tanstack/server-functions-plugin
@tanstack/solid-router
@tanstack/solid-router-devtools
@tanstack/solid-start
@tanstack/solid-start-client
@tanstack/solid-start-plugin
@tanstack/solid-start-server
@tanstack/start-client-core
@tanstack/start-plugin-core
@tanstack/start-server-core
@tanstack/start-server-functions-client
@tanstack/start-server-functions-fetcher
@tanstack/start-server-functions-server
@tanstack/start-storage-context
@tanstack/valibot-adapter
@tanstack/virtual-file-routes
@tanstack/zod-adapter
commit: |
Sheraff
deleted the
refactor-router-core-process-route-tree-own-file
branch
![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
🧹 Nitpick comments (4)
packages/router-core/src/process-route-tree.ts (2)
14-22: Consider documenting the scoring constants.While the constant names are descriptive, adding comments explaining the scoring strategy would help future maintainers understand why these specific values were chosen and how they affect route ranking.
+// Scoring constants for route ranking algorithm +// Higher scores indicate higher priority in route matching const REQUIRED_PARAM_BASE_SCORE = 0.5 const OPTIONAL_PARAM_BASE_SCORE = 0.4 const WILDCARD_PARAM_BASE_SCORE = 0.25 +// Additional bonuses for params with prefixes/suffixes (more specific patterns) const BOTH_PRESENCE_BASE_SCORE = 0.05 const PREFIX_PRESENCE_BASE_SCORE = 0.02 const SUFFIX_PRESENCE_BASE_SCORE = 0.01 +// Length multipliers provide fine-grained differentiation const PREFIX_LENGTH_SCORE_MULTIPLIER = 0.0002 const SUFFIX_LENGTH_SCORE_MULTIPLIER = 0.0001
97-111: Consider extracting the static-after-param detection logic.The nested loop that checks for static segments after parameters could be extracted into a helper function for better readability and testability.
+function hasStaticSegmentAfter(parsed: ReadonlyArray<Segment>, startIndex: number): boolean { + for (let i = startIndex + 1; i < parsed.length; i++) { + const nextSegment = parsed[i]! + if ( + nextSegment.type === SEGMENT_TYPE_PATHNAME && + nextSegment.value !== '/' + ) { + return true + } + } + return false +} if (baseScore) { // if there is any static segment (that is not an index) after a required / optional param, // we will boost this param so it ranks higher than a required/optional param without a static segment after it // JUST FOR SORTING, NOT FOR MATCHING - for (let i = index + 1; i < parsed.length; i++) { - const nextSegment = parsed[i]! - if ( - nextSegment.type === SEGMENT_TYPE_PATHNAME && - nextSegment.value !== '/' - ) { - hasStaticAfter = true - return handleParam(segment, baseScore + 0.2) - } - } + if (hasStaticSegmentAfter(parsed, index)) { + hasStaticAfter = true + return handleParam(segment, baseScore + 0.2) + } return handleParam(segment, baseScore) }packages/router-core/src/router.ts (2)
942-951: Optional: remove downstreamascasts by typing the destructure.Annotate the destructured result with the exported result type to avoid three separate
asassertions.-import { processRouteTree } from './process-route-tree' +import { processRouteTree, type ProcessRouteTreeResult } from './process-route-tree' @@ - const { routesById, routesByPath, flatRoutes } = processRouteTree({ + const { routesById, routesByPath, flatRoutes }: ProcessRouteTreeResult<TRouteTree> = processRouteTree({ routeTree: this.routeTree, initRoute: (route, i) => { route.init({ originalIndex: i, }) }, }) @@ - this.routesById = routesById as RoutesById<TRouteTree> - this.routesByPath = routesByPath as RoutesByPath<TRouteTree> - this.flatRoutes = flatRoutes as Array<AnyRoute> + this.routesById = routesById + this.routesByPath = routesByPath + this.flatRoutes = flatRoutes
959-963: Nit: avoid magic number for not-found route index.Use
Number.MAX_SAFE_INTEGERfor clarity.- notFoundRoute.init({ - originalIndex: 99999999999, - }) + notFoundRoute.init({ + originalIndex: Number.MAX_SAFE_INTEGER, + })
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
packages/router-core/src/index.ts(1 hunks)packages/router-core/src/process-route-tree.ts(1 hunks)packages/router-core/src/route.ts(1 hunks)packages/router-core/src/router.ts(3 hunks)packages/router-core/tests/processRouteTree.test.ts(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
packages/router-core/src/process-route-tree.ts (2)
packages/router-core/src/route.ts (1)
RouteLike(1753-1764)packages/router-core/src/path.ts (4)
SEGMENT_TYPE_PARAM(7-7)SEGMENT_TYPE_OPTIONAL_PARAM(9-9)SEGMENT_TYPE_WILDCARD(8-8)SEGMENT_TYPE_PATHNAME(6-6)
packages/router-core/tests/processRouteTree.test.ts (1)
packages/router-core/src/route.ts (1)
RouteLike(1753-1764)
🔇 Additional comments (13)
packages/router-core/src/route.ts (1)
1753-1764: LGTM! Well-structured interface for route-like objects.The
RouteLikeinterface provides a clean abstraction for representing routes with just the essential properties needed for route tree processing. The optional properties are appropriately marked, and the interface is well-placed in the route module.packages/router-core/src/index.ts (1)
197-198: LGTM! Clean re-export from the new module.The exports are correctly updated to re-export from the new
process-route-treemodule, maintaining the same public API while improving code organization.packages/router-core/tests/processRouteTree.test.ts (3)
2-5: LGTM! Imports correctly updated to use the new module structure.The test file properly imports
processRouteTreefrom the new module and uses the publicRouteLiketype, aligning with the refactored code organization.
9-12: LGTM! Return type properly updated to use RouteLike.The function signature correctly uses the
RouteLiketype from the public API.
40-40: LGTM! Consistent type usage throughout test helpers.The
createRouteTreefunction also correctly returns theRouteLiketype, maintaining consistency across all test utilities.packages/router-core/src/process-route-tree.ts (5)
23-50: LGTM! Clean parameter scoring logic.The
handleParamfunction elegantly handles different parameter configurations with appropriate scoring adjustments based on prefix/suffix presence and length.
52-174: Well-structured route sorting implementation with comprehensive ranking criteria.The
sortRoutesfunction implements a sophisticated multi-criteria sorting algorithm that handles various edge cases. The segment-by-segment comparison with multiple tie-breakers ensures predictable and intuitive route matching behavior.
176-180: LGTM! Clean type definition for the processing result.The
ProcessRouteTreeResulttype clearly defines the expected output structure with appropriate generic constraints.
182-228: Well-implemented route tree processing with proper error handling.The
processRouteTreefunction correctly:
- Detects duplicate route IDs with clear error messages
- Handles root routes appropriately
- Manages path trimming consistently
- Provides an optional initialization hook for routes
206-213: Confirm/document trailing-slash override behavior.The condition lets index routes (childRoute.fullPath.endsWith('/')) overwrite non-index routes for the same trimmed path — packages/router-core/src/process-route-tree.ts (lines 206–213). The docs describe index-route/trailing-slash semantics and expose a RouterOptions.trailingSlash setting, so this looks intentional. (tanstack.com) There is a reported types vs runtime mismatch around routes-by-path that you may want to address or note in a test/inline comment (#3780). (github.com)
packages/router-core/src/router.ts (3)
32-32: Type-only import is spot on.Keeps runtime clean and tree-shakeable.
56-56: Resolved — RouteLike exposes fullPath, path, parentRoute, and options.caseSensitive.
Interface in packages/router-core/src/route.ts includes fullPath, path?, parentRoute?, and options?.caseSensitive.
15-15: Extraction OK — sorting tie-break uses insertion order, not route.originalIndex.sortRoutes (packages/router-core/src/process-route-tree.ts) falls back to
return a.index - b.indexwhereindexis the position in the array passed (fromObject.values(routesById)); processRouteTree buildsroutesByIdvia depth‑firstrecurseRoutes.router.tsstill callsroute.init({ originalIndex: i }), but thatoriginalIndexproperty is not used by the comparator. Confirm the recursion/insertion order matches the pre‑extraction ordering, or change the comparator to useroute.originalIndexif that was the intended tie‑breaker.
- move `processRouteTree` to a separate file because the logic is very self-contained - create a new function `sortRoutes` w/ the sorting logic, so it can be tested independently This PR is a no-op, we're just moving code around, *nothing* was changed. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - New Features - Added a public Route type for describing route objects in route trees. - Refactor - Consolidated route processing into a dedicated module for clearer separation and maintainability. - Updated public exports for route processing; existing imports may need to be adjusted. - No runtime behavior changes expected. - Tests - Updated tests to use the new public Route type and revised import paths. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
processRouteTreeto a separate file because the logic is very self-containedsortRoutesw/ the sorting logic, so it can be tested independentlyThis PR is a no-op, we're just moving code around, nothing was changed.
Summary by CodeRabbit
New Features
Refactor
Tests