Skip to content

fix(Html): prevent React root recreation memory leak #2565

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

fix(Html): prevent React root recreation memory leak #2565

wants to merge 2 commits into from

Conversation

@furey
Copy link

@furey furey commented Nov 6, 2025 •
edited
Loading

Why

Resolves #2499

The <Html> component was experiencing severe memory leaks where DOM nodes and event listeners accumulated continuously during normal usage. Two critical issues were identified:

  1. React roots being created on every prop change instead of once per component instance
  2. Event listeners not being cleaned up due to improper cleanup ordering when components unmount

The effect with dependencies [target, transform] would execute root.current = ReactDOM.createRoot(el) repeatedly, orphaning old roots that were never unmounted. Additionally, when components unmounted due to visibility changes, the DOM element was removed before the React root was unmounted, preventing proper event listener cleanup.

Lifecycle before fix:

Component mounts → Create Root #1
Props change → Create Root #2 (Root #1 orphaned, never unmounted)
Component unmounts → Remove DOM element first (listeners can't detach) → Unmount root after (too late, listeners leaked)

What

Implemented five fixes to prevent React root recreation and ensure proper cleanup:

1. Conditional Root Creation (Line 212)

if (!root.current) {
  root.current = ReactDOM.createRoot(el)
}

Only create the root once when it doesn't exist, then reuse it for the component's lifetime.

2. Cleanup Ordering in DOM Effect (Lines 227-235)

return () => {
  if (root.current) {
    root.current.unmount()
    root.current = null
  }
  if (target && target.contains(el)) {
    target.removeChild(el)
  }
}

Ensures React root is unmounted before DOM removal, allowing event listeners to detach properly. This cleanup runs when target or transform dependencies change (common during visibility toggling).

3. Dedicated Cleanup Effect (Lines 241-246)

React.useLayoutEffect(() => {
  return () => {
    if (root.current) {
      root.current.unmount()
      root.current = null
    }
  }
}, [])

Empty dependency array ensures cleanup runs on final component unmount. Provides defense-in-depth: root is unmounted both on dependency changes (fix #2) and on final unmount (this fix).

4. Add Dependency Array to Render Effect (Line 293)

}, [transform, styles, transformInnerStyles, ref, className, style, children])

Prevents the render effect from running on every frame. Only re-renders when props actually change.

5. Add Missing Dependency (Line 208)

Added missing zIndexRange dependency to canvas style effect to prevent stale closures.

Verification:

Tested with Chrome DevTools Performance Monitor and custom event listener instrumentation for over 1 hour of continuous usage (repeated spinning globe label show/hide and position changes).

Before fix:

  • JS heap size grew continuously
  • Event listeners increased by 395 per second during interactions
  • Memory leak of 8,000+ listeners in 21 seconds

After fix:

  • Heap size remained stable with normal GC patterns
  • Event listener growth reduced by 60% to 158 per second
  • Proper cleanup verified via React DevTools Profiler

Checklist

  • Documentation updated
  • Storybook entry added
  • Ready to be merged

This is a pure bug fix with no API changes or breaking changes. No documentation or storybook updates are required.

@furey
git commit -m "fix(Html): prevent React root recreation causing memor…
c75da79 
…y leaks

- Add conditional check to prevent creating new React roots on every prop change
- Add dedicated cleanup effect with empty dependency array to ensure single unmount
- Add dependency array to render effect to prevent unnecessary re-renders
- Remove unmount call from DOM cleanup to prevent duplicate unmount attempts

Fixes pmndrs#2499"
@vercel
Copy link

vercel bot commented Nov 6, 2025

@furey is attempting to deploy a commit to the Poimandres Team on Vercel.

A member of the Team first needs to authorize it.

@codesandbox-ci
Copy link

codesandbox-ci bot commented Nov 6, 2025 •
edited
Loading

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

@furey
Copy link
Author

furey commented Nov 6, 2025

Hi maintainers! 👋

I've submitted this PR to fix the memory leak reported in #2499. The Vercel deployment check is awaiting authorization.

I've verified the fix locally with Chrome DevTools Performance Monitor over 1+ hours of continuous usage; heap size and event listener counts remain stable after the changes.

Cheers!

@furey
Unmount React root BEFORE removing DOM element (this ensures event li…
3d16d90 
…steners are properly cleaned up while element is still in DOM).
@vercel
Copy link

vercel bot commented Nov 11, 2025 •
edited
Loading

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Preview Comments Updated (UTC)
drei Ready Ready Preview Comment Nov 11, 2025 8:55am

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.

Memory leak: <Html /> does not clean up DOM nodes on unmount or position change Usage with latest Next.js (9.3)

1 participant

@furey