perf(router-core): optimize buildLocation path resolution - #8108
Conversation
|
View your CI Pipeline Execution ↗ for commit 4b877ce
☁️ Nx Cloud last updated this comment at |
🚀 Changeset Version PreviewNo changeset entries found. Merging this PR will not cause a version bump for any packages. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review. 📝 WalkthroughWalkthroughRouter-core now normalizes duplicate slashes during path resolution and applies trailing-slash settings directly. Location construction and parameter updates preserve shared route parameters during stringification and updates. ChangesRouter path and parameter handling
Estimated code review effort: 3 (Moderate) | ~20 minutes Merge Risk: ⚪ Minimal · up to This PR optimizes router path resolution and parameter handling without introducing a known user-facing correctness or production risk; no actionable merge-blocking risk remains beyond normal checks and review. Possibly related PRs
🚥 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 |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 931e51a568
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
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 regress 0 benchmarks
|
Inspired by #8086
Summary
resolvePath, avoiding the LRU,split,join, and final regex passbuildLocationdestinations in one pass instead of first resolvingfromagainst./a//b+../../c->/cThe production diff is ten net lines:
path.tsadds 14 androuter.tsremoves 4. Benchmark source is intentionally not included in this PR.Benchmark setup
The benchmark constructs a real
RouterCorewith a 1,200-route tree and executes 1,200buildLocation()calls per sample. Production, browser-conditioned bundles run in Node withcreateMemoryHistoryandRouterCoreconfigured as server. Baseline (main) and the final candidate were bundled separately, loaded into the same process, and alternated for 1,500-2,000 samples. Each case was also rerun with candidate/baseline construction order reversed.Times below are median milliseconds per 1,200
buildLocation()calls. Arrows are baseline -> candidate.Important wins
The mixed-unique case intentionally exceeds the 1,000-entry resolve-path LRU. The large win comes from one-off absolute destinations bypassing the LRU instead of evicting useful relative-path entries. It is a large-app/cache-pressure case, not the expected gain for every navigation.
The unique-relative case also exceeds the LRU and measures the final redundant
cleanPathscan on every relative cache miss. Repeated relative paths that remain cached do not receive this additional 3.8-4.7% gain.The repository Vitest benchmark, rerun in production mode in separate processes, also improved:
Neutral case
The sub-percent differences should be treated as no demonstrated change.
Bundle size
react-router.minimalcompared withmain:Test plan
pnpm nx run @tanstack/router-core:test:unit -- tests/path.test.ts tests/build-location.test.ts tests/optional-path-params.test.ts tests/optional-path-params-clean.test.tspnpm nx run @tanstack/router-core:test:typespnpm nx run @tanstack/router-core:test:eslintreact-router.minimalbundle-size comparisonSummary by CodeRabbit
Bug Fixes
Tests