fix(ImageTrail): stop the render loop and drop listeners on unmount - #1026
Open
noron12234 wants to merge 3 commits into
Open
fix(ImageTrail): stop the render loop and drop listeners on unmount#1026noron12234 wants to merge 3 commits into
noron12234 wants to merge 3 commits into
Conversation
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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The bug
ImageTrail's effect constructs a variant class and returns nothing:Every one of the eight variants starts a self-scheduling loop:
and every
ImageItemregisters a listener that is never removed:So unmounting the component leaves the rAF loop running forever, still
calling
lerp,gsap.killTweensOfandgsap.timelineagainst detachednodes, plus one leaked
windowlistener per image and two leakedcontainer listeners per instance.
Why it compounds
The dependency list is
[variant, items]anditemsis an array prop. Aparent rendering
<ImageTrail items={[...]} />inline hands over a freshreference 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
ImageItemgainsdestroy()to unregister itsresizehandler.so they can actually be released, and
render()returns early oncedestroyed.
destroy(): cancel the pending frame, remove thefour container listeners, kill the GSAP tweens still targeting the
images, dispose every
ImageItem.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 theStrictMode rAF fix for
LineSidebarandOptionWheel(cleanup nullsrafRef.currentso the loop can restart), butpublic/r/*.jsonwasnever regenerated after it landed, so it still holds the pre-fix shape:
with no
rafRef.current = nullin the cleanup. Since the registry iswhat 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
eslintclean on both JS variants (--max-warnings 0, exit 0).tsc --noEmitreports zeroImageTraildiagnostics.npm run registry:build; the only artifactstouched are the four
ImageTrailfiles plus the eight staleLineSidebar/OptionWheelfiles described above.