Skip to content

Fix infinite rendering loops#6247

Merged
ckifer merged 2 commits intomainfrom
infinite-hook-loop
Aug 24, 2025
Merged

Fix infinite rendering loops#6247
ckifer merged 2 commits intomainfrom
infinite-hook-loop

Conversation

@PavelVanecek
Copy link
Collaborator

Description

Shuffle code around to better leverage memoization.

Related Issue

Fixes #6211

Fixes #6236

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have added tests to cover my changes.
  • I have added a storybook story or VR test, or extended an existing story or VR test to show my changes

Copilot AI review requested due to automatic review settings August 24, 2025 08:03
Copy link
Contributor

Copilot AI left a 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 fixes infinite rendering loops by refactoring tooltip and offset calculations to better leverage memoization. The changes extract axis data key selection into a memoized selector and optimize chart offset calculations by breaking down complex computations into smaller, memoizable functions.

  • Replaces direct axis object usage with memoized data key selection in tooltip payload logic
  • Refactors chart offset calculations to use separate functions for each axis direction
  • Updates test assertions to reflect the reduced render counts achieved through better memoization

Reviewed Changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated no comments.

Show a summary per file
File Description
src/state/selectors/selectTooltipAxis.ts Adds new memoized selector for tooltip axis data key
src/state/selectors/tooltipSelectors.ts Updates to use the new data key selector instead of full axis object
src/state/selectors/selectors.ts Updates tooltip payload selector to use data key selector
src/state/selectors/combiners/combineTooltipPayload.ts Changes parameter from axis object to data key for better memoization
src/state/selectors/selectChartOffsetInternal.ts Refactors offset calculation into separate functions for each axis direction
test/state/selectors/selectors.spec.tsx Updates tests to pass data key directly instead of axis object
test/hooks/useOffset.spec.tsx Adds test for infinite rendering fix and updates render count assertions
test/context/chartLayoutContext.spec.tsx Updates render count assertion to reflect memoization improvements

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@codecov
Copy link

codecov bot commented Aug 24, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 96.64%. Comparing base (71521bc) to head (1c0e921).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #6247   +/-   ##
=======================================
  Coverage   96.63%   96.64%           
=======================================
  Files         221      221           
  Lines       19898    19931   +33     
  Branches     4093     4107   +14     
=======================================
+ Hits        19229    19262   +33     
  Misses        663      663           
  Partials        6        6           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@codecov
Copy link

codecov bot commented Aug 24, 2025

Bundle Report

Changes will increase total bundle size by 1.85kB (0.08%) ⬆️. This is within the configured threshold ✅

Detailed changes
Bundle name Size Change
recharts/bundle-cjs 1.03MB 815 bytes (0.08%) ⬆️
recharts/bundle-es6 885.03kB 843 bytes (0.1%) ⬆️
recharts/bundle-umd 484.12kB 189 bytes (0.04%) ⬆️

Affected Assets, Files, and Routes:

view changes for bundle: recharts/bundle-umd

Assets Changed:

Asset Name Size Change Total Size Change (%)
Recharts.js 189 bytes 484.12kB 0.04%
view changes for bundle: recharts/bundle-cjs

Assets Changed:

Asset Name Size Change Total Size Change (%)
state/selectors/tooltipSelectors.js 7 bytes 15.44kB 0.05%
state/selectors/combiners/combineTooltipPayload.js -104 bytes 6.95kB -1.47%
state/selectors/selectors.js 7 bytes 6.38kB 0.11%
state/selectors/selectChartOffsetInternal.js 649 bytes 5.09kB 14.63% ⚠️
state/selectors/selectTooltipAxis.js 256 bytes 849 bytes 43.17% ⚠️
view changes for bundle: recharts/bundle-es6

Assets Changed:

Asset Name Size Change Total Size Change (%)
state/selectors/tooltipSelectors.js 33 bytes 12.16kB 0.27%
state/selectors/combiners/combineTooltipPayload.js -104 bytes 6.69kB -1.53%
state/selectors/selectors.js 14 bytes 4.82kB 0.29%
state/selectors/selectChartOffsetInternal.js 716 bytes 4.57kB 18.6% ⚠️
state/selectors/selectTooltipAxis.js 184 bytes 552 bytes 50.0% ⚠️

@ckifer
Copy link
Member

ckifer commented Aug 24, 2025

What a precarious thing

@ckifer ckifer merged commit 58f669e into main Aug 24, 2025
27 checks passed
@PavelVanecek PavelVanecek deleted the infinite-hook-loop branch August 25, 2025 09:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

3 participants