fix(router-core): matching competing optional routes uses proper fullPath for params extraction #6015
+38
−1
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…Path for params extraction
Contributor
WalkthroughAdjusts route-tree assignment logic: node.route replacement now prefers index routes over non-index ones and updates node.fullPath from the chosen route; also removes assignment of node.notFound for non-index leaf routes. Adds tests covering optional root-level index param extraction. Changes
Sequence Diagram(s)(omitted) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ 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)
Comment |
|
View your CI Pipeline Execution ↗ for commit 8c6a211
☁️ Nx Cloud last updated this comment at |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/router-core/tests/new-process-route-tree.test.ts (1)
834-864: Good test coverage for issue #6012.The test correctly validates that when multiple optional routes compete at the root level, the index route takes precedence and uses the proper fullPath for parameter extraction.
Consider adding complementary test cases to ensure robustness:
- Test when the non-index route should win (e.g., matching
/2024/12/02should extract year/month/day, not language)- Test with different ordering of route definitions to verify precedence is consistent
- Test when both competing routes are index routes or both are non-index routes
These additional cases would help prevent regressions in the precedence logic.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/router-core/src/new-process-route-tree.ts(2 hunks)packages/router-core/tests/new-process-route-tree.test.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use TypeScript strict mode with extensive type safety throughout the codebase
Files:
packages/router-core/src/new-process-route-tree.tspackages/router-core/tests/new-process-route-tree.test.ts
🧠 Learnings (6)
📓 Common learnings
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.
📚 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.tspackages/router-core/tests/new-process-route-tree.test.ts
📚 Learning: 2025-11-25T00:18:21.282Z
Learnt from: CR
Repo: TanStack/router PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T00:18:21.282Z
Learning: Applies to **/src/routes/**/*.{ts,tsx} : Use file-based routing in src/routes/ directories or code-based routing with route definitions
Applied to files:
packages/router-core/src/new-process-route-tree.tspackages/router-core/tests/new-process-route-tree.test.ts
📚 Learning: 2025-11-25T00:18:21.282Z
Learnt from: CR
Repo: TanStack/router PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T00:18:21.282Z
Learning: Applies to packages/solid-router/**/*.{ts,tsx} : Solid Router components and primitives should use the tanstack/solid-router package
Applied to files:
packages/router-core/src/new-process-route-tree.tspackages/router-core/tests/new-process-route-tree.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/new-process-route-tree.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/new-process-route-tree.test.ts
🧬 Code graph analysis (1)
packages/router-core/tests/new-process-route-tree.test.ts (1)
packages/router-core/src/new-process-route-tree.ts (2)
processRouteTree(651-705)findRouteMatch(618-640)
⏰ 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 (1)
packages/router-core/src/new-process-route-tree.ts (1)
335-341: Index route precedence fix for issue #6012 is correct and well-implemented.The conditional logic
!node.route || (!node.isIndex && isIndex)correctly prioritizes index routes over non-index routes when assigning routes to leaf nodes. The code comment explicitly documents this precedence rule. The correspondingfullPathupdate ensures parameter extraction uses the correct route definition.The test coverage at line 835 validates the key scenario. Index route precedence is tested across multiple test cases (lines 130, 233, 513), confirming the implementation handles the intended behavior correctly.
Contributor
Author
|
Fix is confirmed 
|
Sheraff
deleted the
fix-router-core-competing-optional-routes-use-proper-fullpath
branch
enBonnet
pushed a commit
to enBonnet/router
that referenced
this pull request
Dec 6, 2025
…Path for params extraction (TanStack#6015) * fix(router-core): matching competing optional routes uses proper fullPath for params extraction * remove debug console.log
This was referenced Dec 21, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #6012 (maybe)
Summary by CodeRabbit
Bug Fixes
Tests
✏️ Tip: You can customize this high-level summary in your review settings.