Skip to content

Fix stop() firing extra update on object animations#3586

Open
mattgperry wants to merge 1 commit intomainfrom
worktree-fix-issue-3336
Open

Fix stop() firing extra update on object animations#3586
mattgperry wants to merge 1 commit intomainfrom
worktree-fix-issue-3336

Conversation

@mattgperry
Copy link
Collaborator

Summary

  • Bug: After calling stop() on an object animation (e.g. animating a Proxy), one extra update was fired on the next frame, modifying the target object after stop() had returned
  • Cause: JSAnimation.stop() called this.tick(time.now()) which triggered the onUpdatemotionValue.set()scheduleRender() chain, scheduling a VisualElement render for the next frame. The teardown() only cancelled the animation's update callback, not the pending render
  • Fix: Remove the tick() call from stop(). The motionValue already holds its value from the last rendered frame, which is sufficient

Fixes #3336

Test plan

  • Added unit test that creates a Proxy with a set trap counter, animates it with repeat: Infinity, calls stop(), then asserts no additional updates fire
  • All existing JSAnimation tests pass (68/68)
  • All existing animate tests pass (43/43)
  • Full yarn build passes
  • Full yarn test passes (only pre-existing flaky use-instant-transition timeout unrelated to this change)

🤖 Generated with Claude Code

Remove the tick() call in JSAnimation.stop() that was scheduling a
VisualElement render after the animation was supposedly stopped. The
tick triggered onUpdate → motionValue.set → scheduleRender, causing
one extra frame to update the target object after stop() returned.

Fixes #3336

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@greptile-apps
Copy link

greptile-apps bot commented Mar 3, 2026

Greptile Summary

This PR fixes a bug where calling stop() on an object animation (e.g., animating a Proxy) would fire one extra update on the next frame after stop() returned. The root cause was that JSAnimation.stop() was calling this.tick(time.now()), which triggered the update callback chain (onUpdatemotionValue.set()scheduleRender()). While teardown() cancelled the animation's update callback, it didn't cancel the pending render scheduled for the next frame.

The fix removes the tick() call from stop(). The motion value already holds its value from the last rendered frame, making the extra tick unnecessary. The change is clean and well-tested:

  • A comprehensive unit test was added using a Proxy with a set trap counter to verify no updates fire after stop()
  • All 68 existing JSAnimation tests pass
  • All 43 existing animate tests pass
  • Full build and test suite passes

The fix aligns with the expected behavior: stop() should immediately stop the animation without triggering any additional updates, similar to how pause() works (which calls updateTime() but not tick()).

Confidence Score: 5/5

  • This PR is safe to merge with minimal risk
  • The fix is surgical and well-understood: it removes an unnecessary tick() call that was causing unintended side effects. The change is validated by comprehensive testing (new unit test + all 111 existing tests passing), and the PR description clearly documents the bug, cause, and fix. The author has verified the full build and test suite passes.
  • No files require special attention

Important Files Changed

Filename Overview
packages/motion-dom/src/animation/JSAnimation.ts Removed unnecessary tick() call from stop() method that was causing an extra update after stopping object animations
packages/framer-motion/src/animation/animate/tests/animate.test.tsx Added comprehensive test using Proxy to verify no updates fire after stop() is called on object animations

Last reviewed commit: 9d27a00

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.

[BUG] After AnimationPlaybackControlsWithThen.stop(), an update is executed

1 participant