-
Notifications
You must be signed in to change notification settings - Fork 187
feat(performance): implement React.memo for critical components #2832
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
- Add React.memo to ValidatorList - Optimise Pool component with pool-specific prop comparisons - Memoise 5 chart components to prevent Chart.js re-initializations Improves list interactions and chart performance significantly
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR improves UI performance by wrapping key list and chart components with React.memo and custom comparison logic to minimize unnecessary re-renders.
- Introduces React.memo with custom
arePropsEqualfunctions for charts and list items. - Refactors component exports to use memoized variants.
- Optimizes Pool and ValidatorList components with tailored prop checks.
Reviewed Changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/ui-graphs/src/graphs/PayoutLine.tsx | Renamed component, added memo and custom arePropsEqual |
| packages/ui-graphs/src/graphs/PayoutBar.tsx | Wrapped Bar chart in memo with tailored prop comparisons |
| packages/ui-graphs/src/graphs/GeoDonut.tsx | Added memo for Doughnut chart with deep prop checks |
| packages/ui-graphs/src/graphs/EraPointsLine.tsx | Memoized Line chart component with custom equality check |
| packages/ui-graphs/src/graphs/AveragePayoutLine.tsx | Wrapped AveragePayoutLine in memo, removed some tooltip config |
| packages/app/src/library/ValidatorList/Item.tsx | Memoized Validator list item with selective prop comparisons |
| packages/app/src/library/Pool/index.tsx | Added memo to Pool component, implemented arePropsEqual |
Comments suppressed due to low confidence (2)
packages/ui-graphs/src/graphs/AveragePayoutLine.tsx:133
- The removal of
intersect: falseand theinteraction.modeconfiguration will alter tooltip behavior (it may now only show when hovering exactly over points). Please verify this change against UX requirements or restore these options if the previous behavior was intended.
.toFormat()} ${unit}`,
packages/ui-graphs/src/graphs/PayoutLine.tsx:165
- There are no tests covering this custom
arePropsEquallogic. Please add unit tests to ensure it returnsfalsewhen critical props change andtrueotherwise.
const arePropsEqual = (
- Replace JSON.stringify with efficient deepEqual utility in arePropsEqual functions - Add displayName properties to all memoized components for better debugging - Create reusable deep comparison utilities in ui-graphs package
|
Good optimisation - the app could definitely do with more I think the props comparison makes sense in most cases. React only checks shallow comparisons so objects will be forcing those re-renders. To tidy up the These utils could be defined in |
… Item memo - Added `deepEqual`, `boolEqual`, `stringEqual`, and `arePropsEqual` to `packages/utils/src/props.ts` for consistent prop comparison. - Refactored `ValidatorList/Item.tsx` to use new utilities in its `React.memo` custom comparison. - Reduces boilerplate and standardises prop equality checks across the app.
|
Thanks for the suggestion @rossbulat. I've implemented centralised prop comparison utilities (deepEqual, boolEqual, stringEqual, and arePropsEqual) in utils/props.ts, and refactored the ValidatorList/Item.tsx component to use them. This approach standardises our arePropsEqual logic, reduces boilerplate, and should help prevent unnecessary re-renders, especially where objects and arrays are passed as props. Would you like me to roll this out to other components as well? I see several potential opportunities, such as the custom memo comparison functions in our graph components (EraPointsLine, GeoDonut, PayoutLine, PayoutBar, AveragePayoutLine in ui-graphs), which could benefit from this consistency and optimisation. |
- No functional changes
Improves list interactions and chart performance.