Skip to content

fix: guard portal setState against React 19 Suspense disappearLayoutEffects#7474

Open
mixelburg wants to merge 1 commit into
recharts:mainfrom
mixelburg:fix/react19-suspense-update-depth
Open

fix: guard portal setState against React 19 Suspense disappearLayoutEffects#7474
mixelburg wants to merge 1 commit into
recharts:mainfrom
mixelburg:fix/react19-suspense-update-depth

Conversation

@mixelburg

@mixelburg mixelburg commented Jun 19, 2026 •
edited by coderabbitai Bot
Loading

Copy link
Copy Markdown
Contributor

Fixes #7463

React 19's disappearLayoutEffects phase calls ref callbacks with null when a Suspense boundary hides a subtree. The innerRef callback in RechartsWrapper was calling setTooltipPortal(null) and setLegendPortal(null) unconditionally, which triggered setState during ref cleanup and caused an infinite re-render loop.

Moved both portal setters inside the existing node != null guard so they only fire on mount, not cleanup. This is a minimal 2-line change matching the fix direction suggested in the issue.

Summary by CodeRabbit

  • Bug Fixes
    • Improved chart component state management during unmount to ensure proper cleanup of internal UI elements.

@mixelburg
fix(RechartsWrapper): guard portal setState against React 19 Suspense…
ddf46a0 
… disappearLayoutEffects

Move setTooltipPortal and setLegendPortal calls inside the existing
node != null check so they are not called with null during ref cleanup.
React 19's disappearLayoutEffects phase calls ref callbacks with null
during Suspense hiding, and unconditional setState in that phase causes
infinite re-render loops.
@coderabbitai

coderabbitai Bot commented Jun 19, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: f3f40cbf-7ad2-41c7-8b35-02632ef92990

📥 Commits

Reviewing files that changed from the base of the PR and between 4f27699 and ddf46a0.

📒 Files selected for processing (1)
  • src/chart/RechartsWrapper.tsx

Walkthrough

In RechartsWrapper.tsx, the innerRef ref callback now gates setTooltipPortal and setLegendPortal calls behind a node != null check. Previously these setters were called unconditionally, including with null during React 19 Suspense boundary ref teardown (disappearLayoutEffects), causing an infinite setState loop.

Changes

Portal null guard in innerRef

Layer / File(s) Summary
Gate portal setters on non-null node
src/chart/RechartsWrapper.tsx		 setTooltipPortal and setLegendPortal are now only called when node != null inside the innerRef callback, preventing setState calls during React 19 Suspense ref cleanup that previously caused infinite re-renders.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~5 minutes

Suggested reviewers

  • ckifer
  • PavelVanecek
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly describes the main fix: guarding portal state setters against React 19's Suspense disappearLayoutEffects behavior.
Description check ✅ Passed The description adequately explains the issue, the root cause, and the fix applied. It links to the related issue and describes the React 19 behavior that triggered the problem.
Linked Issues check ✅ Passed The PR addresses all requirements from issue #7463: it guards setState calls with a null check on the node parameter to prevent infinite loops during React 19's disappearLayoutEffects phase.
Out of Scope Changes check ✅ Passed The changes are minimal and focused on the specific issue: only moving portal state setters inside the node null guard in RechartsWrapper.tsx, with no extraneous modifications.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@codecov

codecov Bot commented Jun 19, 2026

Copy link
Copy Markdown

Bundle Report

Changes will decrease total bundle size by 77.67kB (-1.39%) ⬇️. This is within the configured threshold ✅

Detailed changes
Bundle name Size Change
recharts/bundle-cjs 1.44MB 4 bytes (0.0%) ⬆️
recharts/bundle-treeshaking-sankey* 346.57kB -25.85kB (-6.94%) ⬇️
recharts/bundle-treeshaking-sunburst* 324.23kB -22.61kB (-6.52%) ⬇️
recharts/bundle-es6 1.27MB 4 bytes (0.0%) ⬆️
recharts/bundle-treeshaking-treemap* 357.36kB -29.22kB (-7.56%) ⬇️

ℹ️ *Bundle size includes cached data from a previous commit

Affected Assets, Files, and Routes:

view changes for bundle: recharts/bundle-treeshaking-sankey

Assets Changed:

Asset Name Size Change Total Size Change (%)
bundle.js -25.85kB 346.57kB -6.94%
view changes for bundle: recharts/bundle-es6

Assets Changed:

Asset Name Size Change Total Size Change (%)
chart/RechartsWrapper.js 4 bytes 16.1kB 0.02%
view changes for bundle: recharts/bundle-treeshaking-sunburst

Assets Changed:

Asset Name Size Change Total Size Change (%)
bundle.js -22.61kB 324.23kB -6.52%
view changes for bundle: recharts/bundle-cjs

Assets Changed:

Asset Name Size Change Total Size Change (%)
chart/RechartsWrapper.js 4 bytes 17.95kB 0.02%
view changes for bundle: recharts/bundle-treeshaking-treemap

Assets Changed:

Asset Name Size Change Total Size Change (%)
bundle.js -29.22kB 357.36kB -7.56%

@codecov

codecov Bot commented Jun 19, 2026 •
edited
Loading

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 88.25%. Comparing base (fc96239) to head (ddf46a0).
⚠️ Report is 195 commits behind head on main.

Additional details and impacted files 
@@            Coverage Diff             @@
##             main    #7474      +/-   ##
==========================================
- Coverage   89.06%   88.25%   -0.81%     
==========================================
  Files         539      607      +68     
  Lines       41011    14102   -26909     
  Branches     5553     3544    -2009     
==========================================
- Hits        36527    12446   -24081     
+ Misses       4476     1469    -3007     
- Partials        8      187     +179

☔ View full report in Codecov by Harness.
📢 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.

@PavelVanecek

PavelVanecek commented Jun 20, 2026

Copy link
Copy Markdown
Collaborator

Can we have a unit test for this?

@mixelburg

mixelburg commented Jun 26, 2026

Copy link
Copy Markdown
Contributor Author

Good call, will add a unit test covering the Suspense disappearLayoutEffects scenario. Stay tuned!

@elleszabo

elleszabo commented Jul 13, 2026

Copy link
Copy Markdown

bumping this, would be great

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Maximum update depth exceeded when chart unmounts behind Suspense boundary (React 19)

3 participants

@mixelburg @PavelVanecek @elleszabo