perf(router-core): match the current location lazily in buildLocation - #8406
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (1)
Included review availability: Your plan provides up to 10 included reviews per hour; 6 remain after this review. 📝 WalkthroughWalkthrough
ChangesLocation build evaluation
Priority: ⬇️ Low Estimated code review effort: 2 (Simple) | ~10 minutes Change: Refactor Suggested reviewers: Merge Risk: ⚪ Minimal · up to The lazy matching and template-check changes preserve existing behavior, with no actionable merge-blocking risk identified. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 1f197ad
☁️ Nx Cloud last updated this comment at |
🚀 Changeset Version Preview7 package(s) bumped directly, 22 bumped as dependents. 🟩 Patch bumps
|
Bundle Size Benchmarks
The following scenarios have bundle-size changes compared with the baseline:
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 improve performance by 12.83%
|
| Mode | Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|---|
| ⚡ | Simulation | client-links navigation loop (react) |
218.4 ms | 97.6 ms | ×2.2 |
| ⚡ | Memory | mem client interrupted-navigations (react) |
385.3 KB | 213 KB | +80.84% |
| ⚡ | Memory | mem client interrupted-navigations (vue) |
389.7 KB | 222.8 KB | +74.93% |
| ⚡ | Memory | mem client interrupted-navigations (solid) |
384.6 KB | 253.4 KB | +51.78% |
| ⚡ | Simulation | client-rewrites navigation loop (react) |
110.1 ms | 79.4 ms | +38.68% |
| ⚡ | Simulation | client-history navigation loop (react) |
84.5 ms | 69.7 ms | +21.33% |
| ⚡ | Simulation | client-links navigation loop (vue) |
347.1 ms | 296.4 ms | +17.11% |
| ⚡ | Simulation | client-route-tree-scale navigation loop (react) |
128.8 ms | 111.2 ms | +15.88% |
| ⚡ | Simulation | client-links navigation loop (solid) |
524.1 ms | 456.3 ms | +14.85% |
| ⚡ | Simulation | client-async-pipeline navigation loop (vue) |
87.5 ms | 77.7 ms | +12.64% |
| ⚡ | Simulation | client-route-tree-scale navigation loop (vue) |
161.4 ms | 144 ms | +12.04% |
| ⚡ | Simulation | client-async-pipeline navigation loop (react) |
75.7 ms | 67.6 ms | +11.94% |
| ⚡ | Simulation | client-control-flow navigation loop (vue) |
83.6 ms | 75.2 ms | +11.07% |
| ⚡ | Simulation | ssr request loop (react) |
177.3 ms | 160.9 ms | +10.21% |
| ⚡ | Simulation | client-loaders navigation loop (react) |
105.9 ms | 97.6 ms | +8.47% |
| ⚡ | Simulation | client-side navigation loop (react) |
108.9 ms | 100.9 ms | +7.96% |
| ⚡ | Memory | mem client loader-data-retention (react) |
949.3 KB | 879.9 KB | +7.89% |
| ⚡ | Memory | mem server error-paths redirect (react) |
863.4 KB | 806.7 KB | +7.03% |
| ⚡ | Simulation | client-search-params navigation loop (react) |
142.4 ms | 133.9 ms | +6.3% |
| ⚡ | Memory | mem client loader-data-retention (vue) |
979 KB | 921.4 KB | +6.26% |
| ... | ... | ... | ... | ... | ... |
ℹ️ Only the first 20 benchmarks are displayed. Go to the app to view all benchmarks.
Tip
Curious why performance improved? Comment @codspeedbot explain why performance improved on this PR, or directly use the CodSpeed MCP with your agent.
Comparing schiller-manuel-lazy-current-match-layer (1f197ad) with main (ae68535)1
Footnotes
`buildLocation` matched the current location up front for every build, although the result is only read by relative destinations, inherited params, `search: true` or updater functions and search middlewares. An absolute destination with its params given, on a branch without search middlewares, never reads it, yet still paid three store reads and a WeakMap lookup per call. The match is now computed on first read, and the destination template is scanned for `$` once instead of twice. Server `buildLocation` of `/items/$itemId` with params on a middleware-free tree: 399 -> 382 ns per call (-4%). Builds that read the current match are unchanged. react-router.minimal gzip +4 (86008 -> 86012, the stack base). Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
4e93585 to
1f197ad
Compare
…#8406) `buildLocation` matched the current location up front for every build, although the result is only read by relative destinations, inherited params, `search: true` or updater functions and search middlewares. An absolute destination with its params given, on a branch without search middlewares, never reads it, yet still paid three store reads and a WeakMap lookup per call. The match is now computed on first read, and the destination template is scanned for `$` once instead of twice. Server `buildLocation` of `/items/$itemId` with params on a middleware-free tree: 399 -> 382 ns per call (-4%). Builds that read the current match are unchanged. react-router.minimal gzip +4 (86008 -> 86012, the stack base). Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Stacked on #8405.
🎯 Changes
buildLocationmatched the current location up front for every build (matchRoutesLightweight: three store reads and a WeakMap lookup), although the result is only read by relative destinations, inherited params,search: trueor updater functions, and search middlewares. An absolute destination with its params given, on a branch without search middlewares, never reads it, yet still paid for the match on every call.currentMatch()(lightweight ??= this.matchRoutesLightweight(currentLocation)).$once (isTemplate) instead of twice.Behavior is unchanged: every read of the current match still goes through
currentMatch(), which also setsusedCurrent, so the client-side location cache keeps excluding builds that depend on the current location. Public API unchanged. One file touched:packages/router-core/src/router.ts(+7/−7).Measurements (local, macOS arm64, Node 24.8.0)
buildLocationof/items/$itemIdwith params on a middleware-free tree: 399 → 382 ns per call (−4%).react-router.minimalgzip: 86008 → 86012 (+4 bytes, exactly the stack base).Tests run (fresh Nx runs,
--skipNxCache --skipRemoteCache)@tanstack/router-core:test:unit— 133 files, 3303 passed, 4 expected fail@tanstack/router-core:test:types— TS 5.6, 5.7, 5.8, 5.9, 6.0, 7.0 all pass@tanstack/router-core:test:eslint— 0 errors (27 pre-existing warnings, none in the changed region)@tanstack/react-router:test:unit— 92 files, 1175 passed, 1 skipped@tanstack/solid-router:test:unit— 68 files, 942 passed, 1 skipped (+ 7 files, 51 passed)@tanstack/vue-router:test:unit— 71 files, 968 passed, 1 skippedgit diff --check HEAD~1andpnpm exec prettier --check packages/router-core/src/router.ts— clean✅ Checklist
🚀 Release Impact
.changeset/brisk-links-serve.mdfrom the lower layers of this stack already declares a@tanstack/router-corepatch, so no new changeset is added here.Summary by CodeRabbit