From b8ca59cd745ffc74a669603875e368d16fd93465 Mon Sep 17 00:00:00 2001 From: Joshua Gross Date: Mon, 28 Dec 2020 22:33:01 -0800 Subject: [PATCH] Reduce re-rendering of Animated components in Fabric Summary: The `isFabric` method used in createAnimatedComponent is unreliable (another reason in a long list of reasons why you should not duplicate this code elsewhere, and why we want to delete it ASAP). In particular, during the first render, the ref component has not been set yet, so we /cannot/ detect if the component is Fabric or non-Fabric and assume it's non-Fabric. In Fabric, this causes `collapsable` and `nativeID` values to change after the first render. To reduce this re-rendering, but not eliminate it for all components, I've introduced a flag that indicates if a component will /never/ be flattened. In particular, Image components, ScrollViews, Text cannot ever be flattened, so we should always pass `collapsable:false` and the same nativeID prop for those components. For Animated s and other components, the re-rendering issue is still a problem in Fabric for now. Changelog: [Internal] Reviewed By: mdvacca Differential Revision: D25720322 fbshipit-source-id: fe3234d8ae974911a2b5f82e4f6a093216f43d4b --- .../Animated/components/AnimatedImage.js | 6 ++-- .../Animated/components/AnimatedScrollView.js | 6 ++-- Libraries/Animated/components/AnimatedText.js | 6 ++-- Libraries/Animated/components/AnimatedView.js | 4 ++- Libraries/Animated/createAnimatedComponent.js | 35 +++++++++++++------ 5 files changed, 36 insertions(+), 21 deletions(-) diff --git a/Libraries/Animated/components/AnimatedImage.js b/Libraries/Animated/components/AnimatedImage.js index 56cdb7d18f7..5aae7d90056 100644 --- a/Libraries/Animated/components/AnimatedImage.js +++ b/Libraries/Animated/components/AnimatedImage.js @@ -17,9 +17,9 @@ const createAnimatedComponent = require('../createAnimatedComponent'); import type {AnimatedComponentType} from '../createAnimatedComponent'; -module.exports = (createAnimatedComponent( - (Image: $FlowFixMe), -): AnimatedComponentType< +module.exports = (createAnimatedComponent((Image: $FlowFixMe), { + collapsable: false, +}): AnimatedComponentType< React.ElementConfig, React.ElementRef, >); diff --git a/Libraries/Animated/components/AnimatedScrollView.js b/Libraries/Animated/components/AnimatedScrollView.js index 259cbb81b0e..bfa75b7167b 100644 --- a/Libraries/Animated/components/AnimatedScrollView.js +++ b/Libraries/Animated/components/AnimatedScrollView.js @@ -24,9 +24,9 @@ const ScrollViewWithEventThrottle = React.forwardRef((props, ref) => ( )); -module.exports = (createAnimatedComponent( - ScrollViewWithEventThrottle, -): AnimatedComponentType< +module.exports = (createAnimatedComponent(ScrollViewWithEventThrottle, { + collapsable: false, +}): AnimatedComponentType< React.ElementConfig, React.ElementRef, >); diff --git a/Libraries/Animated/components/AnimatedText.js b/Libraries/Animated/components/AnimatedText.js index 5a184e0626f..7a940093aad 100644 --- a/Libraries/Animated/components/AnimatedText.js +++ b/Libraries/Animated/components/AnimatedText.js @@ -17,9 +17,9 @@ const createAnimatedComponent = require('../createAnimatedComponent'); import type {AnimatedComponentType} from '../createAnimatedComponent'; -module.exports = (createAnimatedComponent( - (Text: $FlowFixMe), -): AnimatedComponentType< +module.exports = (createAnimatedComponent((Text: $FlowFixMe), { + collapsable: false, +}): AnimatedComponentType< React.ElementConfig, React.ElementRef, >); diff --git a/Libraries/Animated/components/AnimatedView.js b/Libraries/Animated/components/AnimatedView.js index 0ce54601fe9..d2a87807b02 100644 --- a/Libraries/Animated/components/AnimatedView.js +++ b/Libraries/Animated/components/AnimatedView.js @@ -17,7 +17,9 @@ const createAnimatedComponent = require('../createAnimatedComponent'); import type {AnimatedComponentType} from '../createAnimatedComponent'; -module.exports = (createAnimatedComponent(View): AnimatedComponentType< +module.exports = (createAnimatedComponent(View, { + collapsable: true, +}): AnimatedComponentType< React.ElementConfig, React.ElementRef, >); diff --git a/Libraries/Animated/createAnimatedComponent.js b/Libraries/Animated/createAnimatedComponent.js index 230aa164203..8e8638edb63 100644 --- a/Libraries/Animated/createAnimatedComponent.js +++ b/Libraries/Animated/createAnimatedComponent.js @@ -37,8 +37,13 @@ export type AnimatedComponentType< Instance, >; +type AnimatedComponentOptions = { + collapsable?: boolean, +}; + function createAnimatedComponent( Component: React.AbstractComponent, + options?: AnimatedComponentOptions, ): AnimatedComponentType { invariant( typeof Component !== 'function' || @@ -79,6 +84,9 @@ function createAnimatedComponent( } _isFabric = (): boolean => { + // When called during the first render, `_component` is always null. + // Therefore, even if a component is rendered in Fabric, we can't detect + // that until ref is set, which happens sometime after the first render. if (this._component == null) { return false; } @@ -213,23 +221,28 @@ function createAnimatedComponent( const {style: passthruStyle = {}, ...passthruProps} = this.props.passthroughAnimatedPropExplicitValues || {}; const mergedStyle = {...style, ...passthruStyle}; + const forceNativeId = + props.collapsable ?? + (this._propsAnimated.__isNative || + this._isFabric() || + options?.collapsable === false); + // The native driver updates views directly through the UI thread so we + // have to make sure the view doesn't get optimized away because it cannot + // go through the NativeViewHierarchyManager since it operates on the shadow + // thread. TODO: T68258846 + const collapsableProps = forceNativeId + ? { + nativeID: props.nativeID ?? 'animatedComponent', + collapsable: false, + } + : {}; return ( ); }