Skip to content

Conversation

@Sheraff
Copy link
Contributor

@Sheraff Sheraff commented Nov 16, 2025

Summary by CodeRabbit

  • Refactor
    • Route matching now includes route ancestry (branch) with each match, simplifying how matched routes are built.
    • Simplified traversal: matched routes are derived directly from branch data rather than walking parent links.
    • Immutable handling: matched route lists are treated as readonly and extended immutably (no in-place mutation).

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 16, 2025

Walkthrough

A new internal RouteMatch<T> type (including a branch array) replaces the prior implicit match result; the match cache and findRouteMatch now use RouteMatch<T> | null. buildRouteBranch creates the ancestry chain. Router logic now reads matched routes from match.branch instead of manually traversing parents.

Changes

Cohort / File(s) Summary
RouteMatch type and matching internals
packages/router-core/src/new-process-route-tree.ts
Added RouteMatch<T> (route, params, branch); updated matchCache to `LRUCache<string, RouteMatch
Matched routes derivation simplification
packages/router-core/src/router.ts
Replaced parent-traversal + reversal with direct use of match?.branch (or root fallback) to produce a ReadonlyArray<AnyRoute>; updated call sites to use immutable spreads when appending notFoundRoute.

Sequence Diagram(s)

sequenceDiagram
  participant Router
  participant ProcessedTree
  participant MatchCache
  participant findRouteMatch
  participant buildRouteBranch

  Router->>ProcessedTree: request match for path (fuzzy?)
  ProcessedTree->>MatchCache: lookup key
  alt cache hit (RouteMatch)
    MatchCache-->>Router: RouteMatch (contains branch)
  else cache miss
    Router->>findRouteMatch: compute match
    findRouteMatch->>buildRouteBranch: build branch from leaf->root
    buildRouteBranch-->>findRouteMatch: branch (root->...->leaf)
    findRouteMatch->>MatchCache: store RouteMatch | null
    findRouteMatch-->>Router: RouteMatch | null
  end
  Router->>Router: derive matchedRoutes = match?.branch ?? [root]
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

  • Review type changes where RouteMatch propagates (new generic constraints).
  • Verify buildRouteBranch correctness and ordering.
  • Confirm cache key construction and null vs undefined semantics for fuzzy/non-fuzzy cases.
  • Ensure router usages of ReadonlyArray and immutable updates preserve previous behavior.

Possibly related PRs

Suggested reviewers

  • schiller-manuel
  • nlynzaad

Poem

🐇 I hop from branch to branch with glee,

Each path now lists its ancestry,
No more loops to chase the light,
RouteMatch keeps the trails in sight. 🥕

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: caching the matched route branch as part of the route match in router-core.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch refactor-router-core-cache-reconstructed-route-branch

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between da014c1 and 8030232.

📒 Files selected for processing (2)
  • packages/router-core/src/new-process-route-tree.ts (5 hunks)
  • packages/router-core/src/router.ts (4 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • packages/router-core/src/new-process-route-tree.ts
🧰 Additional context used
🧠 Learnings (4)
📚 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/src/router.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-core/src/router.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/src/router.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/src/router.ts
🧬 Code graph analysis (1)
packages/router-core/src/router.ts (2)
packages/router-core/src/route.ts (1)
  • AnyRoute (778-797)
packages/router-core/src/root.ts (1)
  • rootRouteId (2-2)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: Test
  • GitHub Check: Preview
🔇 Additional comments (2)
packages/router-core/src/router.ts (2)

700-700: Good defensive type change.

Changing to ReadonlyArray prevents accidental mutations at compile time and signals immutability intent to callers.


1251-1266: LGTM - immutable spread prevents cache mutation.

The destructuring and subsequent spread operation correctly creates a new array when appending notFoundRoute, preventing mutation of the cached branch array.


Comment @coderabbitai help to get the list of available commands and usage tips.

@nx-cloud
Copy link

nx-cloud bot commented Nov 16, 2025

View your CI Pipeline Execution ↗ for commit 8030232

Command Status Duration Result
nx affected --targets=test:eslint,test:unit,tes... ✅ Succeeded 7m 46s View ↗
nx run-many --target=build --exclude=examples/*... ✅ Succeeded 1m 23s View ↗

☁️ Nx Cloud last updated this comment at 2025-11-16 17:26:34 UTC

@pkg-pr-new
Copy link

pkg-pr-new bot commented Nov 16, 2025

More templates

@tanstack/arktype-adapter

npm i https://pkg.pr.new/TanStack/router/@tanstack/arktype-adapter@5882

@tanstack/directive-functions-plugin

npm i https://pkg.pr.new/TanStack/router/@tanstack/directive-functions-plugin@5882

@tanstack/eslint-plugin-router

npm i https://pkg.pr.new/TanStack/router/@tanstack/eslint-plugin-router@5882

@tanstack/history

npm i https://pkg.pr.new/TanStack/router/@tanstack/history@5882

@tanstack/nitro-v2-vite-plugin

npm i https://pkg.pr.new/TanStack/router/@tanstack/nitro-v2-vite-plugin@5882

@tanstack/react-router

npm i https://pkg.pr.new/TanStack/router/@tanstack/react-router@5882

@tanstack/react-router-devtools

npm i https://pkg.pr.new/TanStack/router/@tanstack/react-router-devtools@5882

@tanstack/react-router-ssr-query

npm i https://pkg.pr.new/TanStack/router/@tanstack/react-router-ssr-query@5882

@tanstack/react-start

npm i https://pkg.pr.new/TanStack/router/@tanstack/react-start@5882

@tanstack/react-start-client

npm i https://pkg.pr.new/TanStack/router/@tanstack/react-start-client@5882

@tanstack/react-start-server

npm i https://pkg.pr.new/TanStack/router/@tanstack/react-start-server@5882

@tanstack/router-cli

npm i https://pkg.pr.new/TanStack/router/@tanstack/router-cli@5882

@tanstack/router-core

npm i https://pkg.pr.new/TanStack/router/@tanstack/router-core@5882

@tanstack/router-devtools

npm i https://pkg.pr.new/TanStack/router/@tanstack/router-devtools@5882

@tanstack/router-devtools-core

npm i https://pkg.pr.new/TanStack/router/@tanstack/router-devtools-core@5882

@tanstack/router-generator

npm i https://pkg.pr.new/TanStack/router/@tanstack/router-generator@5882

@tanstack/router-plugin

npm i https://pkg.pr.new/TanStack/router/@tanstack/router-plugin@5882

@tanstack/router-ssr-query-core

npm i https://pkg.pr.new/TanStack/router/@tanstack/router-ssr-query-core@5882

@tanstack/router-utils

npm i https://pkg.pr.new/TanStack/router/@tanstack/router-utils@5882

@tanstack/router-vite-plugin

npm i https://pkg.pr.new/TanStack/router/@tanstack/router-vite-plugin@5882

@tanstack/server-functions-plugin

npm i https://pkg.pr.new/TanStack/router/@tanstack/server-functions-plugin@5882

@tanstack/solid-router

npm i https://pkg.pr.new/TanStack/router/@tanstack/solid-router@5882

@tanstack/solid-router-devtools

npm i https://pkg.pr.new/TanStack/router/@tanstack/solid-router-devtools@5882

@tanstack/solid-router-ssr-query

npm i https://pkg.pr.new/TanStack/router/@tanstack/solid-router-ssr-query@5882

@tanstack/solid-start

npm i https://pkg.pr.new/TanStack/router/@tanstack/solid-start@5882

@tanstack/solid-start-client

npm i https://pkg.pr.new/TanStack/router/@tanstack/solid-start-client@5882

@tanstack/solid-start-server

npm i https://pkg.pr.new/TanStack/router/@tanstack/solid-start-server@5882

@tanstack/start-client-core

npm i https://pkg.pr.new/TanStack/router/@tanstack/start-client-core@5882

@tanstack/start-plugin-core

npm i https://pkg.pr.new/TanStack/router/@tanstack/start-plugin-core@5882

@tanstack/start-server-core

npm i https://pkg.pr.new/TanStack/router/@tanstack/start-server-core@5882

@tanstack/start-static-server-functions

npm i https://pkg.pr.new/TanStack/router/@tanstack/start-static-server-functions@5882

@tanstack/start-storage-context

npm i https://pkg.pr.new/TanStack/router/@tanstack/start-storage-context@5882

@tanstack/valibot-adapter

npm i https://pkg.pr.new/TanStack/router/@tanstack/valibot-adapter@5882

@tanstack/virtual-file-routes

npm i https://pkg.pr.new/TanStack/router/@tanstack/virtual-file-routes@5882

@tanstack/zod-adapter

npm i https://pkg.pr.new/TanStack/router/@tanstack/zod-adapter@5882

commit: 8030232

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 40b7576 and da014c1.

📒 Files selected for processing (2)
  • packages/router-core/src/new-process-route-tree.ts (5 hunks)
  • packages/router-core/src/router.ts (1 hunks)
🧰 Additional context used
🧠 Learnings (3)
📚 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/src/router.ts
  • packages/router-core/src/new-process-route-tree.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/src/router.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-core/src/router.ts
  • packages/router-core/src/new-process-route-tree.ts
🧬 Code graph analysis (2)
packages/router-core/src/router.ts (1)
packages/router-core/src/root.ts (1)
  • rootRouteId (2-2)
packages/router-core/src/new-process-route-tree.ts (3)
packages/router-core/src/lru-cache.ts (2)
  • LRUCache (1-5)
  • createLRUCache (7-74)
packages/router-core/src/Matches.ts (1)
  • RouteMatch (118-175)
packages/router-core/src/route.ts (2)
  • RouteLike (1968-1979)
  • path (1551-1553)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: Test
  • GitHub Check: Preview

@Sheraff Sheraff merged commit dbc9cfe into main Nov 16, 2025
6 checks passed
@Sheraff Sheraff deleted the refactor-router-core-cache-reconstructed-route-branch branch November 16, 2025 18:13
roduyemi pushed a commit to roduyemi/oss-router that referenced this pull request Nov 19, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants