Animated: Optimize onUserDrivenAnimationEnded Deopt (#48511)

Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/48511

{D60499583} added a new`onUserDrivenAnimationEnded` listener that requires `AnimatedValue` instances to have up-to-date values reported by `onAnimatedValueUpdate` (if native driver is in use).

Previously, the only way to ensure `onAnimatedValueUpdate` events were always fired to update JavaScript values in `AnimatedValue` instance was to attach a listener — even an empty one. This is exactly what D60499583 did: it traverses `props` for `AnimatedNode` instances and attaches listeners to them.

However, this is really inefficient and makes the code extra convoluted. Instead, this diff changes `AnimatedValue` so that it always subscribes to changes in `__attach`, and then it cleans up the extraneous props traversal and "empty listener" logic.

Changelog:
[Internal]

Reviewed By: javache

Differential Revision: D67872307

fbshipit-source-id: e7d7e486bbfd9ef03e2dd9f201089e2f68b2dbb2

packages/react-native/Libraries/Animated/__tests__/AnimatedValue-test.js

@@ -68,22 +68,15 @@ describe('AnimatedValue', () => {
     expect(callback).toBeCalledTimes(1);
   });
 
-  it('creates a native node when adding a listener', () => {
+  it('creates a native node on attach', () => {
     const node = createNativeAnimatedValue();
     node.__attach();
-    expect(NativeAnimatedHelper.API.createAnimatedNode).not.toBeCalled();
-
-    const id = node.addListener(jest.fn());
-    node.removeListener(id);
     expect(NativeAnimatedHelper.API.createAnimatedNode).toBeCalledTimes(1);
   });
 
   it('drops a created native node on detach', () => {
     const node = createNativeAnimatedValue();
     node.__attach();
-    expect(NativeAnimatedHelper.API.createAnimatedNode).toBeCalledTimes(0);
-
-    node.addListener(jest.fn());
     expect(NativeAnimatedHelper.API.createAnimatedNode).toBeCalledTimes(1);
     expect(NativeAnimatedHelper.API.dropAnimatedNode).toBeCalledTimes(0);
 

packages/react-native/Libraries/Animated/nodes/AnimatedValue.js

@@ -9,7 +9,6 @@
  */
 
 import type {EventSubscription} from '../../vendor/emitter/EventEmitter';
-import type {PlatformConfig} from '../AnimatedPlatformConfig';
 import type Animation, {EndCallback} from '../animations/Animation';
 import type {InterpolationConfigType} from './AnimatedInterpolation';
 import type AnimatedNode from './AnimatedNode';
@@ -85,7 +84,6 @@ function _executeAsAnimatedBatch(id: string, operation: () => void) {
  * See https://reactnative.dev/docs/animatedvalue
  */
 export default class AnimatedValue extends AnimatedWithChildren {
-  #listenerCount: number = 0;
   #updateSubscription: ?EventSubscription = null;
 
   _value: number;
@@ -107,8 +105,20 @@ export default class AnimatedValue extends AnimatedWithChildren {
     }
   }
 
-  __detach() {
+  __attach(): void {
     if (this.__isNative) {
+      // NOTE: In theory, we should only need to call this when any listeners
+      // are added. However, there is a global `onUserDrivenAnimationEnded`
+      // listener that relies on `onAnimatedValueUpdate` having fired to update
+      // the values in JavaScript. If that listener is removed, this could be
+      // re-optimized.
+      this.#ensureUpdateSubscriptionExists();
+    }
+  }
+
+  __detach(): void {
+    if (this.__isNative) {
+      this.#updateSubscription?.remove();
       NativeAnimatedAPI.getValue(this.__getNativeTag(), value => {
         this._value = value - this._offset;
       });
@@ -121,38 +131,6 @@ export default class AnimatedValue extends AnimatedWithChildren {
     return this._value + this._offset;
   }
 
-  __makeNative(platformConfig: ?PlatformConfig): void {
-    super.__makeNative(platformConfig);
-    if (this.#listenerCount > 0) {
-      this.#ensureUpdateSubscriptionExists();
-    }
-  }
-
-  addListener(callback: (value: any) => mixed): string {
-    const id = super.addListener(callback);
-    this.#listenerCount++;
-    if (this.__isNative) {
-      this.#ensureUpdateSubscriptionExists();
-    }
-    return id;
-  }
-
-  removeListener(id: string): void {
-    super.removeListener(id);
-    this.#listenerCount--;
-    if (this.__isNative && this.#listenerCount === 0) {
-      this.#updateSubscription?.remove();
-    }
-  }
-
-  removeAllListeners(): void {
-    super.removeAllListeners();
-    this.#listenerCount = 0;
-    if (this.__isNative) {
-      this.#updateSubscription?.remove();
-    }
-  }
-
   #ensureUpdateSubscriptionExists(): void {
     if (this.#updateSubscription != null) {
       return;

packages/react-native/Libraries/Animated/useAnimatedProps.js

@@ -17,9 +17,7 @@ import * as ReactNativeFeatureFlags from '../../src/private/featureflags/ReactNa
 import {isPublicInstance as isFabricPublicInstance} from '../ReactNative/ReactFabricPublicInstance/ReactFabricPublicInstanceUtils';
 import useRefEffect from '../Utilities/useRefEffect';
 import {AnimatedEvent} from './AnimatedEvent';
-import AnimatedNode from './nodes/AnimatedNode';
 import AnimatedProps from './nodes/AnimatedProps';
-import AnimatedValue from './nodes/AnimatedValue';
 import {
   useCallback,
   useEffect,
@@ -39,11 +37,6 @@ type CallbackRef<T> = T => mixed;
 
 type UpdateCallback = () => void;
 
-type AnimatedValueListeners = Array<{
-  propValue: AnimatedValue,
-  listenerId: string,
-}>;
-
 const useMemoOrAnimatedPropsMemo =
   ReactNativeFeatureFlags.enableAnimatedPropsMemo()
     ? useAnimatedPropsMemo
@@ -169,7 +162,6 @@ export default function useAnimatedProps<TProps: {...}, TInstance>(
 
       const target = getEventTarget(instance);
       const events = [];
-      const animatedValueListeners: AnimatedValueListeners = [];
 
       for (const propName in props) {
         // $FlowFixMe[invalid-computed-prop]
@@ -177,8 +169,6 @@ export default function useAnimatedProps<TProps: {...}, TInstance>(
         if (propValue instanceof AnimatedEvent && propValue.__isNative) {
           propValue.__attach(target, propName);
           events.push([propName, propValue]);
-          // $FlowFixMe[incompatible-call] - the `addListenersToPropsValue` drills down the propValue.
-          addListenersToPropsValue(propValue, animatedValueListeners);
         }
       }
 
@@ -188,10 +178,6 @@ export default function useAnimatedProps<TProps: {...}, TInstance>(
         for (const [propName, propValue] of events) {
           propValue.__detach(target, propName);
         }
-
-        for (const {propValue, listenerId} of animatedValueListeners) {
-          propValue.removeListener(listenerId);
-        }
       };
     },
     [node, useNativePropsInFabric, props],
@@ -215,35 +201,6 @@ function reduceAnimatedProps<TProps>(
   };
 }
 
-function addListenersToPropsValue(
-  propValue: AnimatedValue,
-  accumulator: AnimatedValueListeners,
-) {
-  // propValue can be a scalar value, an array or an object.
-  if (propValue instanceof AnimatedValue) {
-    const listenerId = propValue.addListener(() => {});
-    accumulator.push({propValue, listenerId});
-  } else if (Array.isArray(propValue)) {
-    // An array can be an array of scalar values, arrays of arrays, or arrays of objects
-    for (const prop of propValue) {
-      addListenersToPropsValue(prop, accumulator);
-    }
-  } else if (propValue instanceof Object) {
-    addAnimatedValuesListenersToProps(propValue, accumulator);
-  }
-}
-
-function addAnimatedValuesListenersToProps(
-  props: AnimatedNode,
-  accumulator: AnimatedValueListeners,
-) {
-  for (const propName in props) {
-    // $FlowFixMe[prop-missing] - This is an object contained in a prop, but we don't know the exact type.
-    const propValue = props[propName];
-    addListenersToPropsValue(propValue, accumulator);
-  }
-}
-
 /**
  * Manages the lifecycle of the supplied `AnimatedProps` by invoking `__attach`
  * and `__detach`. However, this is more complicated because `AnimatedProps`

packages/react-native/Libraries/__tests__/__snapshots__/public-api-test.js.snap

@@ -1166,12 +1166,9 @@ declare export default class AnimatedValue extends AnimatedWithChildren {
   _animation: ?Animation;
   _tracking: ?AnimatedTracking;
   constructor(value: number, config?: ?AnimatedValueConfig): void;
+  __attach(): void;
   __detach(): void;
   __getValue(): number;
-  __makeNative(platformConfig: ?PlatformConfig): void;
-  addListener(callback: (value: any) => mixed): string;
-  removeListener(id: string): void;
-  removeAllListeners(): void;
   setValue(value: number): void;
   setOffset(offset: number): void;
   flattenOffset(): void;

packages/react-native/src/private/animated/__tests__/AnimatedNative-test.js

@@ -208,15 +208,21 @@ describe('Native Animated', () => {
 
       const value1 = new Animated.Value(0);
       value1.__makeNative();
+      const nativeTag = value1.__getNativeTag();
+
+      value1.__attach();
       const listener = jest.fn();
       const id = value1.addListener(listener);
       expect(
         NativeAnimatedModule.startListeningToAnimatedNodeValue,
-      ).toHaveBeenCalledWith(value1.__getNativeTag());
+      ).toHaveBeenCalledTimes(1);
+      expect(
+        NativeAnimatedModule.startListeningToAnimatedNodeValue,
+      ).toHaveBeenCalledWith(nativeTag);
 
       NativeAnimatedHelper.nativeEventEmitter.emit('onAnimatedValueUpdate', {
         value: 42,
-        tag: value1.__getNativeTag(),
+        tag: nativeTag,
       });
       expect(listener).toHaveBeenCalledTimes(1);
       expect(listener).toBeCalledWith({value: 42});
@@ -224,20 +230,24 @@ describe('Native Animated', () => {
 
       NativeAnimatedHelper.nativeEventEmitter.emit('onAnimatedValueUpdate', {
         value: 7,
-        tag: value1.__getNativeTag(),
+        tag: nativeTag,
       });
       expect(listener).toHaveBeenCalledTimes(2);
       expect(listener).toBeCalledWith({value: 7});
       expect(value1.__getValue()).toBe(7);
 
       value1.removeListener(id);
+      value1.__detach();
       expect(
         NativeAnimatedModule.stopListeningToAnimatedNodeValue,
-      ).toHaveBeenCalledWith(value1.__getNativeTag());
+      ).toHaveBeenCalledTimes(1);
+      expect(
+        NativeAnimatedModule.stopListeningToAnimatedNodeValue,
+      ).toHaveBeenCalledWith(nativeTag);
 
       NativeAnimatedHelper.nativeEventEmitter.emit('onAnimatedValueUpdate', {
         value: 1492,
-        tag: value1.__getNativeTag(),
+        tag: nativeTag,
       });
       expect(listener).toHaveBeenCalledTimes(2);
       expect(value1.__getValue()).toBe(7);
@@ -248,27 +258,37 @@ describe('Native Animated', () => {
 
       const value1 = new Animated.Value(0);
       value1.__makeNative();
+      const nativeTag = value1.__getNativeTag();
+
+      value1.__attach();
       const listener = jest.fn();
       [1, 2, 3, 4].forEach(() => value1.addListener(listener));
       expect(
         NativeAnimatedModule.startListeningToAnimatedNodeValue,
-      ).toHaveBeenCalledWith(value1.__getNativeTag());
+      ).toHaveBeenCalledTimes(1);
+      expect(
+        NativeAnimatedModule.startListeningToAnimatedNodeValue,
+      ).toHaveBeenCalledWith(nativeTag);
 
       NativeAnimatedHelper.nativeEventEmitter.emit('onAnimatedValueUpdate', {
         value: 42,
-        tag: value1.__getNativeTag(),
+        tag: nativeTag,
       });
       expect(listener).toHaveBeenCalledTimes(4);
       expect(listener).toBeCalledWith({value: 42});
 
       value1.removeAllListeners();
+      value1.__detach();
       expect(
         NativeAnimatedModule.stopListeningToAnimatedNodeValue,
-      ).toHaveBeenCalledWith(value1.__getNativeTag());
+      ).toHaveBeenCalledTimes(1);
+      expect(
+        NativeAnimatedModule.stopListeningToAnimatedNodeValue,
+      ).toHaveBeenCalledWith(nativeTag);
 
       NativeAnimatedHelper.nativeEventEmitter.emit('onAnimatedValueUpdate', {
         value: 7,
-        tag: value1.__getNativeTag(),
+        tag: nativeTag,
       });
       expect(listener).toHaveBeenCalledTimes(4);
     });