Skip to content

Commit

Permalink
Fix order of operations when creating native DAG
Browse files Browse the repository at this point in the history
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
  • Loading branch information
genkikondo authored and facebook-github-bot committed Mar 2, 2023
1 parent b044ece commit c72c592
Showing 1 changed file with 7 additions and 6 deletions.
13 changes: 7 additions & 6 deletions Libraries/Animated/nodes/AnimatedProps.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit c72c592

Please sign in to comment.