From c72c592ecd9d31ec1661958d4e5f77f8dfb37cac Mon Sep 17 00:00:00 2001
From: Genki Kondo <gkondo@meta.com>
Date: Thu, 2 Mar 2023 10:28:30 -0800
Subject: [PATCH] Fix order of operations when creating native DAG

Summary:
## Issue

You will run into an exception like:
  Animated node with tag (child) [8] does not exist
when attempting to convert an Animated node (or starting a native animation) to native when there are multiple non-style props that have AnimatedValues (this should be a very rare case, unless you have a custom native component, as most animated props are in style).

## Root cause

Due to how __makeNative is recursively called up and down the node graph, in a specific case where multiple AnimatedValues have an AnimatedProps as their child, the "connect" operation will occur before the "create" operation for the AnimatedProps node. For example:

- When a native animation is fired via Animated.timing().start(), __makeNative is called on the AnimatedValue (call this node AnimatedValueA) that is being animated. This results in the following sequence of events:
  - AnimatedValueA iterates through its children (in this case, AnimatedProps) and calls child.__makeNative
  - AnimatedProps iterates through all props with AnimatedValue values and calls __makeNative on them (call these nodes AnimatedValueB and AnimatedValueC).
  - AnimatedValueA.__makeNative is called again, but this time, the call early exits and does not do anything.
  - AnimatedValueB.__makeNative is called. This in turn calls AnimatedProps.__makeNative, which early exits (ROOT CAUSE OF BUG).
    - connectAnimatedNodes is queued (undesired)

## Fix

Short of completely refactoring of how we handle converting the DAG to native (which we should do at some point - the current implementation is error prone and suboptimal. Tracking in T146991336), for now we can make AnimatedProps.__makeNative behavior consistent with AnimatedTransform and AnimatedStyle by just avoiding the early exit, so that the connectAnimatedNodes calls all occur after all native nodes have been created

Changelog:
[General][Fixed] - Fixed error during native DAG creation when there are multiple AnimatedValue props

Reviewed By: rshest

Differential Revision: D43717819

fbshipit-source-id: 258682300a2be65935646b499591acf41eabc56e
---
 Libraries/Animated/nodes/AnimatedProps.js | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/Libraries/Animated/nodes/AnimatedProps.js b/Libraries/Animated/nodes/AnimatedProps.js
index ef15cdf1312..d6ebbb02e3b 100644
--- a/Libraries/Animated/nodes/AnimatedProps.js
+++ b/Libraries/Animated/nodes/AnimatedProps.js
@@ -90,14 +90,15 @@ export default class AnimatedProps extends AnimatedNode {
   }
 
   __makeNative(platformConfig: ?PlatformConfig): void {
+    for (const key in this._props) {
+      const value = this._props[key];
+      if (value instanceof AnimatedNode) {
+        value.__makeNative(platformConfig);
+      }
+    }
+
     if (!this.__isNative) {
       this.__isNative = true;
-      for (const key in this._props) {
-        const value = this._props[key];
-        if (value instanceof AnimatedNode) {
-          value.__makeNative(platformConfig);
-        }
-      }
 
       // Since this does not call the super.__makeNative, we need to store the
       // supplied platformConfig here, before calling __connectAnimatedView