fix: kill GSAP timeline instances in useEffect cleanup to prevent mem…#308
fix: kill GSAP timeline instances in useEffect cleanup to prevent mem…#308HamidKhan1001 wants to merge 1 commit intoAOSSIE-Org:mainfrom
Conversation
📝 WalkthroughWalkthroughThe component's cleanup function now explicitly calls Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/page.jsx (1)
84-106:⚠️ Potential issue | 🟠 MajorFeature-item timelines are not killed in cleanup — the memory leak this PR aims to fix persists for these.
Each loop iteration (lines 84–106) creates a
tltimeline that is registered in GSAP's global ticker. TheScrollTrigger.getAll().forEach((t) => t.kill())on line 223 kills the ScrollTrigger controllers but leaves the timeline objects themselves alive in GSAP's internal registry — exactly the same class of leak being fixed for the named timelines. Becausetlis block-scoped inside theforEachcallback, there is no reference available at cleanup time.The most idiomatic fix for this class of problem in React is
gsap.context(), which automatically tracks every GSAP object created inside it (including loop-local timelines) and cleans all of them up onctx.revert():♻️ Proposed fix using
gsap.context()— replaces the entire effect bodyuseEffect(() => { + const ctx = gsap.context(() => { // Initialize Lenis for smooth scrolling const lenis = new Lenis({ ... }); let rafId; let isActive = true; function raf(time) { if (!isActive) return; lenis.raf(time); rafId = requestAnimationFrame(raf); } rafId = requestAnimationFrame(raf); // ... all heroTl, feature forEach tl, techStackTl, aboutTl, downloadTl ... return () => { isActive = false; if (rafId != null) cancelAnimationFrame(rafId); lenis.destroy(); - heroTl.kill(); - techStackTl.kill(); - aboutTl.kill(); - downloadTl.kill(); ScrollTrigger.getAll().forEach((t) => t.kill()); }; + }); + + return () => ctx.revert(); // kills every GSAP object created inside the context }, []);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/page.jsx` around lines 84 - 106, The feature-item loop creates block-scoped timelines (tl) via gsap.utils.toArray(".feature-item") that are not tracked and thus not killed on cleanup; wrap the entire GSAP setup (the gsap.utils.toArray(...) forEach and tl creation) inside a gsap.context(() => { ... }) so all timelines and ScrollTriggers are automatically tracked, store the context in a variable (e.g., ctx) in the effect, and call ctx.revert() in the effect cleanup to ensure tl and related GSAP objects are disposed; keep the existing ScrollTrigger config but remove the need to manually kill timelines since ctx.revert() handles them.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@app/page.jsx`:
- Around line 84-106: The feature-item loop creates block-scoped timelines (tl)
via gsap.utils.toArray(".feature-item") that are not tracked and thus not killed
on cleanup; wrap the entire GSAP setup (the gsap.utils.toArray(...) forEach and
tl creation) inside a gsap.context(() => { ... }) so all timelines and
ScrollTriggers are automatically tracked, store the context in a variable (e.g.,
ctx) in the effect, and call ctx.revert() in the effect cleanup to ensure tl and
related GSAP objects are disposed; keep the existing ScrollTrigger config but
remove the need to manually kill timelines since ctx.revert() handles them.
fix: kill GSAP timeline instances in useEffect cleanup to prevent memory leak
Problem
Four GSAP timelines in
app/page.jsxwere never killed in theuseEffect cleanup function, causing memory to accumulate on every
remount/scroll.
Affected timelines:
heroTl(line 45)techStackTl(line 109)aboutTl(line 142)downloadTl(line 181)Evidence
Heap snapshots taken before and after scrolling show the leak clearly:
Fix
Added
.kill()calls for all four timelines in the cleanup return block.Related
Closes #[your issue number]
Relates to #292
Fixes # (issue)
Type of change
Please delete options that are not relevant.
)
How Has This Been Tested?
Live version and my local version
Checklist:
Maintainer Checklist
Summary by CodeRabbit