fix(router-core): handle wildcard nodes like any other index node #5972
+23
−8
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
Contributor
WalkthroughThis PR refactors wildcard segment handling in the route tree matching algorithm. It replaces a temporary wildcardMatch variable with stack-based exploration, adjusts index-route handling for wildcard nodes, and reorganizes the getNodeMatch control flow. Corresponding test coverage validates that trailing wildcard routes match expected URL paths, addressing issue Changes
Sequence Diagram(s)sequenceDiagram
participant Stack as Exploration Stack
participant Node as Route Node
participant Match as Match Result
rect rgb(240, 248, 255)
Note over Stack,Match: Old Approach (wildcardMatch variable)
Stack->>Stack: Create wildcardMatch temp
Stack->>Node: Evaluate current segment
alt Static/Dynamic match
Node->>Match: Return early if bestMatch
else Wildcard exists
Stack->>Stack: Update wildcardMatch
Stack->>Match: Return wildcardMatch on failure
end
end
rect rgb(240, 250, 240)
Note over Stack,Match: New Approach (Stack-based)
Stack->>Stack: Process frame from stack
Stack->>Node: Evaluate segment
alt Static/Dynamic match
Node->>Stack: Enqueue matching frame
else Wildcard exists
Node->>Stack: Push wildcard frame (index=partsLength, depth+1)
end
Stack->>Stack: Continue until stack empty
Stack->>Match: Return best match found
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
|
View your CI Pipeline Execution ↗ for commit c36a8e0
☁️ 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: 0
🧹 Nitpick comments (1)
packages/router-core/tests/new-process-route-tree.test.ts (1)
549-565: Good test coverage for the regression fix.These tests properly verify the expected behavior for issue #5969:
- Basic case: Trailing wildcard matches paths with/without trailing content
- Layout route interaction: Wildcard takes precedence over non-index layout route
- Index route precedence: Actual index routes still win over wildcards
The test naming "should not match" on line 560 might be slightly confusing—consider clarifying that it's the wildcard that should not match (the index route wins).
- it('with index route (should not match)', () => { + it('with index route (index wins over wildcard)', () => {
📜 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(3 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 (7)
📓 Common learnings
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.
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: 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: CR
Repo: TanStack/router PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T00:18:21.258Z
Learning: Applies to packages/solid-router/**/*.{ts,tsx} : Solid Router components and primitives should use the tanstack/solid-router package
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: 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.
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.
📚 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.tspackages/router-core/tests/new-process-route-tree.test.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.tspackages/router-core/tests/new-process-route-tree.test.ts
📚 Learning: 2025-11-25T00:18:21.258Z
Learnt from: CR
Repo: TanStack/router PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T00:18:21.258Z
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.258Z
Learnt from: CR
Repo: TanStack/router PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T00:18:21.258Z
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: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/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 (1)
findRouteMatch(613-635)
⏰ 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 (2)
packages/router-core/src/new-process-route-tree.ts (2)
323-323: LGTM — Key fix for issue #5969.Setting
isIndex = truefor wildcard nodes allows trailing splat routes (e.g.,/a/$) to match paths both with and without trailing content (e.g.,/a/and/a). This restores the behavior from v1.136.4.The precedence logic in
getNodeMatchensures actual index routes still win via the early return at line 881 (statics === partsLength && node.isIndex).
891-919: Refactor to stack-based wildcard exploration looks correct.The change from a dedicated
wildcardMatchvariable to stack-based exploration is a clean simplification:
index: partsLengthcorrectly signals that the wildcard consumes the remaining pathdepth: depth + 1properly tracks the wildcard node depth forisFrameMoreSpecificcomparison- The
breakafter pushing ensures only the first (highest priority) wildcard is consideredThis aligns with the existing approach for other segment types and maintains correct prioritization.
Sheraff
deleted the
fix-router-core-wildcard-node-is-regular-index-node
branch
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 #5969
This implementation, however elegant, would fails this test:
We're going to go w/ #5971 instead
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.