fix(router-core): extract params from URL for snapshot when using string navigation #6441
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.
Summary
When navigating with a string URL (e.g.,
navigate({ to: '/users/abc123' })), the match snapshot was incorrectly using params from the current location instead of extracting params from the destination URL.This caused
Route.useParams()to returnundefinedfor dynamic params when the source route didn't have those params in its path.The Bug
In
buildLocation(), whendest.paramsis not provided:nextParamswas set tofromParams(params from current location)matchRoutesInternal()used the snapshot via fast-pathuseParams()to failExample scenario:
/users/123/settingsto/users/123/posts/abcnavigate({ to: '/users/123/posts/abc' })(string URL)Route.useParams()returned{ userId: '123' }butpostIdwasundefined/settingsroute didn't havepostIdin its paramsThe Fix
Extract params from the final interpolated pathname using
getMatchedRoutes()before building the snapshot:Why This Only Affected Production
The snapshot fast-path (line 1290-1295 in
matchRoutesInternal) is only used when the snapshot's session ID matches the current router session. In development with hot reloading, session IDs often don't match, causing the fallback path (which correctly extracts params from the URL) to be used instead.Test Plan
load.test.tstests pass (26 tests)callbacks.test.tstests pass (3 tests)match-by-path.test.tstests pass (91 tests)path.test.tstests pass (205 tests)Considerations
A few things we thought about but believe are acceptable trade-offs:
Extra
getMatchedRoutescall: This adds one additional route matching operation per navigation. Route matching is a fast trie lookup, and this is called once per navigation (not in a hot loop), so the performance impact should be negligible. Happy to explore reusing the existingdestMatchesresult if you'd prefer to avoid this.parsedParamstyping: When using typed params with non-string values (e.g.,params: { id: 123 }), the snapshot'sparsedParamswill now contain strings extracted from the URL rather than the original types. However, params are re-parsed later in the match creation flow using the route'sparseParamsfunction, so this shouldn't cause issues in practice.opts.leaveParamsedge case: IfleaveParamsis true, the pathname retains$paramplaceholders and params won't be extracted. This is an existing edge case for template path preservation that this fix doesn't make worse.Open to feedback on any of these points!
Summary by CodeRabbit
Release Notes
✏️ Tip: You can customize this high-level summary in your review settings.