fix(router): ensure useParams returns parsed params when strict is false#6387
fix(router): ensure useParams returns parsed params when strict is false#6387nlynzaad merged 7 commits intoTanStack:mainfrom
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
📝 WalkthroughWalkthroughAdds file-based routes for /params-ps/strict-false and /params-ps/strict-false/$version (with parse/stringify), UI links, E2E and unit tests validating useParams({ strict: false }) receives parsed child params after navigation, updates generated route tree, and introduces a router previous-match lookup optimization. Changes
Sequence Diagram(s)sequenceDiagram
participant Browser as Browser/Client
participant Router as Router Core
participant Parent as Parent Route
participant Child as Child Route ($version)
Browser->>Router: Navigate to /params-ps/strict-false/1
Router->>Child: match child route, parse params (version -> number)
Router->>Parent: update parent match with parsed child params
Parent->>Browser: render parent showing parsed version type/value
Browser->>Router: Click link -> navigate to /params-ps/strict-false/2
Router->>Child: match child route, parse params (version -> number)
Router->>Parent: propagate updated parsed params to parent
Parent->>Browser: re-render with new parsed version value
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 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🧪 Generate unit tests (beta)
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 fffc257
☁️ 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
|
@nlynzaad |
|
@nlynzaad Thank you for the review and merge ! |
…lse (#6387) * fix: optimize match retrieval by using a Map for previous matches * feat: add strict false parameter handling and related routes * replace params only when routeParams is fully parsed --------- Co-authored-by: Nico Lynzaad <nlynzaad@zylangroup.com> Co-authored-by: Nico Lynzaad <44094871+nlynzaad@users.noreply.github.com>
3 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