Optimize Bar component rendering when activeBar is falsy#6290
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR optimizes the Bar component rendering when the activeBar prop is falsy by conditionally using hooks only when necessary. The optimization prevents unnecessary selector calls and React reconciliation when bars don't need active state tracking.
- Extracts Bar rectangle rendering into separate components with and without active state hooks
- Optimizes animation ID generation by memoizing only relevant properties
- Refactors SVG properties filtering to use a reusable type alias
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/util/svgPropertiesNoEvents.ts | Adds type alias for cleaner return type definition |
| src/shape/Rectangle.tsx | Optimizes animation ID generation by memoizing only animation-relevant properties |
| src/cartesian/Bar.tsx | Splits bar rectangle rendering into conditional components to avoid unnecessary hook calls when activeBar is falsy |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #6290 +/- ##
=======================================
Coverage 96.63% 96.63%
=======================================
Files 221 221
Lines 20195 20205 +10
Branches 4145 4150 +5
=======================================
+ Hits 19515 19525 +10
Misses 673 673
Partials 7 7 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Bundle ReportChanges will increase total bundle size by 3.46kB (0.14%) ⬆️. This is within the configured threshold ✅ Detailed changes
Affected Assets, Files, and Routes:view changes for bundle: recharts/bundle-cjsAssets Changed:
view changes for bundle: recharts/bundle-es6Assets Changed:
view changes for bundle: recharts/bundle-umdAssets Changed:
|
|
Perhaps we could use https://react.dev/reference/react/memo for the activeBar case, let me try |
|
will merge this as is |
|
Legend |
Description
Do not read which index is active if we know we will never use this information anyway.
This optimizes the chart when
activeBaris falsy. It remains slow whenactiveBaris set! But it is the same slow in 2.x.Related Issue
Fixes #6267
How Has This Been Tested?
Exhibit A, before any changes
Mouse hover - 177m jank
Lot of time spent in rendering Rectangle - updateFunctionComponent 41% (256ms)
Chart takes about 4 seconds to load before it's interactive.
Exhibit B: with optimized animation ID dependencies
Only generate new animation IDs when props that actually animate change.
Mouse hover - 50-110ms jank
updateFunctionComponent still high 52% (539ms)
Feels about the same in the browser, no subjective improvement.
4.5s to interactive.
Exhibit C: with hooks extracted to child component
introduce
BarRectangleWithActiveStatethat calls the selectActiveTooltipIndex selector in a child, not in a loop.Mouse hover - 50-110ms jank
updateFunctionComponent still high - 46% 416ms
This subjectively feels faster but profile says it's about the same.
3.9s to interactive.
Exhibit D: do not call hooks if active prop is not set
Adds BarRectangleNeverActive so that we never call any hooks if
activeBarprop is falsy.Looks like a jackpot, the Tooltip is as smooth as it gets on my 60fps screen.
Mouse hover - no jank
updateFunctionComponent Rectangle went away - it never updates at all. The Tooltip is still there, that has to stay.
3.8s to interactive, this didn't get any better.
Okay I'm happy with this. The chart remains slow with
activeBarprop but that's the same in 2.x so it's not a regression anymore.