fix(bento): dispose homepage animation listeners and timers on unmount - #3168
fix(bento): dispose homepage animation listeners and timers on unmount#3168WhoamiI00 wants to merge 2 commits into
Conversation
The bento animation components register motion `hover()` and `inView()`
handlers inside `$effect`/`onMount` but discard the disposer each one
returns, and never cancel the in-flight animation. Nothing is torn down
when the component is destroyed by a client-side navigation.
Two consequences:
- Every card leaks its IntersectionObserver. Leaving the homepage left
10 of 12 observers alive, holding a reference to the detached subtree.
- `auth.svelte` starts a 1000ms `write()` interval on hover. Clicking the
card mid-animation navigates away, but the interval keeps running and
its `.then()` still fires `animate(button, ...)` after `bind:this` has
reset `button` to null, throwing "You're trying to perform an animation
on null". `sites.svelte` similarly leaves a 44s `animate()` running.
Capture the disposers, return a teardown from the effect, and stop any
in-flight animation. `auth.svelte` also guards the deferred button pulse,
since a write that settles exactly as the component unmounts resolves
after teardown has already run.
Measured on the homepage, hovering the Auth card then clicking it:
before after
animation ticks after unmount 11 0
IntersectionObservers still live 10 3
The 7 disposed observers are exactly the 7 bento cards; the remaining 3
belong to other components on the page.
Greptile SummaryThis PR adds lifecycle cleanup to the homepage bento animations so client-side navigation no longer leaves their Motion handlers and long-running animation work active.
Confidence Score: 4/5The PR is safe to merge, with a non-blocking cleanup gap for short-lived animations in four bento cards. Listener and observer disposal is correctly added, and the stateful or long-running Auth, Functions, and Sites animations are explicitly cancelled; several other cards still allow already-started element animations to finish after destruction. Files Needing Attention: src/routes/(marketing)/(components)/bento/(animations)/databases.svelte, messaging.svelte, realtime.svelte, and storage.svelte Important Files Changed
Prompt To Fix All With AI### Issue 1
src/routes/(marketing)/(components)/bento/(animations)/databases.svelte:140-143
**Active animations survive teardown**
`stopHover()` and `stopInView()` only detach Motion's event and observer handlers; they do not stop animations already started by their callbacks. Navigating away during an animation therefore leaves work running against the detached card until completion; retain and stop the animation controls here and in the equivalent messaging, realtime, and storage cleanup paths.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.Reviews (1): Last reviewed commit: "fix(bento): dispose homepage animation l..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
|
|
||
| return () => { | ||
| stopHover(); | ||
| stopInView(); |
There was a problem hiding this comment.
stopHover() and stopInView() only detach Motion's event and observer handlers; they do not stop animations already started by their callbacks. Navigating away during an animation therefore leaves work running against the detached card until completion; retain and stop the animation controls here and in the equivalent messaging, realtime, and storage cleanup paths.
Knowledge Base Used: Marketing pages
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/routes/(marketing)/(components)/bento/(animations)/databases.svelte
Line: 140-143
Comment:
**Active animations survive teardown**
`stopHover()` and `stopInView()` only detach Motion's event and observer handlers; they do not stop animations already started by their callbacks. Navigating away during an animation therefore leaves work running against the detached card until completion; retain and stop the animation controls here and in the equivalent messaging, realtime, and storage cleanup paths.
**Knowledge Base Used:** [Marketing pages](https://app.greptile.com/appwrite/-/custom-context/knowledge-base/appwrite/website/-/docs/marketing-pages.md)
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
There was a problem hiding this comment.
Valid, and fixed in f2f5c3a — thanks.
You're right that the returned cleanups only detach the pointer listener and the observer. What made this worth fixing is that the PR was already inconsistent about it: auth, functions and sites do hold their controls and stop them on teardown, while databases, messaging, realtime and storage discarded the return value of animate() entirely. So the four you flagged were the gap, and the fix just brings them in line with the three that were already right.
Each of those four animates a single element and a new animation supersedes the previous one on it, so holding the latest controls is sufficient — no set or bookkeeping needed:
let animation: ReturnType<typeof animate> | undefined;
...
animation = animate(table, { ... }, { ... });
...
return () => {
stopHover();
stopInView();
animation?.stop();
};I used stop() rather than cancel() deliberately: stop() halts at the current state, while cancel() applies the initial state, which would visibly snap the card back if the effect re-ran rather than unmounted.
Typed via ReturnType<typeof animate> because AnimationPlaybackControlsWithThen is imported from motion-dom inside framer-motion's dom.d.ts rather than re-exported from motion, so naming it directly would reach past the public entry point.
svelte-check is clean — 0 errors, 0 warnings.
`hover()` and `inView()` return cleanups that detach the pointer listener and the observer, but they do not touch an animation those callbacks have already started. Leaving the page mid-transition left that animation running against a card no longer in the document. `auth`, `functions` and `sites` already held their controls and stopped them; `databases`, `messaging`, `realtime` and `storage` discarded the return value of `animate()` and only detached the handlers. They now keep the latest controls and stop them alongside, which is all that is needed since a new animation on the same element supersedes the previous one. `stop()` halts at the current state rather than `cancel()`, which would snap the card back to the initial state if the effect re-ran.
Appwrite WebsiteProject ID: Website (appwrite/website)Project ID: Tip Custom domains work with both CNAME for subdomains and NS records for apex domains |
1 participant
What does this PR do?
The bento animation components on the homepage register motion's
hover()andinView()handlers inside$effect/onMount, but discard the disposer function each one returns and never cancel the in-flight animation. Nothing is torn down when the component is destroyed by a client-side navigation.This has two visible consequences:
1. Every bento card leaks its
IntersectionObserver. Navigating away from the homepage left 10 of 12 observers alive, each still holding a reference to the now-detached subtree.2.
auth.sveltethrows an uncaught error. Hovering the Auth card starts a 1000mswrite()interval. Because the card is a link, clicking it mid-animation is a completely ordinary interaction — the page navigates, the component is destroyed, but the interval keeps running and its.then()still callsanimate(button, ...)afterbind:thishas resetbuttontonull:This reproduces on production today — on
appwrite.iothe minified build surfaces it asInvalid value used as weak map keyfrom the same call.sites.sveltehas the same shape and additionally leaves a 44-secondanimate()running after unmount.The fix
Capture the disposers that
hover()/inView()already return, return a teardown from the effect, and stop any in-flight animation.auth.sveltealso guards the deferred button pulse, since awrite()that settles at the exact moment of unmount resolves after teardown has run and so cannot be cancelled.No behaviour changes while the component is mounted — this only adds cleanup.
Test Plan
Repro (before this PR): load the homepage, hover the Auth bento card, and click it within ~1s. The error above appears in the console on
/products/auth.Measured with an instrumented Playwright run that hovers the Auth card, clicks through to
/products/auth, then counts animation ticks that still fire and observers that are still connected:IntersectionObservers still liveThe 7 newly-disposed observers correspond exactly to the 7 bento cards; the remaining 3 belong to other components on the page and are out of scope here.
Also verified there is no regression to the hover animation itself — rapid hover in/out over 12 cycles still shows 1 concurrent animation, 0 multi-character jumps, and clean write/unwrite alternation, identical to
main.Checks:
svelte-checkreports 0 errors and 0 warnings, andprettier --checkpasses on all touched files.Related PRs and Issues
None. Found while investigating #2758 (which is already fixed by d1b92c9 — I've left a note on that issue).
Have you read the Contributing Guidelines on issues?
Yes.