perf(react-router): avoid redundant Link state prop allocations - #8311
Conversation
Links resolve one state-prop bag instead of separate active and inactive bags. Class names do not need temporary arrays. Unchanged styles retain their original references. Callback behavior and prop precedence remain unchanged. React Link rendering benchmark, measured separately for this commit and its parent using exact production builds: - Workload: benchmarks/client-nav/scenarios/links/react/speed.bench.ts (200 persistent Links, eight navigations per measured batch). - Apple M4, Node 24.20.0, Vitest 4.1.4, NODE_ENV=production. - 8 fresh parent processes and 8 fresh processes for this commit, each with warmupIterations=50 and time=10000 ms; counterbalanced run order. - Parent mean times (ms): 3.9412, 3.8998, 3.8950, 4.2674, 4.0423, 3.8793, 3.9043, 4.1120. - This commit mean times (ms): 3.8767, 3.8900, 4.5507, 4.0116, 3.8297, 3.8582, 3.8879, 4.0157. - Median run means: 3.9228 -> 3.8889 ms. - Incremental effect: 0.86% less time; throughput change +0.87%. - Largest within-run RME for this revision: 3.08%. - The small difference is not a reliable speedup claim; it is close to process-to-process variation. Incremental minimal/full React gzip impact: -12/-7 bytes. Benchmark sources and commit implementation trees are unchanged. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
|
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 (3)
Included review availability: Your plan provides up to 10 included reviews per hour; 8 remain after this review. 📝 WalkthroughWalkthrough
ChangesLink state prop resolution
Priority: ⬇️ Low Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk: ⚪ Minimal · up to Link rendering now avoids redundant state-prop allocations while preserving active and inactive class/style behavior. No current merge-blocking risk remains. Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 2 files. (1 skipped: 1 unsupported.)
✨ Finishing Touches 💡 1📝 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 238fe17
☁️ Nx Cloud last updated this comment at |
🚀 Changeset Version Preview8 package(s) bumped directly, 18 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. |
Links resolve one state-prop bag instead of separate active and inactive bags. Class names do not need temporary arrays. Unchanged styles retain their original references. Callback behavior and prop precedence remain unchanged.
React Link rendering benchmark, measured separately for this commit and its parent using exact production builds:
Incremental minimal/full React gzip impact: -12/-7 bytes. Benchmark sources and commit implementation trees are unchanged.
🎯 Changes
✅ Checklist
🚀 Release Impact
Summary by CodeRabbit
Bug Fixes
Tests