perf(react-router): avoid unnecessary composed handlers - #8279
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
View your CI Pipeline Execution ↗ for commit 3e584cb
☁️ Nx Cloud last updated this comment at |
🚀 Changeset Version Preview1 package(s) bumped directly, 4 bumped as dependents. 🟩 Patch bumps
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Team Run ID: 📒 Files selected for processing (1)
Included review availability: Your plan provides up to 10 included reviews per hour; 8 remain after this review. 📝 WalkthroughWalkthroughReact Router now composes link event handlers with two arguments. It returns the internal handler directly when no user handler exists and preserves ChangesEvent handler composition
Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk: ⚪ Minimal · up to Link event handlers now avoid unnecessary allocations when no user handler is supplied while preserving cancellation and invocation behavior. The changed behavior is covered by focused tests and introduces no active merge-blocking risk. Suggested reviewers: 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
Full details: Description checkExplanation The description explains the implementation, motivation, performance data, tests, and validation results. It does not follow the required template because it omits the Changes, Checklist, and Release Impact sections, including the required checklist items and changeset declaration. ✨ 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 |
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. |
Summary
Avoid allocating six event-handler arrays and wrapper closures for links that do not provide user event handlers.
The two-argument
composeHandlersreturns the internal handler directly when the user handler is absent. When a user handler is present, it preserves the existing ordering anddefaultPreventedsemantics. The wrapper uses a short-circuit expression so the emitted handler remains compact; the suggested nestedifform minifies to the same output as the original block form.Performance
Focused client navigation benchmark using the React links scenario (200 links, repeated navigation loop; same local environment, one run per version):
The runtime result is effectively neutral within benchmark noise. The important workload improvement is avoiding per-link allocations during render.
Bundle-size benchmark for
react-router.minimal:The compact expression improves the committed implementation by 3 gzip bytes (85,846 B -> 85,843 B), reducing the optimization's gzip impact to +2 bytes over the original array composer.
Tests
composeHandlersevent-semantics tests, including already-prevented events.@tanstack/react-router:test:unit— 1,097 passed, 1 skipped.@tanstack/react-router:test:types— passed.@tanstack/react-router:test:eslint— passed.Summary by CodeRabbit
Performance
Bug Fixes