fix(router-core): support parsed params in matchRoute#7776
fix(router-core): support parsed params in matchRoute#7776LadyBluenotes wants to merge 9 commits into
Conversation
|
Note Currently processing new changes in this PR. This may take a few minutes, please wait... ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (7)
📝 WalkthroughWalkthrough
ChangesTyped route matching
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant useMatchRoute
participant RouterCoreMatchRoute
participant RouteParamParser
useMatchRoute->>RouterCoreMatchRoute: Match route with typed params
RouterCoreMatchRoute->>RouteParamParser: Parse raw URL parameters
RouteParamParser-->>RouterCoreMatchRoute: Return parsed params or rejection
RouterCoreMatchRoute-->>useMatchRoute: Return typed match or false
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 06a3de5
☁️ Nx Cloud last updated this comment at |
🚀 Changeset Version Preview1 package(s) bumped directly, 22 bumped as dependents. 🟩 Patch bumps
|
…ck/router into match-route-parsed-params
Bundle Size Benchmarks
Current gzip tracks all emitted client JS chunks. Initial gzip tracks only the entry/import graph. Trend sparkline is historical current gzip ending with this PR measurement; lower is better. |
Merging this PR will degrade performance by 9.66%
|
| Mode | Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|---|
| ❌ | Memory | mem server peak-large-page (react) |
948.6 KB | 2,110 KB | -55.04% |
| ❌ | Memory | mem server error-paths redirect (vue) |
304.6 KB | 404 KB | -24.59% |
| ❌ | Memory | mem server peak-large-page (solid) |
769.6 KB | 802.8 KB | -4.14% |
| ❌ | Simulation | client-route-tree-scale navigation loop (react) |
74.1 ms | 77.2 ms | -3.98% |
| ❌ | Memory | mem server error-paths not-found (react) |
262 KB | 271.9 KB | -3.65% |
| ❌ | Simulation | ssr streaming deferred (react) |
70.6 ms | 72.9 ms | -3.13% |
| ⚡ | Memory | mem server streaming-peak chunked (vue) |
13.9 MB | 11.1 MB | +25.41% |
| ⚡ | Simulation | client-loaders navigation loop (react) |
55.5 ms | 52.8 ms | +5.2% |
| ⚡ | Memory | mem server server-fn-churn (react) |
283 KB | 271.4 KB | +4.26% |
Tip
Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.
Comparing match-route-parsed-params (24a76d9) with main (a3e24c3)
Sheraff
left a comment
There was a problem hiding this comment.
I think it makes sense to fix this. But also router.matchRoute is broken more generally because it does not take the route tree into account. It could return true for the specific route you're asking for, but when actually compared against the entire tree, another route would match because it had higher matching priority.
Added a regression for |
There was a problem hiding this comment.
Important
At least one additional CI pipeline execution has run since the conclusion below was written and it may no longer be applicable.
Nx Cloud has identified a possible root cause for your failed CI:
We determined that this failure is unrelated to the PR's changes, which are scoped entirely to router-core param parsing logic. The tanstack-start-example-rscs:build task fails due to a nitro/nitro-nightly package version incompatibility (ERR_PACKAGE_PATH_NOT_EXPORTED) in the environment. No fix is required on this PR.
No code changes were suggested for this issue.
Trigger a rerun:
🎓 Learn more about Self-Healing CI on nx.dev
|
@Sheraff made the changes we talked about in discord |
matchRoutenow resolves matches through the processed route tree.params.parseresults before comparing or returning params.findSingleMatchand its cache withfindRouteMatch.caseSensitive, basepath, search, pending, optional-param, and wildcard behavior covered.matchRoutenow returnsfalsefor destination templates that are not registered in the route tree.Includes the reproducer from #2460.
Closes #2450.
Summary by CodeRabbit
params.parse/params.stringify.