feat(router-core): validate params while matching #5936
Conversation
📝 WalkthroughWalkthroughAdds per-route parameter parsing: Changes
Sequence DiagramsequenceDiagram
participant Orchestrator as Router (match orchestrator)
participant Tree as Segment Tree
participant Node as Segment Node
participant Parser as Route params.parse
participant Result as Match Result
Orchestrator->>Tree: begin traversal for path
Tree->>Node: examine candidate node
alt node has parse
Node-->>Orchestrator: parse present on leaf
Orchestrator->>Orchestrator: run extractParams (early) -> [params,state]
Orchestrator->>Parser: call parse(extractedParams)
alt parse succeeds
Parser-->>Orchestrator: parsed params
Orchestrator->>Orchestrator: attach params to frame
Orchestrator->>Tree: continue traversal with parsed params
Tree-->>Result: produce match
else parse throws
Parser-->>Orchestrator: error
Orchestrator->>Orchestrator: prune this path
Orchestrator->>Tree: try alternate nodes/routes
end
else no parse
Node-->>Orchestrator: no parse
Orchestrator->>Tree: continue standard matching
Tree-->>Result: produce match
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
Comment |
Sheraff
changed the title
|
View your CI Pipeline Execution ↗ for commit 40b9428
☁️ Nx Cloud last updated this comment at |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/router-core/src/new-process-route-tree.ts (1)
917-919: Silent error handling may hinder debugging.Parse errors are caught and silently cause the route to be skipped via
continue. While this enables fallback to alternative routes, it provides no visibility into why a route was rejected, making debugging difficult for developers.Consider adding optional logging or a debug mode that reports parse failures, especially during development.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/router-core/src/new-process-route-tree.ts(21 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
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.
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/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.
📚 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
⏰ 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 (4)
packages/router-core/src/new-process-route-tree.ts (4)
375-376: Verify parse prioritization is always desired.The sort function unconditionally prioritizes nodes with parse functions over those without. While this makes sense for validation (try more specific routes first), it's hardcoded with no way to customize priority.
Consider whether this ordering is always appropriate, especially when multiple routes with different parse functions compete. The current tie-breaking logic (lines 377-394) would then determine order, which might be arbitrary.
236-244: Node reuse prevented when parse is present.Routes with parse functions cannot share segment nodes, even if they're otherwise identical. This conservative approach is safe (prevents incorrect sharing of different parse functions), but increases memory usage if many routes have parse functions.
This is likely acceptable for correctness, but worth noting for performance-sensitive applications with large route trees.
Also applies to: 278-286
765-767: LGTM: Param extraction correctly resumes from saved state.The logic properly handles incremental param extraction:
- Resumes from
leaf.extractstate if present (line 765-767)- Extracts params from remaining nodes
- Merges previously extracted params back in (line 823)
This avoids re-extracting and re-validating params when multiple parse functions exist in the route path.
Also applies to: 823-824
534-547: LGTM: Type definitions are consistent with implementation.The parse field is properly typed across
SegmentNode,RouteLike, andMatchStackFrame. The function signature(params: Record<string, string>) => anyaccurately reflects that params are strings and the return type is currently unused.
✅ Actions performedReview triggered.
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
docs/router/framework/react/api/router/RouteOptionsType.mddocs/router/framework/react/guide/path-params.md
🧰 Additional context used
📓 Path-based instructions (1)
docs/**/*.{md,mdx}
📄 CodeRabbit inference engine (AGENTS.md)
Use relative links to
docs/folder format (e.g.,./guide/data-loading) for internal documentation references
Files:
docs/router/framework/react/api/router/RouteOptionsType.mddocs/router/framework/react/guide/path-params.md
🧠 Learnings (9)
📓 Common learnings
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.
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: Sheraff
Repo: TanStack/router PR: 6171
File: packages/router-core/src/new-process-route-tree.ts:898-898
Timestamp: 2025-12-21T12:52:35.231Z
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: 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: CR
Repo: TanStack/router PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-06T15:03:07.223Z
Learning: Implement type-safe routing with search params and path params
📚 Learning: 2025-12-21T12:52:35.231Z
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:35.231Z
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:
docs/router/framework/react/guide/path-params.md
📚 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:
docs/router/framework/react/guide/path-params.md
📚 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:
docs/router/framework/react/guide/path-params.md
📚 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: Implement type-safe routing with search params and path params
Applied to files:
docs/router/framework/react/guide/path-params.md
📚 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:
docs/router/framework/react/guide/path-params.md
📚 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:
docs/router/framework/react/guide/path-params.md
📚 Learning: 2025-09-22T00:56:53.426Z
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.
Applied to files:
docs/router/framework/react/guide/path-params.md
📚 Learning: 2025-09-22T00:56:49.237Z
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.
Applied to files:
docs/router/framework/react/guide/path-params.md
🪛 markdownlint-cli2 (0.18.1)
docs/router/framework/react/guide/path-params.md
745-745: Blank line inside blockquote
(MD028, no-blanks-blockquote)
⏰ 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). (1)
- GitHub Check: Test
🔇 Additional comments (2)
docs/router/framework/react/api/router/RouteOptionsType.md (1)
91-110: Well-documented experimental feature with clear warnings. The newskipRouteOnParseErrorproperty is properly documented with type definition, experimental warning, and performance callout. The cross-reference to the comprehensive guide section is helpful. The documentation aligns with the related PR changes in route matching behavior.docs/router/framework/react/guide/path-params.md (1)
741-1037: Excellent comprehensive documentation for the new feature. The "Validating Path Parameters During Matching" section is thorough and well-structured, with clear progression from basic concepts to advanced use cases. The examples covering UUIDs, dates, numeric IDs, priorities, nested routes, and optional parameters demonstrate practical real-world scenarios. The explanation of route priority and custom priority handling is particularly valuable. The integration with RouterOptions for allowed characters is practical. This documentation provides clear guidance on when and how to use this experimental feature while appropriately warning about performance implications.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/router-core/src/route.ts
🧰 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/route.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/route.ts
🧠 Learnings (5)
📓 Common learnings
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.
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-12-06T15:03:07.223Z
Learning: Implement type-safe routing with search params and path params
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:35.231Z
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.
📚 Learning: 2025-12-21T12:52:35.231Z
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:35.231Z
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/route.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/route.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/route.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/route.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: Test
- GitHub Check: Preview
🔇 Additional comments (2)
packages/router-core/src/route.ts (2)
1218-1223: LGTM: Documentation improvement for caseSensitive.Good addition of JSDoc for the existing
caseSensitiveoption. The documentation is clear and follows the project's documentation style.
1224-1227: LGTM: Documentation improvement for wrapInSuspense.Good addition of JSDoc for the existing
wrapInSuspenseoption. The documentation clearly explains the behavior.
| /** | ||
| * Options to control route matching behavior with runtime code. | ||
| */ | ||
| skipRouteOnParseError?: { | ||
| /** | ||
| * If `true`, skip this route during matching if `params.parse` fails. | ||
| * | ||
| * Without this option, a `/$param` route could match *any* value for `param`, | ||
| * and only later during the route lifecycle would `params.parse` run and potentially | ||
| * show the `errorComponent` if validation failed. | ||
| * | ||
| * With this option enabled, the route will only match if `params.parse` succeeds. | ||
| * If it fails, the router will continue trying to match other routes, potentially | ||
| * finding a different route that works, or ultimately showing the `notFoundComponent`. | ||
| * | ||
| * @default false | ||
| */ | ||
| params?: boolean | ||
| /** | ||
| * In cases where multiple routes would need to run `params.parse` during matching | ||
| * to determine which route to pick, this priority number can be used as a tie-breaker | ||
| * for which route to try first. Higher number = higher priority. | ||
| * | ||
| * @default 0 | ||
| */ | ||
| priority?: number | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, check the RouteLike interface definition and its options field
rg -n "interface RouteLike" --type=ts packages/router-core/src/
# Then view the specific lines in route.ts to see the RouteLike definition
sed -n '2003,2014p' packages/router-core/src/route.tsRepository: TanStack/router
Length of output: 342
🏁 Script executed:
# Search for skipRouteOnParseError usage throughout the codebase
rg -n "skipRouteOnParseError" --type=ts packages/router-core/src/
# Check where RouteLike is used in matching logic
rg -n "RouteLike" --type=ts -A3 packages/router-core/src/ | head -100Repository: TanStack/router
Length of output: 7940
🏁 Script executed:
# Look for test files related to skipRouteOnParseError
find packages -name "*.test.ts" -o -name "*.spec.ts" | xargs rg -l "skipRouteOnParseError" 2>/dev/null || echo "No test files found with skipRouteOnParseError"
# Check for any @experimental tags in the route.ts file
grep -n "@experimental\|@alpha\|experimental" packages/router-core/src/route.ts | head -20Repository: TanStack/router
Length of output: 216
🏁 Script executed:
# Check the route.ts file to see if there are other interfaces besides RouteLike
# and understand the complete type hierarchy
grep -n "interface.*Route\|type.*Route" packages/router-core/src/route.ts | head -30
# Look at the UpdatableRouteOptions interface mentioned in the review
rg -n "UpdatableRouteOptions" --type=ts packages/router-core/src/route.ts -A20Repository: TanStack/router
Length of output: 4834
🏁 Script executed:
# Check the test file to verify if tests exist
head -50 packages/router-core/tests/skip-route-on-parse-error.test.ts
# Check how parseSegments is called and what route object is passed
rg -n "parseSegments" --type=ts packages/router-core/src/ -B2 -A2Repository: TanStack/router
Length of output: 3893
🏁 Script executed:
# Check if there's a type declaration or wider options interface used elsewhere
rg -n "route\.options\?" --type=ts packages/router-core/src/new-process-route-tree.ts -B2 -A2 | head -50Repository: TanStack/router
Length of output: 650
🏁 Script executed:
# Check if Route extends RouteLike or what the relationship is
rg -n "class Route\|extends.*Route\|implements.*RouteLike" --type=ts packages/router-core/src/route.ts -A3 -B1 | head -40
# Check the full test file to see test coverage quality
wc -l packages/router-core/tests/skip-route-on-parse-error.test.ts
# Check how RouteLike is used - is it for serialization/export?
rg -n "RouteLike" --type=ts packages/router-core/src/ -B2 -A2 | grep -E "export|JSON|serialize|toJSON" | head -20Repository: TanStack/router
Length of output: 778
🏁 Script executed:
# Check the processRouteTree function to see what it accepts and returns
rg -n "function processRouteTree\|export.*processRouteTree" --type=ts packages/router-core/src/ -A10Repository: TanStack/router
Length of output: 41
🏁 Script executed:
# Get the processRouteTree function signature and implementation
sed -n '755,790p' packages/router-core/src/new-process-route-tree.tsRepository: TanStack/router
Length of output: 1590
🏁 Script executed:
# Check what the test file passes as tree parameter to processRouteTree
grep -n "processRouteTree" packages/router-core/tests/skip-route-on-parse-error.test.ts -A5 | head -30Repository: TanStack/router
Length of output: 1422
🏁 Script executed:
# Check Route class definition to understand relationship to RouteLike
sed -n '586,650p' packages/router-core/src/route.tsRepository: TanStack/router
Length of output: 1417
🏁 Script executed:
# Search for @experimental or @alpha markers in the entire file
grep -n "@experimental\|@alpha\|experimental\|alpha" packages/router-core/src/route.tsRepository: TanStack/router
Length of output: 41
🏁 Script executed:
# Check if @experimental/@alpha tags are used elsewhere in the codebase
rg -n "@experimental|@alpha" --type=ts packages/router-core/src/ | head -20Repository: TanStack/router
Length of output: 114
🏁 Script executed:
# Check Route vs RouteLike - see if they're related or separate concerns
# Look at RouteOptions to see if it extends UpdatableRouteOptions
rg -n "type RouteOptions\|interface RouteOptions" --type=ts packages/router-core/src/route.ts -A20 | head -40Repository: TanStack/router
Length of output: 41
🏁 Script executed:
# Check the test more thoroughly - see if it's testing real routes or just mock objects
tail -100 packages/router-core/tests/skip-route-on-parse-error.test.tsRepository: TanStack/router
Length of output: 2824
Update RouteLike interface and add @experimental tag for type safety and consistency.
The type structure and documentation for skipRouteOnParseError are sound and well-implemented. However, two improvements are needed:
-
RouteLike interface compatibility: Update the
RouteLikeinterface (lines 2011-2013) to exposeskipRouteOnParseErrorin its options field:options?: { caseSensitive?: boolean skipRouteOnParseError?: { params?: boolean priority?: number } }
Currently, the code accesses
route.options?.skipRouteOnParseError(new-process-route-tree.ts:192, 389) but RouteLike only declarescaseSensitive. The optional chaining masks this type mismatch. -
Add @experimental tag: Add
@experimentalto the JSDoc forskipRouteOnParseErrorto match the pattern used elsewhere in the codebase (e.g., Matches.ts:90).
Comprehensive tests for this feature already exist in packages/router-core/tests/skip-route-on-parse-error.test.ts, covering successful parsing, error skipping with fallbacks, priority ordering, and edge cases.
🤖 Prompt for AI Agents
packages/router-core/src/route.ts around lines 1191-1217 and update RouteLike at
lines ~2011-2013: expose skipRouteOnParseError in the RouteLike.options type and
mark the JSDoc as @experimental; specifically, add skipRouteOnParseError?: {
params?: boolean; priority?: number } to RouteLike.options so calls to
route.options?.skipRouteOnParseError (new-process-route-tree.ts:192, 389) are
type-safe, and add an @experimental tag to the existing JSDoc block for
skipRouteOnParseError to match project conventions.
beaussan
left a comment
beaussan
left a comment
There was a problem hiding this comment.
Docs LGTM, only one nitpick about wording
What are the prefixes on the comment and what do they mean?
My comments are formatted according to the conventional comment convention to help you identify what they are, and how much they are a hard requirement or not. Don't hesitate to reach out if you w ant to learn more
| - See [Guides > Path Params > Validating path parameters during matching](../../guide/path-params#validating-path-parameters-during-matching) for detailed usage examples. | ||
| > [!IMPORTANT] | ||
| > **Performance impact**: This option has a **non-negligible performance cost** and should not be used indiscriminately. Routes with `skipRouteOnParseError` are placed on separate branches in the route matching tree instead of sharing nodes with other dynamic routes. This reduces the tree's ability to efficiently narrow down matches and requires testing more route, even for routes that wouldn't match the path structure alone. |
There was a problem hiding this comment.
nitpick(non-blocking): I found the sentence hard to read as it is, here is a suggestion to keep is simple
| > **Performance impact**: This option has a **non-negligible performance cost** and should not be used indiscriminately. Routes with `skipRouteOnParseError` are placed on separate branches in the route matching tree instead of sharing nodes with other dynamic routes. This reduces the tree's ability to efficiently narrow down matches and requires testing more route, even for routes that wouldn't match the path structure alone. | |
| > **Performance impact**: This option has a **non-negligible performance cost** and should only be enabled when needed. Routes with `skipRouteOnParseError` are placed on separate branches in the route matching tree instead of sharing nodes with other dynamic routes. This reduces the tree's ability to efficiently narrow down matches and requires testing more route, even for routes that wouldn't match the path structure alone. |
| > The `skipRouteOnParseError` option is currently **experimental** and may change in future releases. | ||
| > [!IMPORTANT] | ||
| > **Performance cost**: This feature has a **non-negligible performance cost** and should not be used indiscriminately. It creates additional branches in the route matching tree, reducing matching efficiency and requiring more route evaluations. Use it only when you genuinely need type-specific routes at the same path level. |
There was a problem hiding this comment.
nitpick(non-blocking): I found the sentence hard to read as it is, here is a suggestion to keep is simple
| > **Performance cost**: This feature has a **non-negligible performance cost** and should not be used indiscriminately. It creates additional branches in the route matching tree, reducing matching efficiency and requiring more route evaluations. Use it only when you genuinely need type-specific routes at the same path level. | |
| > **Performance cost**: This feature has a **non-negligible performance cost** and should only be enabled when needed. It creates additional branches in the route matching tree, reducing matching efficiency and requiring more route evaluations. Use it only when you genuinely need type-specific routes at the same path level. |
SeanCassiere
left a comment
SeanCassiere
left a comment
There was a problem hiding this comment.
I wonder if this guide should be approached a little differently. This guide, the whole markdown file, automatically jumps into skipRouteOnParseError and kind of glosses over the params.stringify and params.parse options. For this feature this section is good, but its assuming knowledge that's not documented elsewhere on this page, or even in the docs in general I think (outside of the reference page) 🤔.
Given that this could be a lengthy bit, I think it could even warrant going into its own guide/page on the website. Something like 'Validate Path Params'. This'd be the time to do it, since its not really covered in the other guides.
That guide could use an approach something loosely along the lines of;
- You can use
parseandstringifyfor specifying your param behaviors. - When parsing fails, it puts that route into an error-state and trigger the errorComponent.
- You might want different routes to possibly occupy the same param, we have an experimental feature for that
skipRouteOnParseError.
| /** | ||
| * Options to control route matching behavior with runtime code. | ||
| */ | ||
| skipRouteOnParseError?: { |
There was a problem hiding this comment.
Add a JSDoc annotation for @experimental mentioned this can change. Maybe even a link to the docs RouteOptionsType reference page.
If you search the codebase we should have something similar to copy off of marked with @link.
| > [!IMPORTANT] | ||
| > **Performance cost**: This feature has a **non-negligible performance cost** and should not be used indiscriminately. It creates additional branches in the route matching tree, reducing matching efficiency and requiring more route evaluations. Use it only when you genuinely need type-specific routes at the same path level. | ||
| By default, TanStack Router matches routes based purely on URL structure. A route with `/$param` will match any value for that parameter, and validation via `params.parse` happens later in the route lifecycle. If validation fails, the route shows an error state. |
There was a problem hiding this comment.
| By default, TanStack Router matches routes based purely on URL structure. A route with `/$param` will match any value for that parameter, and validation via `params.parse` happens later in the route lifecycle. If validation fails, the route shows an error state. | |
| By default, TanStack Router doing route matching purely base on URL structure. A route with `/$param` will match any value for that parameter, with validation via `params.parse` happening later in the route lifecycle. If validation fails, the route is set into an 'error state'. |
summary
Use the
params.parseoption (https://tanstack.com/router/latest/docs/framework/react/api/router/RouteOptionsType#paramsparse-method) to validate path params while matching a route. This allows us to look for another match if the validation fails.Pros: more flexible routing. Any
$paramcan now become a regex route segment (or any validation you like).Cons: routes with a
params.parsemethod cannot share the same node as other routes of otherwise the same shape. If used frequently, this increases branching, which increases the number of branches we need to explore to find a match.For more details,
Some details:
params.parsehave priority over routes without (all else being equal)extractParamsis now "resumable" so we can call it at several point while matching without repeating work (at the expense of allocating more objects)indexandpathlessnodes in the segment tree so that previously "invisible" segments can carry "route matching metadata", i.e. in this caseparams.parseandskipRouteOnParseErrorSummary by CodeRabbit
New Features
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.