refactor(router-core): interpolatePath skips optional segments w/ nullish param value #5934
+27
−16
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
WalkthroughinterpolatePath's handling of optional path segments was changed: when an optional param value is missing the segment is now skipped entirely (no prefix/suffix emitted); when present, prefix/suffix are computed and emitted around the value. Changes
Sequence Diagram(s)sequenceDiagram
participant Caller
participant interpolatePath
participant Segment (optional param)
Caller->>interpolatePath: build path with segments
interpolatePath->>Segment: examine segment type
alt SEGMENT_TYPE_OPTIONAL_PARAM
Segment->>interpolatePath: hasValue? (value)
alt value is missing
interpolatePath-->>Caller: skip segment (no prefix/suffix)
else value present
interpolatePath->>Segment: compute prefix & suffix
interpolatePath-->>Caller: emit prefix + value + suffix
end
else other segment types
interpolatePath-->>Caller: emit segment per existing rules
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (4)
🧰 Additional context used🧠 Learnings (6)📓 Common learnings📚 Learning: 2025-10-08T08:11:47.088ZApplied to files:
📚 Learning: 2025-10-14T18:59:33.990ZApplied to files:
📚 Learning: 2025-10-01T18:30:26.591ZApplied to files:
📚 Learning: 2025-09-22T00:56:49.237ZApplied to files:
📚 Learning: 2025-10-09T12:59:02.129ZApplied 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 (4)
Comment |
|
View your CI Pipeline Execution ↗ for commit a30b88d
☁️ 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 (2)
packages/router-core/src/path.ts (1)
321-333: Optional param interpolation correctly skips nullish valuesThe new logic for
SEGMENT_TYPE_OPTIONAL_PARAMlooks solid:
valueRaw == nullshort‑circuits before computing prefix/suffix or callingencodeParam, so segments likef_{-$folderId}are fully omitted (including the slash and any prefix/suffix) when the param isundefinedornull.- For present values (including empty string), prefix/suffix are still honored and
encodeParamis reused, so cases like''yielding/posts/prefixor/posts/.htmlbehave as expected.usedParamsonly records actually interpolated optional params andisMissingParamsremains unaffected by omitted optionals, which matches the intended “optional” semantics.Behavior aligns with the described bug and with the new tests. As a minor enhancement, you might consider adding a dedicated test for
params: { category: null }to document thatnullis treated like an omitted value, but the current implementation is already correct.packages/router-core/tests/optional-path-params.test.ts (1)
205-251: Tests accurately pin new prefix/suffix behavior for optional paramsThe updated expectations and added cases around:
optional param with prefix(omitted →/posts, empty string →/posts/prefix),optional param with suffix(omitted →/posts, empty string →/posts/.html),optional param with prefix and suffix(omitted →/posts, empty string →/posts/prefixsuffix),nicely exercise the new interpolation semantics: nullish/omitted optionals drop the whole segment (including surrounding text), while empty string keeps the segment and just omits the value.
You already cover omitted, undefined, and empty string; if you want to be extra explicit, you could add a
params: { category: null }variant mirroring the undefined case to lock in thatnullis treated as “not provided”.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/router-core/src/path.ts(1 hunks)packages/router-core/tests/optional-path-params.test.ts(3 hunks)
🧰 Additional context used
🧠 Learnings (6)
📓 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.
📚 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/tests/optional-path-params.test.tspackages/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/tests/optional-path-params.test.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-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
schiller-manuel
approved these changes
Nov 21, 2025
nlynzaad
approved these changes
Nov 21, 2025
Sheraff
deleted the
refactor-router-core-interpolate-path-skips-nullish-optionals
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.
interpolatePathshould be able to skip optional segments when building a URL if the param provided for that segment is nullish (null | undefined). We keep it in if the value is''so users still have the ability to do/prefixsuffixThe issue was reported on discord:
Summary by CodeRabbit
Bug Fixes
Tests
✏️ Tip: You can customize this high-level summary in your review settings.