fix: tear down loop animations the current props no longer request - #55
Open
janicduplessis wants to merge 1 commit into
Open
fix: tear down loop animations the current props no longer request#55janicduplessis wants to merge 1 commit into
janicduplessis wants to merge 1 commit into
Conversation
A looping animation was only ever removed when a property was still in animatedProperties AND its value changed. Props that drop a property from `animate` entirely, or that keep animating it but stop asking for `loop`, hit neither gate, so the infinite animation kept driving the view — and on iOS the saved snapshot replayed it on every didMoveToWindow. Both platforms now filter running loops against the current props on each update, and iOS also filters the snapshot before replaying it on re-attach. Adds example/app/issues/54 covering both cases.
janicduplessis
marked this pull request as ready for review
August 17, 2026 06:17
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.
Summary
Fixes #54.
A looping animation only ever got torn down when a property was still in
animatedPropertiesAND its value changed — every teardown call sits behind both gates. Two shapes of props update hit neither:animateentirely, so the mask bit clears and the whole per-property block is skipped;loop, so nothing changed and the block no-ops.Either way the infinite animation keeps driving the view. On iOS it's worse than "keeps running": the loop stays in
_loopAnimations, sodidMoveToWindowreplays it on every re-attach, and it survives a tab switch.That's what the reporter saw as a shimmer animating a fully-loaded content card. Their write-up blames Fabric view recycling, but that part doesn't hold up —
prepareForRecyclealready clears_loopAnimations, andcleanup()does the same on Android. No recycling is needed to hit this: a list row that keeps the sameEaseViewand just swaps skeleton props for content props is the first case above.Both platforms now filter running loops against the current props on every update, and iOS filters the saved snapshot again before replaying it on re-attach. It only ever removes a loop the current props don't ask for, so it's a no-op for a view that still declares one.
The subtle part is what value the property is left at — cancelling mid-sweep would strand a card permanently offset. The two platforms need different handling here, which is the main thing to know when reading the diff:
applyFirstMountProps:), so removing theCAAnimationalready lands on the right value; only a property that leftanimatedPropertiesneeds a write. Transforms are recomposed viatargetTransformFromProps:rather than poked through atransform.translation.xkey path, since a matrix carryingm34perspective can't be decomposed reliably.cancel()strands it wherever the sweep happened to be and a write is needed either way. It's skipped only when a path below will animate the property anyway, because those read the live view value as their "from".Colours are left alone on both: an unset colour has no identity value, and the style owns it once the mask bit clears.
Test Plan
New reproducer at
example/app/issues/54covering both cases on one screen — a shimmer band that keeps the sameEaseViewinstance across the load, like a list row swapping skeleton for content.Verified on an Android emulator (density 420) by measuring the band's centre across successive frames rather than eyeballing the video. At 420 dpi the 64 dp sweep is 168 px, so the correct resting positions are predictable up front: card A back at screen centre (540 px,
translateXidentity), card B parked at its declared target (540 + 168 = 708 px).Before — both bands keep sweeping over the loaded content, still moving 13 s after the press:
ease54_before.mp4
After — both stop on load and stay stopped across a tab switch out and back, which exercises the
reapplyLoopAnimationspath:ease54_after.mp4
iOS is compile-verified only.
EaseView.mmbuilds clean for arm64 and x86_64 (xcodebuild -sdk iphonesimulator), but I could not run it — CoreSimulator is wedged on this machine (simctlhangs in+[SimServiceContext sharedServiceContextForDeveloperDir:]). The iOS half is reasoned from the source, not exercised on device, so thedidMoveToWindowfiltering in particular is worth a look on a working simulator before merge.