fix(router): ensure useParams returns parsed params when strict is false #6387
Conversation
📝 WalkthroughWalkthroughAdds file-based routes for /params-ps/strict-false and its child /$version (with parse/stringify), new UI links, E2E and unit tests validating useParams({ strict: false }) receives parsed child params after navigation, updates generated route tree, and introduces an optimization in router match lookup. Changes
Sequence Diagram(s)(Skipped — changes are primarily route additions, type mappings, tests, and a localized router optimization that don't require a multi-component sequential visualization.) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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 |
|
View your CI Pipeline Execution ↗ for commit e5ce02e
☁️ Nx Cloud last updated this comment at |
|
oops, clicked the wrong button and accidentally closed this! 😅 Reopening now. |
| // Update the match's params | ||
| const previousMatch = previousMatchesByRouteId.get(match.routeId) | ||
| match.params = previousMatch | ||
| ? replaceEqualDeep(previousMatch.params, routeParams) | ||
| : routeParams |
There was a problem hiding this comment.
Maybe we shouldn't set the match.params in the 1st iteration then? Because it looks like we're now doing the work twice now.
There was a problem hiding this comment.
@Sheraff I tried removing the second update, but the regression (#6385) reappeared.
Here is why the second update is necessary:
The router processes matches from Parent to Child. When the Parent match is created in the first loop, the Child hasn't been processed yet, so routeParams doesn't strictly contain the Child's parsed params at that moment.
Since strict: false allows Parents to access Child params, we must update the Parent's match.params after the entire loop finishes (when routeParams is fully populated).
I've optimized it by piggybacking on the existing second loop (used for Context) instead of creating a new one, so the performance impact is minimal.
Please let me know if I misunderstood anything. Thank you!
jong-kyung
force-pushed
the
fix/router-params
branch
4 times, most recently
from
695c44a to
e5ce02e
Compare
2 participants
fixes #6385
This PR fixes issue #6385, where
useParams({ strict: false })in a parent route would return unparsed parameter values from a child route.The Fix
The fix the issue by ensuring that parsed parameters from child routes are correctly propagated to their parents when
strict: falseis used.Correct Parameter Propagation: The router's internal matching logic was updated. When a route match is being processed, it now correctly looks up the previous match's state, which includes its parsed parameters, and merges them. This ensures the parsed values are available throughout the route hierarchy.
Performance Optimization: As part of the fix, the mechanism for retrieving previous route matches was optimized. The implementation was changed from using
Array.prototype.find()to aMap-based lookup. This improves performance from O(n) to O(1), which is especially beneficial in applications with many routes.Changes
packages/router-core/src/router.ts: Updated the core routing logic to correctly handle parameter propagation and implemented theMap-based optimization for match retrieval.packages/react-router/tests/useParams.test.tsx: Added a new unit test to specifically verify thatuseParams({ strict: false })receives parsed values.e2e/react-router/basic-file-based/**: Added new routes and an e2e test to confirm the fix in a real-world file-based routing scenario.Summary by CodeRabbit
New Features
Tests
Chores
✏️ Tip: You can customize this high-level summary in your review settings.