From 3abb4cab7d1fba738987dca5f317e395dd985d27 Mon Sep 17 00:00:00 2001
From: Tim Yung <yungsters@meta.com>
Date: Wed, 15 Jan 2025 17:00:44 -0800
Subject: [PATCH] Animated: Defer `onAnimatedValueUpdate` on Attach + Native
 (#48715)

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

{D68154908} fixed a problem with the `onAnimatedValueUpdate` listener not being correctly attached if `__attach` were called before `__makeNative` (which sets `__isNative` to true).

We're potentially seeing production symptoms of stuttering interactions and user responsiveness, after queuing up many operations. Our hypothesis is that in scenarios where `ensureUpdateSubscriptionExists` is being called during `__makeNative` (instead of during `__attach`), a backup of operations occurs leading to these symptoms.

This diff attempts to validate and mitigate this hypothesis by deferring `ensureUpdateSubscriptionExists` to when an `AnimatedValue` instance has had both `__attach` and `__makeNative` invoked.

Changelog:
[Internal]

Differential Revision: D68236594

fbshipit-source-id: 2089100a773ebfc161fb5b567123eb58a893939f
---
 .../Libraries/Animated/nodes/AnimatedValue.js | 24 ++++++++++++-------
 1 file changed, 16 insertions(+), 8 deletions(-)

diff --git a/packages/react-native/Libraries/Animated/nodes/AnimatedValue.js b/packages/react-native/Libraries/Animated/nodes/AnimatedValue.js
index 2e500bf64b480d..8ab98a06246ac2 100644
--- a/packages/react-native/Libraries/Animated/nodes/AnimatedValue.js
+++ b/packages/react-native/Libraries/Animated/nodes/AnimatedValue.js
@@ -85,6 +85,7 @@ function _executeAsAnimatedBatch(id: string, operation: () => void) {
  * See https://reactnative.dev/docs/animatedvalue
  */
 export default class AnimatedValue extends AnimatedWithChildren {
+  #attached: boolean = false;
   #updateSubscription: ?EventSubscription = null;
 
   _value: number;
@@ -107,14 +108,8 @@ export default class AnimatedValue extends AnimatedWithChildren {
   }
 
   __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();
-    }
+    this.#attached = true;
+    this.#ensureUpdateSubscriptionExists();
   }
 
   __detach(): void {
@@ -126,6 +121,7 @@ export default class AnimatedValue extends AnimatedWithChildren {
     }
     this.stopAnimation();
     super.__detach();
+    this.#attached = false;
   }
 
   __getValue(): number {
@@ -137,10 +133,22 @@ export default class AnimatedValue extends AnimatedWithChildren {
     this.#ensureUpdateSubscriptionExists();
   }
 
+  /**
+   * 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.
+   */
   #ensureUpdateSubscriptionExists(): void {
     if (this.#updateSubscription != null) {
       return;
     }
+    // The order in which `__attach` and `__makeNative` are called is not
+    // deterministic, and we only want to do this when both have occurred.
+    if (!this.#attached || !this.__isNative) {
+      return;
+    }
     const nativeTag = this.__getNativeTag();
     NativeAnimatedAPI.startListeningToAnimatedNodeValue(nativeTag);
     const subscription: EventSubscription =