fix: use custom params from declarative masks when building the url #5756
Conversation
WalkthroughRouter mask logic now reads per-mask Changes
Sequence Diagram(s)sequenceDiagram
participant Router
participant MaskDef as Mask Definition
participant Dest as Masked Destination
Router->>MaskDef: Find matching mask for pathname
alt mask found
MaskDef-->>Router: returns mask (includes maskParams, maskProps, to/from)
rect rgb(230, 245, 255)
Note over Router: Compute nextParams
Router->>Router: read maskParams
alt maskParams === false or null
Router->>Router: nextParams = {}
else maskParams === true or undefined
Router->>Router: nextParams = current params
else maskParams is function/object
Router->>Router: nextParams = functionalUpdate(maskParams, current params)
end
end
Router->>Dest: build maskedDest with pathname from mask and params = nextParams (preserve maskProps/from)
Dest-->>Router: return masked destination (href/search/hash/state included)
else no mask
Router->>Router: proceed with original destination
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20–25 minutes
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 (1)
🧰 Additional context used📓 Path-based instructions (2)**/*.{ts,tsx}📄 CodeRabbit inference engine (AGENTS.md)
Files:
packages/router-core/**📄 CodeRabbit inference engine (AGENTS.md)
Files:
🧠 Learnings (2)📚 Learning: 2025-10-08T08:11:47.088ZApplied to files:
📚 Learning: 2025-10-01T18:30:26.591ZApplied to files:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![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 (2)
packages/router-core/src/router.ts (2)
1800-1801: Clarify the comment to cover all maskParams cases.The comment mentions "params function" but the logic handles multiple scenarios (false, null, true, undefined, function, object). Consider updating it to describe all cases for better maintainability.
- // If mask has a params function, call it with the matched params as context - // Otherwise, use the matched params or the provided params value + // Compute params for the masked destination: + // - false/null: no params + // - true/undefined: use matched params + // - function/object: merge matched params with the result of functionalUpdate
1807-1807: Consider using spread operator to avoid mutation.
Object.assign(params, ...)mutates theparamsobject. While this may not cause issues in the current scope, using the spread operator would be safer and align better with immutability principles.- : Object.assign(params, functionalUpdate(maskParams, params)) + : { ...params, ...functionalUpdate(maskParams, params) }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/router-core/src/router.ts(1 hunks)
🧰 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/router.ts
packages/router-core/**
📄 CodeRabbit inference engine (AGENTS.md)
Keep framework-agnostic core router logic in packages/router-core/
Files:
packages/router-core/src/router.ts
🧬 Code graph analysis (1)
packages/router-core/src/router.ts (1)
packages/router-core/src/utils.ts (1)
functionalUpdate(203-212)
| const { from: _from, params: maskParams, ...maskProps } = foundMask | ||
|
|
||
| // If mask has a params function, call it with the matched params as context | ||
| // Otherwise, use the matched params or the provided params value | ||
| const nextParams = | ||
| maskParams === false || maskParams === null | ||
| ? {} | ||
| : (maskParams ?? true) === true | ||
| ? params | ||
| : Object.assign(params, functionalUpdate(maskParams, params)) | ||
|
|
||
| maskedDest = { | ||
| from: opts.from, | ||
| ...maskProps, | ||
| params, | ||
| params: nextParams, |
There was a problem hiding this comment.
🧩 Analysis chain
Logic correctly applies custom mask params to resolve issue #5755.
The implementation properly extracts and applies maskParams from the mask definition, enabling dynamic parameter composition per mask. This fixes the reported issue where mask params were being ignored.
Please verify that all maskParams scenarios are covered in tests:
maskParams: falseornull→ empty paramsmaskParams: trueorundefined→ matched params preservedmaskParams: (params) => {...}→ function transformation appliedmaskParams: {...}→ object merged with matched params
Run the following script to check for existing test coverage of mask params:
🏁 Script executed:
#!/bin/bash
# Search for tests related to route mask params functionality
rg -n -C3 --type=ts --type=tsx 'maskParams|mask.*params|routeMask.*param' -g '*test*' -g '*spec*'Length of output: 27
🏁 Script executed:
#!/bin/bash
# Search for tests related to route mask params functionality using corrected syntax
rg -n -C3 --type=ts 'maskParams|mask.*params|routeMask' -g '*test*' -g '*spec*'Length of output: 6422
🏁 Script executed:
#!/bin/bash
# Find test files related to router or mask functionality
fd -e ts -e js '(test|spec)' -path '*router*' -o -path '*mask*'Length of output: 168
🏁 Script executed:
#!/bin/bash
# Search for maskParams usage anywhere in the codebase to understand scope
rg -n 'maskParams' --type=tsLength of output: 417
🏁 Script executed:
#!/bin/bash
# Search for createRouteMask definition to understand maskParams support
rg -n -A20 'function createRouteMask|const createRouteMask' --type=tsLength of output: 2871
🏁 Script executed:
#!/bin/bash
# Search for mask type definition
rg -n 'type.*[Mm]ask|interface.*[Mm]ask' --type=ts -A5 | head -100Length of output: 2021
🏁 Script executed:
#!/bin/bash
# Look for any tests passing functions as mask params
rg -n 'params.*=>|mask.*function' --type=ts -g '*test*' -g '*spec*' -C2Length of output: 173798
🏁 Script executed:
#!/bin/bash
# Check RouteMask and ToMaskOptions type definitions for maskParams support
rg -n -A10 'type RouteMask<|type ToMaskOptions<|interface ToMaskOptions' --type=ts packages/router-core/src/Length of output: 1377
🏁 Script executed:
#!/bin/bash
# Search for functionalUpdate utility used in the maskParams logic
rg -n -B5 -A5 'functionalUpdate' packages/router-core/src/router.ts | head -50Length of output: 1549
Add tests for maskParams feature before merging.
The implementation correctly handles all four maskParams scenarios (false/null, true/undefined, function transformation, and object merge), following established patterns in the codebase. However, there is no test coverage for this new feature.
Add tests covering:
maskParams: falseornull→ empty paramsmaskParams: trueorundefined→ matched params preservedmaskParams: (params) => {...}→ function transformation appliedmaskParams: {...}→ object merged with matched params
🤖 Prompt for AI Agents
In packages/router-core/src/router.ts around lines 1798 to 1812, the new
maskParams handling logic lacks test coverage; add unit tests that exercise all
four branches: (1) maskParams false and null should produce an empty params
object, (2) maskParams true and undefined should preserve the matched params,
(3) maskParams as a function should receive the matched params and the returned
value should be used, and (4) maskParams as an object should be merged with
matched params via the same functionalUpdate semantics. Create tests in the
router-core test suite (e.g., packages/router-core/__tests__ or
tests/router.spec.ts), mock or construct route matches and call the code paths
that produce maskedDest, asserting the resulting maskedDest.params for each
scenario; include edge cases for falsy/undefined returns from the function and
ensure merge semantics match Object.assign(functionalUpdate(...), params).
|
we definitely need tests here. ideally start with the tests first and then the solution |
|
@schiller-manuel thanks for the reply. Since I couldn't find any tests for masks specifically in the router-core I added a new file and tried to cover different cases with masks and params. Let me know what you think! |
2 participants
This PR proposes a fix for issue #5755 , where custom params passed to declarative route masks are ignored. When the masked params differ from the original URL params, the masked URL can display
undefinedinstead of the expected values.I don't have full context on all internal router behaviors, so please treat this as a proposal. If there's a better approach, I’m happy to adjust the implementation.
Summary by CodeRabbit
New Features
Tests