Skip to content

Commit

Permalink
Animated: Fix onUserDrivenAnimationEnded w/ Insertion Effects
Browse files Browse the repository at this point in the history
Summary:
While diagnosing a recent issue in which `AnimatedValue` instances were not being correctly updated as expected, the insertion effects feature flag was identified as a root cause.

Upon further investigation, it appears that this is because the `onUserDrivenAnimationEnded` listener was not implemented the same way in the two feature flag states:

- When `useInsertionEffectsForAnimations` is disabled, `useAnimatedProps` listens to `onUserDrivenAnimationEnded` in a passive effect, after all nodes have been attached.
- When `useInsertionEffectsForAnimations` is enabled, `useAnimatedProps` listens to `onUserDrivenAnimationEnded` in an insertion effect when attaching nodes.

The bugs occurs because `useAnimatedProps` checks whether native driver is employed to decide whether to listen to `onUserDrivenAnimationEnded`. However, we do not know whether native driver will be employed during the insertion effect. (Actually, we do not necessarily know that in a passive effect, either... but that is a separate matter.)

This fixes the bug when that occurs when `useInsertionEffectsForAnimations` is enabled, by moving the listening logic of `onUserDrivenAnimationEnded` into a passive effect. This is the same way that it is implemented when `useInsertionEffectsForAnimations` is disabled.

Changelog:
[Internal]

Differential Revision: D68171721
  • Loading branch information
yungsters authored and facebook-github-bot committed Jan 14, 2025
1 parent b3648be commit 00af87c
Showing 1 changed file with 14 additions and 13 deletions.
27 changes: 14 additions & 13 deletions packages/react-native/Libraries/Animated/useAnimatedProps.js
Original file line number Diff line number Diff line change
Expand Up @@ -276,6 +276,20 @@ function useAnimatedPropsLifecycle_insertionEffects(node: AnimatedProps): void {
// if the queue is empty. When multiple animated components are mounted at
// the same time. Only first component flushes the queue and the others will noop.
NativeAnimatedHelper.API.flushQueue();
let drivenAnimationEndedListener: ?EventSubscription = null;
if (node.__isNative) {
drivenAnimationEndedListener =
NativeAnimatedHelper.nativeEventEmitter.addListener(
'onUserDrivenAnimationEnded',
data => {
node.update();
},
);
}

return () => {
drivenAnimationEndedListener?.remove();
};
});

useInsertionEffect(() => {
Expand All @@ -287,17 +301,6 @@ function useAnimatedPropsLifecycle_insertionEffects(node: AnimatedProps): void {

useInsertionEffect(() => {
node.__attach();
let drivenAnimationEndedListener: ?EventSubscription = null;

if (node.__isNative) {
drivenAnimationEndedListener =
NativeAnimatedHelper.nativeEventEmitter.addListener(
'onUserDrivenAnimationEnded',
data => {
node.update();
},
);
}
if (prevNodeRef.current != null) {
const prevNode = prevNodeRef.current;
// TODO: Stop restoring default values (unless `reset` is called).
Expand All @@ -312,8 +315,6 @@ function useAnimatedPropsLifecycle_insertionEffects(node: AnimatedProps): void {
} else {
prevNodeRef.current = node;
}

drivenAnimationEndedListener?.remove();
};
}, [node]);
}
Expand Down

0 comments on commit 00af87c

Please sign in to comment.