Gate logic to remove js sync at end of native animation loop (#52068)

Summary:
Pull Request resolved: #52068

## Changelog:

[Internal] [Changed] - Gate logic to remove js sync at end of native animation loop

because #51264 is causing regression in some cases, and was recently reverted again in #51933 . Using an extra feature flag to gate this potential optimization

Reviewed By: lenaic

Differential Revision: D76759441

fbshipit-source-id: 6950ab746b02af6e0b710ded0ddb6993673e5212

Author: zeyap

packages/react-native/Libraries/Animated/__tests__/Animated-itest.js

@@ -4,6 +4,7 @@
  * This source code is licensed under the MIT license found in the
  * LICENSE file in the root directory of this source tree.
  *
+ * @fantom_flags cxxNativeAnimatedRemoveJsSync:*
  * @flow strict-local
  * @format
  */
@@ -16,6 +17,7 @@ import ensureInstance from '../../../src/private/__tests__/utilities/ensureInsta
 import * as Fantom from '@react-native/fantom';
 import {createRef} from 'react';
 import {Animated, View, useAnimatedValue} from 'react-native';
+import * as ReactNativeFeatureFlags from 'react-native/src/private/featureflags/ReactNativeFeatureFlags';
 import ReactNativeElement from 'react-native/src/private/webapis/dom/nodes/ReactNativeElement';
 
 test('moving box by 100 points', () => {
@@ -73,11 +75,15 @@ test('moving box by 100 points', () => {
   Fantom.unstable_produceFramesForDuration(500);
 
   // Animation is completed now. C++ Animated will commit the final position to the shadow tree.
-  expect(viewElement.getBoundingClientRect().x).toBe(100);
+  if (ReactNativeFeatureFlags.cxxNativeAnimatedRemoveJsSync()) {
+    expect(viewElement.getBoundingClientRect().x).toBe(100);
+    // TODO(T223344928): this shouldn't be neccessary
+    Fantom.runWorkLoop();
+  } else {
+    expect(viewElement.getBoundingClientRect().x).toBe(0);
+    Fantom.runWorkLoop(); // Animated still schedules a React state update for synchronisation to shadow tree
+  }
 
-  // TODO: this shouldn't be needed but C++ Animated still schedules a React state update
-  // for synchronisation, even though it doesn't need to.
-  Fantom.runWorkLoop();
   expect(viewElement.getBoundingClientRect().x).toBe(100);
 });
 
@@ -188,7 +194,7 @@ test('animated opacity', () => {
     0,
   );
 
-  // TODO: this shouldn't be neccessary but C++ Animated still schedules a React state update.
+  // TODO(T223344928): this shouldn't be neccessary with cxxNativeAnimatedRemoveJsSync:true
   Fantom.runWorkLoop();
 
   expect(root.getRenderedOutput({props: ['opacity']}).toJSX()).toEqual(
@@ -262,12 +268,18 @@ test('moving box by 50 points with offset 10', () => {
       .translateX,
   ).toBeCloseTo(60, 0.001);
 
-  expect(root.getRenderedOutput({props: ['transform']}).toJSX()).toEqual(
-    <rn-view transform='[{"translateX": 60.000000}]' />,
-  );
-
-  // TODO: this shouldn't be neccessary but C++ Animated still schedules a React state update.
-  Fantom.runWorkLoop();
+  if (ReactNativeFeatureFlags.cxxNativeAnimatedRemoveJsSync()) {
+    expect(root.getRenderedOutput({props: ['transform']}).toJSX()).toEqual(
+      <rn-view transform='[{"translateX": 60.000000}]' />,
+    );
+    // TODO(T223344928): this shouldn't be neccessary
+    Fantom.runWorkLoop();
+  } else {
+    expect(root.getRenderedOutput({props: ['transform']}).toJSX()).toEqual(
+      <rn-view transform="[]" />,
+    );
+    Fantom.runWorkLoop(); // Animated still schedules a React state update for synchronisation to shadow tree
+  }
 
   expect(root.getRenderedOutput({props: ['transform']}).toJSX()).toEqual(
     <rn-view transform='[{"translateX": 60.000000}]' />, // // must include offset.
@@ -360,7 +372,7 @@ describe('Value.flattenOffset', () => {
 
     expect(transform.translateY).toBeCloseTo(40, 0.001);
 
-    // TODO: this shouldn't be neccessary.
+    // TODO(T223344928): this shouldn't be neccessary with cxxNativeAnimatedRemoveJsSync:true
     Fantom.runWorkLoop();
   });
 });
@@ -459,7 +471,7 @@ describe('Value.extractOffset', () => {
     // Previously we set offset to 35. The final value is 35.
     expect(transform.translateY).toBeCloseTo(35, 0.001);
 
-    // TODO: this shouldn't be neccessary.
+    // TODO(T223344928): this shouldn't be neccessary with cxxNativeAnimatedRemoveJsSync:true
     Fantom.runWorkLoop();
   });
 });

packages/react-native/Libraries/Animated/animations/Animation.js

@@ -13,6 +13,7 @@ import type AnimatedNode from '../nodes/AnimatedNode';
 import type AnimatedValue from '../nodes/AnimatedValue';
 
 import NativeAnimatedHelper from '../../../src/private/animated/NativeAnimatedHelper';
+import * as ReactNativeFeatureFlags from '../../../src/private/featureflags/ReactNativeFeatureFlags';
 import AnimatedProps from '../nodes/AnimatedProps';
 
 export type EndResult = {
@@ -149,8 +150,15 @@ export default class Animation {
           if (value != null) {
             animatedValue.__onAnimatedValueUpdateReceived(value, offset);
 
-            if (this.__isLooping === true) {
-              return;
+            if (
+              !(
+                ReactNativeFeatureFlags.cxxNativeAnimatedEnabled() &&
+                ReactNativeFeatureFlags.cxxNativeAnimatedRemoveJsSync()
+              )
+            ) {
+              if (this.__isLooping === true) {
+                return;
+              }
             }
 
             // Once the JS side node is synced with the updated values, trigger an

packages/react-native/ReactCxxPlatform/react/renderer/animated/NativeAnimatedNodesManager.cpp

@@ -10,6 +10,7 @@
 #include <folly/json.h>
 #include <glog/logging.h>
 #include <react/debug/react_native_assert.h>
+#include <react/featureflags/ReactNativeFeatureFlags.h>
 #include <react/profiling/perfetto.h>
 #include <react/renderer/animated/drivers/AnimationDriver.h>
 #include <react/renderer/animated/drivers/AnimationDriverUtils.h>
@@ -658,7 +659,9 @@ bool NativeAnimatedNodesManager::onAnimationFrame(double timestamp) {
 
     if (driver->getIsComplete()) {
       hasFinishedAnimations = true;
-      finishedAnimationValueNodes.insert(driver->getAnimatedValueTag());
+      if (ReactNativeFeatureFlags::cxxNativeAnimatedRemoveJsSync()) {
+        finishedAnimationValueNodes.insert(driver->getAnimatedValueTag());
+      }
     }
   }
 

packages/react-native/src/private/animated/createAnimatedPropsHook.js

@@ -132,7 +132,13 @@ export default function createAnimatedPropsHook(
           if (node.__isNative) {
             // Check 2: this is an animation driven by native.
             // In native driven animations, this callback is only called once the animation completes.
-            if (isFabricNode) {
+            if (
+              isFabricNode &&
+              !(
+                ReactNativeFeatureFlags.cxxNativeAnimatedEnabled() &&
+                ReactNativeFeatureFlags.cxxNativeAnimatedRemoveJsSync()
+              )
+            ) {
               // Call `scheduleUpdate` to synchronise Fiber and Shadow tree.
               // Must not be called in Paper.
               scheduleUpdate();