From a04195167bbd8f27c6141c0239a61a345cac5a88 Mon Sep 17 00:00:00 2001 From: Genki Kondo Date: Fri, 3 Jun 2022 13:33:50 -0700 Subject: [PATCH] Only use value of natively driven nodes on initial render Summary: D36612758 (https://github.com/facebook/react-native/commit/40f4c662bc7a66e5caea4909d74f435f5b72190c) attempted to remove the check that the animated node was native when fetching animated prop values for render, in order to fix an issue that arises when a node is converted to native before the initial call to render to mount the component, where the initial value is not applied. However, this causes issues for cases where we call setValue on an animated node soon after render, such as on componentDidUpdate. setValue first updates the animated node value on JS side, then calls the native API setAnimatedNodeValue. The problem is that the next render will then use these updated values in the style props (because we removed the check for native in D36612758 (https://github.com/facebook/react-native/commit/40f4c662bc7a66e5caea4909d74f435f5b72190c)), resulting in a diff in props. Since Animated and Fabric aren't synchronized, there's a race condition between SurfaceMountingManager.updateProps and NativeAnimatedNodesManager.runUpdates To allow proper rendering of initial values of native animated nodes, and mitigate the issue when calling setValue on an animated node soon after render, during initial render we use the initial values of both native and non-native driven nodes, and on subsequent renders we only use the initial values of non-native driven nodes. An alternative considered was to add internal state to the nodes themselves (AnimatedProps, AnimatedStyle), but keeping it in sync with the component state is not easy/clean as AnimatedProps can be recreated and reattached at any time for a mounted component. Changelog: [Internal][Fixed] - Only use value of natively driven nodes on initial render Reviewed By: JoshuaGross Differential Revision: D36902220 fbshipit-source-id: c20f711aa97d18a2ed549b5f90c6296bf19bb327 --- Libraries/Animated/createAnimatedComponent.js | 5 ++++- Libraries/Animated/nodes/AnimatedProps.js | 11 +++++++++-- Libraries/Animated/nodes/AnimatedStyle.js | 15 ++++++++++----- 3 files changed, 23 insertions(+), 8 deletions(-) diff --git a/Libraries/Animated/createAnimatedComponent.js b/Libraries/Animated/createAnimatedComponent.js index 20290c94911009..a59b5fd4fd3877 100644 --- a/Libraries/Animated/createAnimatedComponent.js +++ b/Libraries/Animated/createAnimatedComponent.js @@ -55,6 +55,7 @@ function createAnimatedComponent( _prevComponent: any; _propsAnimated: AnimatedProps; _eventDetachers: Array = []; + _isInitialRender: boolean = true; // Only to be used in this file, and only in Fabric. _animatedComponentId: string = `${animatedComponentNextId++}:animatedComponent`; @@ -200,7 +201,8 @@ function createAnimatedComponent( }); render() { - const {style = {}, ...props} = this._propsAnimated.__getValue() || {}; + const {style = {}, ...props} = + this._propsAnimated.__getValue(this._isInitialRender) || {}; const {style: passthruStyle = {}, ...passthruProps} = this.props.passthroughAnimatedPropExplicitValues || {}; const mergedStyle = {...style, ...passthruStyle}; @@ -232,6 +234,7 @@ function createAnimatedComponent( this._propsAnimated.setNativeView(this._component); this._attachNativeEvents(); this._markUpdateComplete(); + this._isInitialRender = false; } UNSAFE_componentWillReceiveProps(newProps: any) { diff --git a/Libraries/Animated/nodes/AnimatedProps.js b/Libraries/Animated/nodes/AnimatedProps.js index 44e4759750e741..ead32545bc8e64 100644 --- a/Libraries/Animated/nodes/AnimatedProps.js +++ b/Libraries/Animated/nodes/AnimatedProps.js @@ -36,12 +36,19 @@ class AnimatedProps extends AnimatedNode { this._callback = callback; } - __getValue(): Object { + __getValue(isInitialRender: boolean = true): Object { const props: {[string]: any | ((...args: any) => void)} = {}; for (const key in this._props) { const value = this._props[key]; if (value instanceof AnimatedNode) { - props[key] = value.__getValue(); + // During initial render we want to use the initial value of both natively and non-natively + // driven nodes. On subsequent renders, we cannot use the value of natively driven nodes + // as they may not be up to date. + if (value instanceof AnimatedStyle) { + props[key] = value.__getValue(isInitialRender); + } else if (isInitialRender || !value.__isNative) { + props[key] = value.__getValue(); + } } else if (value instanceof AnimatedEvent) { props[key] = value.__getHandler(); } else { diff --git a/Libraries/Animated/nodes/AnimatedStyle.js b/Libraries/Animated/nodes/AnimatedStyle.js index 0d70430b872681..79df1468d253ab 100644 --- a/Libraries/Animated/nodes/AnimatedStyle.js +++ b/Libraries/Animated/nodes/AnimatedStyle.js @@ -34,15 +34,20 @@ class AnimatedStyle extends AnimatedWithChildren { } // Recursively get values for nested styles (like iOS's shadowOffset) - _walkStyleAndGetValues(style: any) { + _walkStyleAndGetValues(style: any, isInitialRender: boolean) { const updatedStyle: {[string]: any | {...}} = {}; for (const key in style) { const value = style[key]; if (value instanceof AnimatedNode) { - updatedStyle[key] = value.__getValue(); + // During initial render we want to use the initial value of both natively and non-natively + // driven nodes. On subsequent renders, we cannot use the value of natively driven nodes + // as they may not be up to date. + if (isInitialRender || !value.__isNative) { + updatedStyle[key] = value.__getValue(); + } } else if (value && !Array.isArray(value) && typeof value === 'object') { // Support animating nested values (for example: shadowOffset.height) - updatedStyle[key] = this._walkStyleAndGetValues(value); + updatedStyle[key] = this._walkStyleAndGetValues(value, isInitialRender); } else { updatedStyle[key] = value; } @@ -50,8 +55,8 @@ class AnimatedStyle extends AnimatedWithChildren { return updatedStyle; } - __getValue(): Object { - return this._walkStyleAndGetValues(this._style); + __getValue(isInitialRender: boolean = true): Object { + return this._walkStyleAndGetValues(this._style, isInitialRender); } // Recursively get animated values for nested styles (like iOS's shadowOffset)