From 53bc4d988681d816c93e23ec41fba19aca35b8b1 Mon Sep 17 00:00:00 2001 From: Dmitry Rykun Date: Thu, 25 Jul 2024 07:44:57 -0700 Subject: [PATCH] Do not send state updates for loop animations Summary: **Motivation** There is a special [code path](https://github.com/facebook/react-native/blob/bb23026daf1a853f4482be46d6f242712a6b7330/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/animated/FrameBasedAnimationDriver.java#L98) for native driven looping animations that restarts the animation in native, never calling the "animation end" callback. This was designed for and works fine with the `Animated.loop(Animated.timing(...))` animations. Unfortunately, it doesn't work well with more complex animations. Consider an `Animated.loop(Animated.sequence(Animated.timing(...)))` animation. It doesn't trigger this code path. Instead, its nested `Animated.timing` animations are continuously rescheduled from JS by the looping `Animated.sequence` as singular native driven animations. Each time they end, they fire their "animation end" callback. This introduces a subtle breakage when those "animation end" callbacks trigger React to update its state. This in turn restarts the rendering. In case with long transitions such looping animations can restart the rendering multiple times. In worst cases it may render the app unresponsive. **Solution** We don't need to tell React to update its state when running looping animations. This diff introduces a mechanism, using which `Animated.loop` can tell its nested animations that they are in a loop, and and there's no need to send state updates when they finish. This is consistent with how `Animated.loop(Animated.timing(...))` behaves. Changelog: [General][Breaking] - Looping animations will not send React state updates. Facebook This diff enables this new behaviour for IGVR and FBVR, it also set up the experiment for FBiOS and FB4a. Reviewed By: javache Differential Revision: D59970265 --- .../Libraries/Animated/AnimatedImplementation.js | 14 +++++++------- .../Libraries/Animated/animations/Animation.js | 10 ++++++++++ .../Animated/animations/TimingAnimation.js | 1 + .../__snapshots__/public-api-test.js.snap | 4 +++- .../featureflags/ReactNativeFeatureFlags.config.js | 5 +++++ .../featureflags/ReactNativeFeatureFlags.js | 8 +++++++- 6 files changed, 33 insertions(+), 9 deletions(-) diff --git a/packages/react-native/Libraries/Animated/AnimatedImplementation.js b/packages/react-native/Libraries/Animated/AnimatedImplementation.js index a11574478a35ce..0677cc0fd4b280 100644 --- a/packages/react-native/Libraries/Animated/AnimatedImplementation.js +++ b/packages/react-native/Libraries/Animated/AnimatedImplementation.js @@ -39,7 +39,7 @@ import AnimatedValue from './nodes/AnimatedValue'; import AnimatedValueXY from './nodes/AnimatedValueXY'; export type CompositeAnimation = { - start: (callback?: ?EndCallback) => void, + start: (callback?: ?EndCallback, isLooping?: boolean) => void, stop: () => void, reset: () => void, _startNativeLoop: (iterations?: number) => void, @@ -234,8 +234,8 @@ const timing = function ( return ( maybeVectorAnim(value, config, timing) || { - start: function (callback?: ?EndCallback): void { - start(value, config, callback); + start: function (callback?: ?EndCallback, isLooping?: boolean): void { + start(value, {...config, isLooping}, callback); }, stop: function (): void { @@ -305,7 +305,7 @@ const sequence = function ( ): CompositeAnimation { let current = 0; return { - start: function (callback?: ?EndCallback) { + start: function (callback?: ?EndCallback, isLooping?: boolean) { const onComplete = function (result: EndResult) { if (!result.finished) { callback && callback(result); @@ -321,13 +321,13 @@ const sequence = function ( return; } - animations[current].start(onComplete); + animations[current].start(onComplete, isLooping); }; if (animations.length === 0) { callback && callback({finished: true}); } else { - animations[current].start(onComplete); + animations[current].start(onComplete, isLooping); } }, @@ -477,7 +477,7 @@ const loop = function ( } else { iterationsSoFar++; resetBeforeIteration && animation.reset(); - animation.start(restart); + animation.start(restart, iterations === -1); } }; if (!animation || iterations === 0) { diff --git a/packages/react-native/Libraries/Animated/animations/Animation.js b/packages/react-native/Libraries/Animated/animations/Animation.js index c93dc21f58b7b0..357af8c30fb107 100644 --- a/packages/react-native/Libraries/Animated/animations/Animation.js +++ b/packages/react-native/Libraries/Animated/animations/Animation.js @@ -14,6 +14,7 @@ import type {PlatformConfig} from '../AnimatedPlatformConfig'; import type AnimatedNode from '../nodes/AnimatedNode'; import type AnimatedValue from '../nodes/AnimatedValue'; +import * as ReactNativeFeatureFlags from '../../../src/private/featureflags/ReactNativeFeatureFlags'; import NativeAnimatedHelper from '../NativeAnimatedHelper'; import AnimatedProps from '../nodes/AnimatedProps'; @@ -26,6 +27,7 @@ export type AnimationConfig = { platformConfig?: PlatformConfig, onComplete?: ?EndCallback, iterations?: number, + isLooping?: boolean, }; let startNativeAnimationNextId = 1; @@ -38,6 +40,7 @@ export default class Animation { __isInteraction: boolean; __onEnd: ?EndCallback; __iterations: number; + __isLooping: ?boolean; _nativeId: number; @@ -107,6 +110,13 @@ export default class Animation { if (value != null) { animatedValue.__onAnimatedValueUpdateReceived(value); + if ( + ReactNativeFeatureFlags.shouldSkipStateUpdatesForLoopingAnimations() && + this.__isLooping + ) { + return; + } + // Once the JS side node is synced with the updated values, trigger an // update on the AnimatedProps nodes to call any registered callbacks. this.__findAnimatedPropsNodes(animatedValue).forEach(node => diff --git a/packages/react-native/Libraries/Animated/animations/TimingAnimation.js b/packages/react-native/Libraries/Animated/animations/TimingAnimation.js index a32c7543c68ba4..cbcc81629c42ad 100644 --- a/packages/react-native/Libraries/Animated/animations/TimingAnimation.js +++ b/packages/react-native/Libraries/Animated/animations/TimingAnimation.js @@ -80,6 +80,7 @@ export default class TimingAnimation extends Animation { this._useNativeDriver = NativeAnimatedHelper.shouldUseNativeDriver(config); this._platformConfig = config.platformConfig; this.__isInteraction = config.isInteraction ?? !this._useNativeDriver; + this.__isLooping = config.isLooping; } __getNativeAnimationConfig(): any { diff --git a/packages/react-native/Libraries/__tests__/__snapshots__/public-api-test.js.snap b/packages/react-native/Libraries/__tests__/__snapshots__/public-api-test.js.snap index fdcb485b26abca..910d0b3f0f4fd3 100644 --- a/packages/react-native/Libraries/__tests__/__snapshots__/public-api-test.js.snap +++ b/packages/react-native/Libraries/__tests__/__snapshots__/public-api-test.js.snap @@ -153,7 +153,7 @@ declare export class AnimatedEvent { exports[`public API should not change unintentionally Libraries/Animated/AnimatedImplementation.js 1`] = ` "export type CompositeAnimation = { - start: (callback?: ?EndCallback) => void, + start: (callback?: ?EndCallback, isLooping?: boolean) => void, stop: () => void, reset: () => void, _startNativeLoop: (iterations?: number) => void, @@ -521,12 +521,14 @@ export type AnimationConfig = { platformConfig?: PlatformConfig, onComplete?: ?EndCallback, iterations?: number, + isLooping?: boolean, }; declare export default class Animation { __active: boolean; __isInteraction: boolean; __onEnd: ?EndCallback; __iterations: number; + __isLooping: ?boolean; _nativeId: number; start( fromValue: number, diff --git a/packages/react-native/scripts/featureflags/ReactNativeFeatureFlags.config.js b/packages/react-native/scripts/featureflags/ReactNativeFeatureFlags.config.js index 7b6893b3693e11..f092601a56ad1d 100644 --- a/packages/react-native/scripts/featureflags/ReactNativeFeatureFlags.config.js +++ b/packages/react-native/scripts/featureflags/ReactNativeFeatureFlags.config.js @@ -242,6 +242,11 @@ const definitions: FeatureFlagDefinitions = { description: 'Function used to enable / disabled Layout Animations in React Native.', }, + shouldSkipStateUpdatesForLoopingAnimations: { + defaultValue: false, + description: + 'If the animation is within Animated.loop, we do not send state updates to React.', + }, shouldUseAnimatedObjectForTransform: { defaultValue: false, description: diff --git a/packages/react-native/src/private/featureflags/ReactNativeFeatureFlags.js b/packages/react-native/src/private/featureflags/ReactNativeFeatureFlags.js index ebbdb13b3a03c1..f396bc2e38f85b 100644 --- a/packages/react-native/src/private/featureflags/ReactNativeFeatureFlags.js +++ b/packages/react-native/src/private/featureflags/ReactNativeFeatureFlags.js @@ -4,7 +4,7 @@ * This source code is licensed under the MIT license found in the * LICENSE file in the root directory of this source tree. * - * @generated SignedSource<> + * @generated SignedSource<> * @flow strict-local */ @@ -31,6 +31,7 @@ export type ReactNativeFeatureFlagsJsOnly = { animatedShouldUseSingleOp: Getter, enableAccessToHostTreeInFabric: Getter, isLayoutAnimationEnabled: Getter, + shouldSkipStateUpdatesForLoopingAnimations: Getter, shouldUseAnimatedObjectForTransform: Getter, shouldUseRemoveClippedSubviewsAsDefaultOnIOS: Getter, shouldUseSetNativePropsInFabric: Getter, @@ -107,6 +108,11 @@ export const enableAccessToHostTreeInFabric: Getter = createJavaScriptF */ export const isLayoutAnimationEnabled: Getter = createJavaScriptFlagGetter('isLayoutAnimationEnabled', true); +/** + * If the animation is within Animated.loop, we do not send state updates to React. + */ +export const shouldSkipStateUpdatesForLoopingAnimations: Getter = createJavaScriptFlagGetter('shouldSkipStateUpdatesForLoopingAnimations', false); + /** * Enables use of AnimatedObject for animating transform values. */