Investigation: virtualized-list memory leak (#3241)

[mattgperry]

Status: draft — investigation only, no code fix applied

Refs #3241

The reporter says scrolling a virtualized list of motion.div items with
initial={{ opacity: 0 }} animate={{ opacity: 1 }} causes the Chrome DevTools
"DOM Nodes" counter to climb without falling, even after manual GC. Removing
initial/animate stops the growth.

What I did

• Read the issue and the linked CodeSandbox URLs (nj8cqp). The sandbox
  source is gated behind Cloudflare's bot challenge — WebFetch,
  curl, and the CSB API endpoints all returned the challenge page, so I
  could not retrieve the original App.js.
• Traced the unmount path for plain animated motion.div:
  • useMotionRef calls visualElement.unmount() when React passes
    null to the ref callback (packages/framer-motion/src/motion/utils/use-motion-ref.ts:39).
  • VisualElement.unmount() cancels both scheduled frame callbacks,
    runs valueSubscriptions cleanup (which calls value.stop() for
    owned MotionValues, in turn calling animation.stop() →
    animation.cancel() for WAAPI), unmounts each feature
    (AnimationFeature.unmount() → animationState.reset()), and
    nulls this.current (packages/motion-dom/src/render/VisualElement.ts:504-525).
  • For plain animate on a motion.div neither projection nor
    InViewFeature is enabled (packages/framer-motion/src/motion/features/definitions.ts),
    so those teardown paths aren't relevant.
• Surveyed module-level collections that could retain DOM nodes
  (animationMaps, appearAnimationStore, observerCallbacks, frame-step
  queues). All are WeakMaps or are cleared per frame. The optimised
  appear store only populates when data-appear-id is present (SSR), so
  CRA-style sandboxes don't touch it.
• Reviewed recent leak-related commits (#3178 release-visual-element,
  #3453 reduced-motion listener, #3381 React 19 cleanup, popchild-refs,
  drag-cleanup). The unmount path has been touched repeatedly since
  this issue was filed.
• Existing test animate-prop.test.tsx > unmount cancels active animations
  already proves onAnimationComplete is not invoked after unmount.
• Issue comment from contributor @rortan134 (2025-08-11): "This seems
  to be fixed? … snapshots don't seem to be hinting any leaks."

Where I got stuck

I cannot reliably reproduce the leak:

• The JSDOM-based unit test environment doesn't run WAAPI, so cancel-on-unmount
  can't be observed there.
• Cypress runs in Electron 80, where document.getAnimations() is
  unavailable and detached-DOM accounting requires Chrome DevTools'
  Performance Monitor — not addressable from a test runner.
• Without the original sandbox source I can't tell whether the original
  reporter's leak was caused by a third-party virtualizer holding refs,
  by a bug since fixed, or by Chrome behaviour around recently-cancelled
  WAAPI animations.

Per feedback_no_repro_no_pr.md: not landing happy-path coverage that
can't fail without a fix. No source change is included here.

What this PR adds

A single manual reproduction page at
dev/react/src/tests/animate-virtualized-list-memory.tsx (route
?test=animate-virtualized-list-memory). It cycles a window of 10
motion.div items every 50ms, simulating the virtualized scroll
described in the issue. A maintainer can open it in Chrome with the
Performance Monitor visible to see whether the DOM Nodes counter
stabilises — useful for confirming whether the bug is still live before
landing a fix.

Suggested next step

Confirm reproducibility against current main using the harness
(or the original CodeSandbox if accessible). If memory stabilises,
close #3241 as already-fixed; if it grows, the harness gives a
controllable starting point that doesn't depend on a third-party
virtualizer or external sandbox.

[greptile-apps]

Greptile Summary

This draft PR adds a single manual reproduction harness (dev/react/src/tests/animate-virtualized-list-memory.tsx) for issue #3241, which reports a DOM-node memory leak when scrolling animated motion.div items in a virtualized list. No production code is changed; the file is intended for a maintainer to open in Chrome with the Performance Monitor to verify whether the leak is still present on main.

• A FinalizationRegistry-based stat tracker (window.__leakStats) is added so a tester can force GC in DevTools and check whether collected DOM nodes match the expected visible count after cycling the virtual window.
• The harness auto-scrolls a 50-item virtual list at 30 ms intervals using setInterval + React state, mounting/unmounting items as the visible window moves — closely mirroring the reported reproduction scenario.
• No Cypress spec is included, explicitly acknowledged in the PR description as intentional given no reproducible failure exists to gate against.

Confidence Score: 3/5

Safe to merge as a dev-only harness with no production code changes, but the measurement logic has a flaw that would mislead any maintainer trying to use it today.

The inline ref callback fires totalUnmounted++ on every parent re-render for each still-visible item, not just on actual unmounts. At 30 ms intervals with ~4 visible items the counter over-counts by roughly 130 false events per second, completely drowning out the real signal. A maintainer following the DevTools workflow described in the PR comment block would read a totalUnmounted value that is wildly inflated and could draw wrong conclusions about the leak severity.

dev/react/src/tests/animate-virtualized-list-memory.tsx — specifically the setRef callback and the stat display logic.

Important Files Changed

Filename	Overview
dev/react/src/tests/animate-virtualized-list-memory.tsx	Manual reproduction harness for issue #3241; contains a logic bug where the inline ref callback inflates totalUnmounted on every re-render rather than only on genuine unmounts, making the harness stats misleading.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[setInterval 30ms] -->|increment cycleRef| B[setScrollTop]
    B --> C[Re-render App]
    C --> D[Compute visible window]
    D --> E{Item in visible range?}
    E -->|Yes same key| F[Re-render ListItem new setRef identity]
    E -->|No longer visible| G[Unmount ListItem]
    E -->|New in range| H[Mount ListItem]
    F -->|React ref cleanup| I[setRef null totalUnmounted++ even though still mounted]
    G --> K[setRef null totalUnmounted++ genuine unmount]
    H --> L[setRef el register FinalizationRegistry]
    L --> M[window.__leakStats updated]
    K --> M
    I --> N[totalUnmounted inflated]

Loading

_{Reviews (1): Last reviewed commit: "test: enhance #3241 repro harness with F..." | Re-trigger Greptile}

[greptile-apps]

Inline ref callback inflates totalUnmounted on every re-render

Because setRef is defined inline (new function identity on every render), React calls the old setRef(null) before the new setRef(el) on every parent re-render. The null branch hits idRef.current (still truthy, never cleared) and increments totalUnmounted — even though the element was never actually detached. At 30 ms intervals with 4 visible items the counter accumulates roughly 4 × (1000/30) ≈ 133 false unmount events per second, swamping the real signal. Wrapping setRef in useCallback with an empty deps array would fix this: React would only call it on genuine mount/unmount, not on every re-render.

[greptile-apps]

Module-level counters survive HMR reloads

totalMounted, totalUnmounted, idCounter, and liveIds are module-level singletons. In the Vite dev server with HMR, module code doesn't re-execute on hot reload — only the React component tree is re-mounted. After any HMR update the counters carry over stale values from the previous run, which makes a second measurement session impossible without a hard page reload. Adding a window.__resetLeakStats helper would let testers restart the measurement without a hard page reload.

[greptile-apps]

liveIds.size display lags after FinalizationRegistry callbacks

The live: {liveIds.size} value in the rendered DOM is captured at render time. FinalizationRegistry callbacks fire asynchronously and don't trigger a React re-render, so the on-screen live: number can be stale long after GC has collected nodes. The PR description already directs testers toward window.__leakStats, but a note in the JSDoc that the on-screen counter lags would avoid confusion.