Skip to content

fix(ImageTrail): stop the render loop and drop listeners on unmount - #1026

Open
noron12234 wants to merge 3 commits into
DavidHDev:mainfrom
noron12234:fix/unmount-animation-loop-leaks
Open

fix(ImageTrail): stop the render loop and drop listeners on unmount#1026
noron12234 wants to merge 3 commits into
DavidHDev:mainfrom
noron12234:fix/unmount-animation-loop-leaks

Conversation

@noron12234

Copy link
Copy Markdown

The bug

ImageTrail's effect constructs a variant class and returns nothing:

useEffect(() => {
  if (!containerRef.current) return;
  const Cls = variantMap[variant] || variantMap[1];
  new Cls(containerRef.current);       // no handle kept, no cleanup
}, [variant, items]);

Every one of the eight variants starts a self-scheduling loop:

render() {
  ...
  requestAnimationFrame(() => this.render());   // never cancelled
}

and every ImageItem registers a listener that is never removed:

window.addEventListener('resize', this.resize);

So unmounting the component leaves the rAF loop running forever, still
calling lerp, gsap.killTweensOf and gsap.timeline against detached
nodes, plus one leaked window listener per image and two leaked
container listeners per instance.

Why it compounds

The dependency list is [variant, items] and items is an array prop. A
parent rendering <ImageTrail items={[...]} /> inline hands over a fresh
reference on every render, so the effect re-runs and builds another
instance. The previous one is never released, so the leak accumulates
with parent renders rather than being a single stranded loop. React
StrictMode makes it visible immediately: two live loops after the first
mount.

The fix

  • ImageItem gains destroy() to unregister its resize handler.
  • Each variant keeps the pending frame id and the two container handlers
    so they can actually be released, and render() returns early once
    destroyed.
  • Each variant gains destroy(): cancel the pending frame, remove the
    four container listeners, kill the GSAP tweens still targeting the
    images, dispose every ImageItem.
  • The effect keeps the instance and returns a cleanup that calls it.

No behavioural change while mounted. Applied to all four variants of the
component (JS/TS x CSS/Tailwind).

Third commit: a fix that never shipped

Rebuilding the registry surfaced something separate. src/ carries the
StrictMode rAF fix for LineSidebar and OptionWheel (cleanup nulls
rafRef.current so the loop can restart), but public/r/*.json was
never regenerated after it landed, so it still holds the pre-fix shape:

if (rafRef.current != null) return;   // still in public/r/*.json today

with no rafRef.current = null in the cleanup. Since the registry is
what the jsrepo CLI writes into a user's project, that fix is currently
not reaching anyone installing through the CLI. Regenerated with
npm run registry:build; no source change in that commit.

Verification

  • eslint clean on both JS variants (--max-warnings 0, exit 0).
  • tsc --noEmit reports zero ImageTrail diagnostics.
  • Registry regenerated with npm run registry:build; the only artifacts
    touched are the four ImageTrail files plus the eight stale
    LineSidebar/OptionWheel files described above.

The effect instantiates a variant class and returns nothing, so nothing
is ever torn down. Each of the eight variants starts a self-scheduling
`requestAnimationFrame(() => this.render())` loop and every `ImageItem`
registers a `resize` handler on `window`. Both outlive the component.

The effect list is `[variant, items]`, and `items` is an array prop, so a
parent that renders an inline array gets a fresh reference on every
render. That re-runs the effect and constructs another instance, each one
adding its own permanent rAF loop, its own window listener and its own
pair of container listeners. Nothing releases the previous instance, so
the cost is cumulative rather than a single stranded loop.

`ImageItem` now exposes `destroy()` to unregister its resize handler, the
variants keep the frame id and the two container handlers so they can be
released, `render()` returns early once destroyed, and `destroy()`
cancels the pending frame, removes the listeners, kills the GSAP tweens
still targeting the images and disposes each item. The effect returns a
cleanup that calls it.

Applied to all four variants of the component (JS/TS x CSS/Tailwind).
public/r/*.json embeds the component source verbatim and is what the
jsrepo CLI writes into a user's project, so a source-only fix would still
ship the leaking version.
`src/` carries the StrictMode rAF fix (cleanup nulls `rafRef.current` so
the loop can restart), but the published registry was never regenerated
after it landed. The shipped payload still has the pre-fix shape:

    if (rafRef.current != null) return;   // old, in public/r/*.json
    ...                                   // and no `rafRef.current = null`
                                          // in the cleanup

So anyone installing LineSidebar or OptionWheel through the CLI still
receives the version whose animation never restarts after a StrictMode
remount. Regenerated with `npm run registry:build`; no source change.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant