refactor(router-core): use original nextTo path when building location in its route form #5851
+450
−26
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
# Conflicts: # packages/router-core/src/path.ts
Contributor
WalkthroughAdds and exports a new useMatchRoute hook, expands React and Solid tests for many param patterns, removes the leaveParams branch from interpolatePath (always encodes params), and adjusts buildLocation to return the original template when leaveParams is requested. Changes
Sequence Diagram(s)sequenceDiagram
participant App as App (component)
participant useMatchRoute as useMatchRoute
participant buildLocation as buildLocation
participant interpolatePath as interpolatePath
participant matcher as pathMatcher
App->>useMatchRoute: call matchRoute({ to: '/page-{$page}' })
activate useMatchRoute
useMatchRoute->>buildLocation: build candidate location (opts.leaveParams = true/false)
activate buildLocation
alt opts.leaveParams == true
buildLocation-->>useMatchRoute: return candidate = '/page-{$page}'
else
buildLocation->>interpolatePath: interpolatePath(template, params)
interpolatePath-->>buildLocation: return '/page-a' (encoded)
buildLocation-->>useMatchRoute: return candidate = '/page-a'
end
deactivate buildLocation
useMatchRoute->>matcher: compare candidate vs current '/page-a'
activate matcher
matcher-->>useMatchRoute: match result (params or false)
deactivate matcher
useMatchRoute-->>App: return match result
deactivate useMatchRoute
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
💤 Files with no reviewable changes (1)
🧰 Additional context used🧠 Learnings (8)📓 Common learnings📚 Learning: 2025-10-01T18:30:26.591ZApplied to files:
📚 Learning: 2025-09-22T00:56:49.237ZApplied to files:
📚 Learning: 2025-09-28T21:41:45.233ZApplied to files:
📚 Learning: 2025-09-22T00:56:53.426ZApplied to files:
📚 Learning: 2025-10-08T08:11:47.088ZApplied to files:
📚 Learning: 2025-10-01T18:31:35.420ZApplied to files:
📚 Learning: 2025-10-14T18:59:33.990ZApplied to files:
⏰ 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)
🔇 Additional comments (1)
Comment |
|
View your CI Pipeline Execution ↗ for commit c8ce2b2
☁️ Nx Cloud last updated this comment at |
Contributor
Author
![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: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/router-core/src/path.ts(6 hunks)packages/router-core/tests/path.test.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/router-core/tests/path.test.ts
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use TypeScript in strict mode with extensive type safety across the codebase
Files:
packages/router-core/src/path.ts
packages/router-core/**
📄 CodeRabbit inference engine (AGENTS.md)
Keep framework-agnostic core router logic in packages/router-core/
Files:
packages/router-core/src/path.ts
🧠 Learnings (4)
📓 Common learnings
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: 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: CR
Repo: TanStack/router PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-09-23T17:36:12.598Z
Learning: Applies to packages/{react-router,solid-router}/** : Implement React and Solid bindings/components only in packages/react-router/ and packages/solid-router/
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: 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: 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-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:
packages/router-core/src/path.ts
📚 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:
packages/router-core/src/path.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/path.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/path.ts (4)
447-454: LGTM - Correctly preserves placeholder for missing splat parameters.When
leaveParamsis true and the splat parameter is missing, this correctly returns the placeholder format (e.g.,prefix{$}suffix) instead of omitting the segment. This enables proper route matching with curly-brace syntax.
508-511: LGTM - Correctly preserves placeholder for missing optional parameters.When
leaveParamsis true and the optional parameter is missing, this correctly returns only the placeholder format (e.g.,prefix{-$param}suffix). This is the right pattern that the wildcard and regular param cases should also follow.
523-526: LGTM - Correctly preserves placeholder for existing optional parameters.When
leaveParamsis true, this correctly returns only the placeholder format, regardless of whether the parameter has a value. This consistent approach enables proper route matching.
838-840: Verify: Should empty optional parameter values be excluded from the result?This code prevents adding optional parameters with empty string values to the extracted params. For example, if a route is
/prefix{-$param}suffixand the actual path is/prefixsuffix, the extracted_paramValuewould be'', which is not added toparams.This creates an asymmetry with the interpolation logic (line 507), which treats empty strings as valid values distinct from null/undefined. Consider:
- Is it intentional that users cannot distinguish between "optional param present but empty" vs "optional param not present"?
- Could any use cases legitimately need to match empty parameter values?
For most scenarios, omitting empty optional params seems reasonable, but please confirm this is the intended behavior.
nlynzaad
changed the title
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/react-router/tests/Matches.test.tsx (1)
148-356: Thorough param‑matching coverage; tightentextContenthandling and double‑check optional param expectationsThe new
matching on different param typestable‑driven suite is a solid addition and clearly targets the regression around prefix/suffix and brace syntax; overall structure and use ofcreateMemoryHistory+useMatchRoutelook good.Two minor points to consider:
- Guard
textContentto satisfy TS and avoid latent null issues
screen.findByTestIdguarantees the element exists, butHTMLElement['textContent']is typed asstring | null. To keep TypeScript happy (and be explicit about the assumption), it would be better to assert non‑null or provide a fallback beforeJSON.parse:- expect(JSON.parse(paramsToCheck.textContent)).toEqual(params) - expect(JSON.parse(matchesToCheck.textContent)).toEqual(matchParams) + expect(JSON.parse(paramsToCheck.textContent!)).toEqual(params) + expect(JSON.parse(matchesToCheck.textContent!)).toEqual(matchParams)(or use a null‑coalescing fallback if you prefer a safer runtime behavior).
- Verify the intentional difference between
paramsandmatchParamsfor some optional casesFor the “optional param with prefix/suffix and no value” cases you expect
paramsto be{}whilematchParamsincludes{ id: '' }, whereas in the complex case"optional param with required param, prefix, suffix, wildcard and no value"bothparamsandmatchParamsincludeid: ''. If this asymmetry reflects existing, intentional behavior betweenRoute.useParams()andmatchRoute, great—might be worth a short inline comment on one of those cases to document it. If not intentional, this is a good place to harmonize the expectations while you’re formalizing this behavior in tests.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
packages/react-router/tests/Matches.test.tsx(4 hunks)packages/router-core/src/path.ts(2 hunks)packages/router-core/src/router.ts(1 hunks)packages/solid-router/tests/Matches.test.tsx(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/router-core/src/path.ts
🧰 Additional context used
🧠 Learnings (5)
📓 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.
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: 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-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/react-router/tests/Matches.test.tsxpackages/router-core/src/router.tspackages/solid-router/tests/Matches.test.tsx
📚 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/react-router/tests/Matches.test.tsxpackages/solid-router/tests/Matches.test.tsx
📚 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/solid-router/tests/Matches.test.tsx
🧬 Code graph analysis (3)
packages/react-router/tests/Matches.test.tsx (1)
packages/react-router/src/Matches.tsx (1)
useMatchRoute(142-172)
packages/router-core/src/router.ts (2)
packages/router-core/src/utils.ts (1)
decodePath(499-559)packages/router-core/src/path.ts (1)
interpolatePath(402-497)
packages/solid-router/tests/Matches.test.tsx (2)
packages/solid-router/src/Matches.tsx (1)
useMatchRoute(123-154)packages/solid-router/src/RouterProvider.tsx (1)
RouterProvider(44-53)
🔇 Additional comments (5)
packages/solid-router/tests/Matches.test.tsx (3)
1-20: LGTM! Well-organized imports for comprehensive testing.The new imports properly support the expanded test suite:
afterEach,describe, andwaitForenable robust test structure and async assertionscreateMemoryHistoryprovides isolated routing history for each testuseMatchRouteis the newly exposed public API hook being tested
153-310: Excellent test coverage for parameter matching edge cases.The parameterized test suite comprehensively covers the issue described in #5674 and beyond:
- Standard and curly-brace parameter syntax (
/$idvs/{$id})- Parameters with prefixes and/or suffixes (e.g.,
/prefix-{$id}-suffix)- Wildcard parameters with various prefix/suffix combinations
- Optional parameters in isolation and combination with other param types
- Complex scenarios mixing multiple parameter types
This thorough coverage should prevent regressions in parameter matching behavior.
311-363: LGTM! Well-structured test implementation following Solid.js patterns.The test implementation correctly:
- Isolates each test with fresh router and memory history instances
- Uses
router.history.push()to trigger navigation programmatically- Employs
waitForto handle async DOM updates from Solid.js reactivity- Calls accessor functions (signals) with
()syntax per Solid.js conventions- Tests both
Route.useParams()and the newuseMatchRoute()hook- Validates extracted parameters match expected values for each scenario
The per-test isolation and comprehensive assertions ensure reliable verification of the parameter matching fixes.
packages/react-router/tests/Matches.test.tsx (2)
1-14: Imports and newuseMatchRouteusage look consistentThe added imports (
afterEach,act,cleanup,createMemoryHistory,useMatchRoute) are all used below and keep the test file cohesive; no issues from my side here.
127-147: Pending component test remains valid after surrounding changesThe
should show pendingComponent of root routetest still accurately exercisespendingComponentvsdefaultPendingComponent; the minor structural/whitespace changes around the router creation do not affect behavior.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/router-core/src/path.ts (1)
390-400: Remove duplicate JSDoc comment.There are two JSDoc blocks before the
interpolatePathfunction. The first block (lines 390-396) contains outdated information stating "Optionally leaves placeholders or wildcards in place," which is no longer accurate after the removal ofleaveParams. The second block (lines 397-400) reflects the current behavior.Apply this diff to remove the outdated comment:
-/** - * Interpolate params and wildcards into a route path template. - * - * - Encodes params safely (configurable allowed characters) - * - Supports `{-$optional}` segments, `{prefix{$id}suffix}` and `{$}` wildcards - * - Optionally leaves placeholders or wildcards in place - */ /** * Interpolate params and wildcards into a route path template. * Encodes safely and supports optional params and custom decode char maps. */
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/router-core/src/path.ts(2 hunks)packages/router-core/src/router.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/router-core/src/router.ts
🧰 Additional context used
🧠 Learnings (8)
📓 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.
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: 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: 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-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/path.ts
📚 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:
packages/router-core/src/path.ts
📚 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:
packages/router-core/src/path.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/path.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/path.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/path.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/src/path.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
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.
During the matching process, when building the location, we interpolate the next path while leaving the params in place, i.e. returning the route back in its original route form. This is unnecessary and can be avoided by just using the original next.to path.
This also simplifies interpolatePath in that leaveParams is no longer necessary which in turn resolves #5674 since the logic in interpolatePath when
leaveParams=truewas not returning the segments in the correct form.This PR applies the above changes and adds tests to verify that the params are still matched correctly and that matchRoutes now also succeeds where it was previously failing
Summary by CodeRabbit
New Features
Improvements
Tests