Skip to content

Conversation

@Sheraff
Copy link
Contributor

@Sheraff Sheraff commented Dec 21, 2025

Following #6171, it turns out the new index nodes are terminating: they don't need to push a frame onto the stack to walk the children, either they match or they don't.

This makes them slightly less costly to match.

Benchmark without cache: still slightly slower than w/o index nodes, but also consistently better than when pushing index node frames onto the stack.

 ✓  @tanstack/router-core  tests/foo.bench.ts > matching 1832ms
     name                            hz     min     max    mean     p75     p99    p995    p999     rme  samples
   · new processRouteTree  2,523,143.15  0.0003  0.0875  0.0004  0.0004  0.0005  0.0005  0.0006  ±0.06%  1261572
   · old processRouteTree  2,616,121.15  0.0003  0.1371  0.0004  0.0004  0.0005  0.0005  0.0006  ±0.10%  1308061

 BENCH  Summary

   @tanstack/router-core  old processRouteTree - tests/foo.bench.ts > matching
    1.04x faster than new processRouteTree

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Dec 21, 2025

Walkthrough

The change refines route tree matching logic by simplifying early-match conditions, adding explicit index match handling with early termination for perfect matches, adjusting wildcard index behavior, and removing a previous stack-based index match exploration path.

Changes

Cohort / File(s) Change Summary
Route tree matching logic refinement
packages/router-core/src/new-process-route-tree.ts
Simplifies early-match logic to only update bestMatch for non-index paths beyond input when frame is more specific; adds explicit index match handling with early return for perfect matches; adjusts wildcard index behavior; removes previous block that pushed index matches onto stack.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

  • Route matching logic complexity: The changes affect core matching behavior and specificity comparisons that require careful tracing of control flow
  • Edge case sensitivity: Modifications to index match handling and early termination conditions could affect matching behavior in subtle ways—should verify against test cases
  • Priority and fallback order: Changes to when bestMatch is updated could alter route resolution priority and tie-breaking behavior

Possibly related PRs

  • TanStack/router#5933: Both PRs modify the same file and refine how index routes and match specificity are handled with early-return and tie-breaker logic
  • TanStack/router#5128: Related route-tree processing logic modifications that established the foundation for these matching behavior adjustments

Suggested labels

package: router-core

Suggested reviewers

  • schiller-manuel

Poem

🐰 Hoppy routes now match with grace,
Index paths find their rightful place,
No more stacks to chase and weave,
Just early returns—believe, believe!
Wildcards skip with bounds so neat,
Route specificity can't be beat!

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 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 refactoring: index nodes skipping the stack loop because they cannot have children, which aligns with the core changes involving simplified index handling and removal of previous stack exploration.
✨ 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-terminating-nodes-skip-stack-frames

📜 Recent review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between a33a295 and 82b4c14.

📒 Files selected for processing (1)
  • packages/router-core/src/new-process-route-tree.ts (3 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

Use TypeScript strict mode with extensive type safety for all code

Files:

  • packages/router-core/src/new-process-route-tree.ts
**/*.{js,ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

Implement ESLint rules for router best practices using the ESLint plugin router

Files:

  • packages/router-core/src/new-process-route-tree.ts
🧠 Learnings (7)
📓 Common learnings
Learnt from: Sheraff
Repo: TanStack/router PR: 6171
File: packages/router-core/src/new-process-route-tree.ts:898-898
Timestamp: 2025-12-21T12:52:27.078Z
Learning: In `packages/router-core/src/new-process-route-tree.ts`, the matching logic intentionally allows paths without trailing slashes to match index routes with trailing slashes (e.g., `/a` can match `/a/` route), but not vice-versa (e.g., `/a/` cannot match `/a` layout route). This is implemented via the condition `!pathIsIndex || node.kind === SEGMENT_TYPE_INDEX` and is a deliberate design decision to provide better UX by being permissive with missing trailing slashes.
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.
📚 Learning: 2025-12-21T12:52:27.078Z
Learnt from: Sheraff
Repo: TanStack/router PR: 6171
File: packages/router-core/src/new-process-route-tree.ts:898-898
Timestamp: 2025-12-21T12:52:27.078Z
Learning: In `packages/router-core/src/new-process-route-tree.ts`, the matching logic intentionally allows paths without trailing slashes to match index routes with trailing slashes (e.g., `/a` can match `/a/` route), but not vice-versa (e.g., `/a/` cannot match `/a` layout route). This is implemented via the condition `!pathIsIndex || node.kind === SEGMENT_TYPE_INDEX` and is a deliberate design decision to provide better UX by being permissive with missing trailing slashes.

Applied to files:

  • packages/router-core/src/new-process-route-tree.ts
📚 Learning: 2025-12-17T02:17:55.086Z
Learnt from: schiller-manuel
Repo: TanStack/router PR: 6120
File: packages/router-generator/src/generator.ts:654-657
Timestamp: 2025-12-17T02:17:55.086Z
Learning: In `packages/router-generator/src/generator.ts`, pathless_layout routes must receive a `path` property when they have a `cleanedPath`, even though they are non-path routes. This is necessary because child routes inherit the path from their parent, and without this property, child routes would not have the correct full path at runtime.

Applied to files:

  • packages/router-core/src/new-process-route-tree.ts
📚 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/new-process-route-tree.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/new-process-route-tree.ts
📚 Learning: 2025-12-06T15:03:07.223Z
Learnt from: CR
Repo: TanStack/router PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-06T15:03:07.223Z
Learning: Applies to **/*.{js,ts,tsx} : Implement ESLint rules for router best practices using the ESLint plugin router

Applied to files:

  • packages/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/src/new-process-route-tree.ts
⏰ 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: Preview
  • GitHub Check: Test
🔇 Additional comments (3)
packages/router-core/src/new-process-route-tree.ts (3)

898-900: LGTM - Preserves intended trailing slash behavior.

The condition !pathIsIndex correctly prevents paths with trailing slashes from matching layout routes while allowing index routes (handled separately at lines 924-926). This maintains the asymmetric matching behavior where /a can match /a/ but not vice-versa.

Based on learnings, this is the intended behavior for better UX with missing trailing slashes.


908-928: Excellent optimization for index route matching.

The explicit index handling with early termination is well-reasoned:

  1. Early return (lines 921-923): The perfect match condition correctly identifies cases where no better match exists (all segments matched statically with no dynamics/optionals/skipped), enabling immediate return.

  2. Skipping stack exploration: Index nodes are terminal by construction (only created for leaf routes at lines 342-352), so pushing them to the stack for further exploration would be wasteful.

  3. Priority preservation: The isFrameMoreSpecific check ensures index routes maintain their highest-priority status.

The self-documenting comment about the optimization is helpful.


948-951: Consistent treatment of terminal nodes.

Setting index: partsLength for wildcard matches is correct since wildcards consume all remaining path segments. The comment clearly explains that wildcards, like index nodes, are terminal and don't need stack-based exploration.

This refactoring improves performance by avoiding unnecessary stack operations for nodes that cannot have children.


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

@nx-cloud
Copy link

nx-cloud bot commented Dec 21, 2025

View your CI Pipeline Execution ↗ for commit 82b4c14

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

☁️ Nx Cloud last updated this comment at 2025-12-21 15:57:35 UTC

@pkg-pr-new
Copy link

pkg-pr-new bot commented Dec 21, 2025

More templates

@tanstack/arktype-adapter

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

@tanstack/directive-functions-plugin

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

@tanstack/eslint-plugin-router

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

@tanstack/history

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

@tanstack/nitro-v2-vite-plugin

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

@tanstack/react-router

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

@tanstack/react-router-devtools

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

@tanstack/react-router-ssr-query

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

@tanstack/react-start

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

@tanstack/react-start-client

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

@tanstack/react-start-server

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

@tanstack/router-cli

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

@tanstack/router-core

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

@tanstack/router-devtools

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

@tanstack/router-devtools-core

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

@tanstack/router-generator

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

@tanstack/router-plugin

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

@tanstack/router-ssr-query-core

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

@tanstack/router-utils

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

@tanstack/router-vite-plugin

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

@tanstack/server-functions-plugin

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

@tanstack/solid-router

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

@tanstack/solid-router-devtools

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

@tanstack/solid-router-ssr-query

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

@tanstack/solid-start

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

@tanstack/solid-start-client

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

@tanstack/solid-start-server

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

@tanstack/start-client-core

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

@tanstack/start-plugin-core

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

@tanstack/start-server-core

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

@tanstack/start-static-server-functions

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

@tanstack/start-storage-context

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

@tanstack/valibot-adapter

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

@tanstack/virtual-file-routes

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

@tanstack/vue-router

npm i https://pkg.pr.new/TanStack/router/@tanstack/vue-router@6173

@tanstack/vue-router-devtools

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

@tanstack/vue-router-ssr-query

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

@tanstack/vue-start

npm i https://pkg.pr.new/TanStack/router/@tanstack/vue-start@6173

@tanstack/vue-start-client

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

@tanstack/vue-start-server

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

@tanstack/zod-adapter

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

commit: 82b4c14

@Sheraff Sheraff merged commit 7a3def0 into main Dec 21, 2025
6 checks passed
@Sheraff Sheraff deleted the refactor-router-core-terminating-nodes-skip-stack-frames branch December 21, 2025 16:01
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