From ab77bdf4715dd1200c4f2ff64e370e7de434b5fe Mon Sep 17 00:00:00 2001 From: Tim Yung Date: Wed, 22 Jan 2025 08:54:58 -0800 Subject: [PATCH] Animated: Feature Flags Cleanup (#48509) Summary: Pull Request resolved: https://github.com/facebook/react-native/pull/48509 Cleans up the following feature flags from `Animated`: - `enableAnimatedAllowlist` - `enableAnimatedPropsMemo` - `useInsertionEffectsForAnimations` This will significantly simplify some future planned work here (e.g. T209740497). Changelog: [Internal] Reviewed By: javache Differential Revision: D67867115 fbshipit-source-id: adf35c70d95f42c240342fda3b4f2e9b4bdfe30a --- .../Libraries/Animated/useAnimatedProps.js | 92 +------------------ .../__snapshots__/public-api-test.js.snap | 6 -- .../ReactNativeFeatureFlags.config.js | 27 ------ .../animated/__tests__/AnimatedNative-test.js | 30 ------ .../featureflags/ReactNativeFeatureFlags.js | 20 +--- 5 files changed, 6 insertions(+), 169 deletions(-) diff --git a/packages/react-native/Libraries/Animated/useAnimatedProps.js b/packages/react-native/Libraries/Animated/useAnimatedProps.js index 5130964f7ed..aa4fde0e193 100644 --- a/packages/react-native/Libraries/Animated/useAnimatedProps.js +++ b/packages/react-native/Libraries/Animated/useAnimatedProps.js @@ -24,8 +24,6 @@ import { useCallback, useEffect, useInsertionEffect, - useLayoutEffect, - useMemo, useReducer, useRef, } from 'react'; @@ -44,11 +42,6 @@ type AnimatedValueListeners = Array<{ listenerId: string, }>; -const useMemoOrAnimatedPropsMemo = - ReactNativeFeatureFlags.enableAnimatedPropsMemo() - ? useAnimatedPropsMemo - : useMemo; - export default function useAnimatedProps( props: TProps, allowlist?: ?AnimatedPropsAllowlist, @@ -57,28 +50,14 @@ export default function useAnimatedProps( const onUpdateRef = useRef(null); const timerRef = useRef(null); - const allowlistIfEnabled = ReactNativeFeatureFlags.enableAnimatedAllowlist() - ? allowlist - : null; - - const node = useMemoOrAnimatedPropsMemo( - () => - new AnimatedProps( - props, - () => onUpdateRef.current?.(), - allowlistIfEnabled, - ), - [allowlistIfEnabled, props], + const node = useAnimatedPropsMemo( + () => new AnimatedProps(props, () => onUpdateRef.current?.(), allowlist), + [allowlist, props], ); const useNativePropsInFabric = ReactNativeFeatureFlags.shouldUseSetNativePropsInFabric(); - const useAnimatedPropsLifecycle = - ReactNativeFeatureFlags.useInsertionEffectsForAnimations() - ? useAnimatedPropsLifecycle_insertionEffects - : useAnimatedPropsLifecycle_layoutEffects; - useAnimatedPropsLifecycle(node); // TODO: This "effect" does three things: @@ -208,9 +187,7 @@ function reduceAnimatedProps( // Force `collapsable` to be false so that the native view is not flattened. // Flattened views cannot be accurately referenced by the native driver. return { - ...(ReactNativeFeatureFlags.enableAnimatedPropsMemo() - ? node.__getValueWithStaticProps(props) - : node.__getValue()), + ...node.__getValueWithStaticProps(props), collapsable: false, }; } @@ -251,66 +228,7 @@ function addAnimatedValuesListenersToProps( * nodes. So in order to optimize this, we avoid detaching until the next attach * unless we are unmounting. */ -function useAnimatedPropsLifecycle_layoutEffects(node: AnimatedProps): void { - const prevNodeRef = useRef(null); - const isUnmountingRef = useRef(false); - - useEffect(() => { - // It is ok for multiple components to call `flushQueue` because it noops - // if the queue is empty. When multiple animated components are mounted at - // the same time. Only first component flushes the queue and the others will noop. - NativeAnimatedHelper.API.flushQueue(); - let drivenAnimationEndedListener: ?EventSubscription = null; - if (node.__isNative) { - drivenAnimationEndedListener = - NativeAnimatedHelper.nativeEventEmitter.addListener( - 'onUserDrivenAnimationEnded', - data => { - node.update(); - }, - ); - } - - return () => { - drivenAnimationEndedListener?.remove(); - }; - }); - - useLayoutEffect(() => { - isUnmountingRef.current = false; - return () => { - isUnmountingRef.current = true; - }; - }, []); - - useLayoutEffect(() => { - node.__attach(); - if (prevNodeRef.current != null) { - const prevNode = prevNodeRef.current; - // TODO: Stop restoring default values (unless `reset` is called). - prevNode.__restoreDefaultValues(); - prevNode.__detach(); - prevNodeRef.current = null; - } - return () => { - if (isUnmountingRef.current) { - // NOTE: Do not restore default values on unmount, see D18197735. - node.__detach(); - } else { - prevNodeRef.current = node; - } - }; - }, [node]); -} - -/** - * Manages the lifecycle of the supplied `AnimatedProps` by invoking `__attach` - * and `__detach`. However, this is more complicated because `AnimatedProps` - * uses reference counting to determine when to recursively detach its children - * nodes. So in order to optimize this, we avoid detaching until the next attach - * unless we are unmounting. - */ -function useAnimatedPropsLifecycle_insertionEffects(node: AnimatedProps): void { +function useAnimatedPropsLifecycle(node: AnimatedProps): void { const prevNodeRef = useRef(null); const isUnmountingRef = useRef(false); 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 ecce66ed4fd..23cab6685c4 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 @@ -9856,16 +9856,13 @@ exports[`public API should not change unintentionally src/private/featureflags/R animatedShouldUseSingleOp: Getter, disableInteractionManager: Getter, enableAccessToHostTreeInFabric: Getter, - enableAnimatedAllowlist: Getter, enableAnimatedClearImmediateFix: Getter, - enableAnimatedPropsMemo: Getter, fixVirtualizeListCollapseWindowSize: Getter, isLayoutAnimationEnabled: Getter, shouldSkipStateUpdatesForLoopingAnimations: Getter, shouldUseAnimatedObjectForTransform: Getter, shouldUseRemoveClippedSubviewsAsDefaultOnIOS: Getter, shouldUseSetNativePropsInFabric: Getter, - useInsertionEffectsForAnimations: Getter, useRefsForTextInputState: Getter, }>; export type ReactNativeFeatureFlagsJsOnlyOverrides = @@ -9925,16 +9922,13 @@ declare export const animatedShouldDebounceQueueFlush: Getter; declare export const animatedShouldUseSingleOp: Getter; declare export const disableInteractionManager: Getter; declare export const enableAccessToHostTreeInFabric: Getter; -declare export const enableAnimatedAllowlist: Getter; declare export const enableAnimatedClearImmediateFix: Getter; -declare export const enableAnimatedPropsMemo: Getter; declare export const fixVirtualizeListCollapseWindowSize: Getter; declare export const isLayoutAnimationEnabled: Getter; declare export const shouldSkipStateUpdatesForLoopingAnimations: Getter; declare export const shouldUseAnimatedObjectForTransform: Getter; declare export const shouldUseRemoveClippedSubviewsAsDefaultOnIOS: Getter; declare export const shouldUseSetNativePropsInFabric: Getter; -declare export const useInsertionEffectsForAnimations: Getter; declare export const useRefsForTextInputState: Getter; declare export const commonTestFlag: Getter; declare export const commonTestFlagWithoutNativeImplementation: Getter; diff --git a/packages/react-native/scripts/featureflags/ReactNativeFeatureFlags.config.js b/packages/react-native/scripts/featureflags/ReactNativeFeatureFlags.config.js index 4a0f089be23..4e757703050 100644 --- a/packages/react-native/scripts/featureflags/ReactNativeFeatureFlags.config.js +++ b/packages/react-native/scripts/featureflags/ReactNativeFeatureFlags.config.js @@ -529,15 +529,6 @@ const definitions: FeatureFlagDefinitions = { purpose: 'release', }, }, - enableAnimatedAllowlist: { - defaultValue: true, - metadata: { - description: - 'Enables Animated to skip non-allowlisted props and styles.', - expectedReleaseValue: true, - purpose: 'release', - }, - }, enableAnimatedClearImmediateFix: { defaultValue: true, metadata: { @@ -548,15 +539,6 @@ const definitions: FeatureFlagDefinitions = { purpose: 'experimentation', }, }, - enableAnimatedPropsMemo: { - defaultValue: true, - metadata: { - description: - 'Enables Animated to analyze props to minimize invalidating `AnimatedProps`.', - expectedReleaseValue: true, - purpose: 'release', - }, - }, fixVirtualizeListCollapseWindowSize: { defaultValue: false, metadata: { @@ -615,15 +597,6 @@ const definitions: FeatureFlagDefinitions = { purpose: 'experimentation', }, }, - useInsertionEffectsForAnimations: { - defaultValue: true, - metadata: { - description: - 'Changes construction of the animation graph to `useInsertionEffect` instead of `useLayoutEffect`.', - expectedReleaseValue: true, - purpose: 'release', - }, - }, useRefsForTextInputState: { defaultValue: false, metadata: { diff --git a/packages/react-native/src/private/animated/__tests__/AnimatedNative-test.js b/packages/react-native/src/private/animated/__tests__/AnimatedNative-test.js index 29301cf8e97..bab75087aaf 100644 --- a/packages/react-native/src/private/animated/__tests__/AnimatedNative-test.js +++ b/packages/react-native/src/private/animated/__tests__/AnimatedNative-test.js @@ -1362,9 +1362,6 @@ describe('Native Animated', () => { describe('Animated Components', () => { it('preserves current values on update and unmount', async () => { - const {ReactNativeFeatureFlags} = importModules(); - ReactNativeFeatureFlags.override({enableAnimatedPropsMemo: () => true}); - const {Animated} = importModules(); const opacity = new Animated.Value(0); @@ -1382,9 +1379,6 @@ describe('Native Animated', () => { }); it('restores defaults when receiving new animated values', async () => { - const {ReactNativeFeatureFlags} = importModules(); - ReactNativeFeatureFlags.override({enableAnimatedPropsMemo: () => true}); - const {Animated} = importModules(); const opacityA = new Animated.Value(0); @@ -1407,30 +1401,6 @@ describe('Native Animated', () => { ); }); - it('should restore default values on prop updates only', async () => { - const {ReactNativeFeatureFlags} = importModules(); - ReactNativeFeatureFlags.override({enableAnimatedPropsMemo: () => false}); - - const {Animated} = importModules(); - - const opacity = new Animated.Value(0); - opacity.__makeNative(); - - const root = await create(); - expect(NativeAnimatedModule.restoreDefaultValues).not.toHaveBeenCalled(); - - await update(root, ); - expect(NativeAnimatedModule.restoreDefaultValues).toHaveBeenCalledWith( - expect.any(Number), - ); - - await unmount(root); - // Make sure it doesn't get called on unmount. - expect(NativeAnimatedModule.restoreDefaultValues).toHaveBeenCalledTimes( - 1, - ); - }); - it('connects the native view on mount and disconnects on unmount', async () => { const {Animated} = importModules(); diff --git a/packages/react-native/src/private/featureflags/ReactNativeFeatureFlags.js b/packages/react-native/src/private/featureflags/ReactNativeFeatureFlags.js index 77dfb398add..64e4d92a8bb 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<<811a054f0ada56d8cc1ed21c6930fc52>> * @flow strict */ @@ -32,16 +32,13 @@ export type ReactNativeFeatureFlagsJsOnly = $ReadOnly<{ animatedShouldUseSingleOp: Getter, disableInteractionManager: Getter, enableAccessToHostTreeInFabric: Getter, - enableAnimatedAllowlist: Getter, enableAnimatedClearImmediateFix: Getter, - enableAnimatedPropsMemo: Getter, fixVirtualizeListCollapseWindowSize: Getter, isLayoutAnimationEnabled: Getter, shouldSkipStateUpdatesForLoopingAnimations: Getter, shouldUseAnimatedObjectForTransform: Getter, shouldUseRemoveClippedSubviewsAsDefaultOnIOS: Getter, shouldUseSetNativePropsInFabric: Getter, - useInsertionEffectsForAnimations: Getter, useRefsForTextInputState: Getter, }>; @@ -123,21 +120,11 @@ export const disableInteractionManager: Getter = createJavaScriptFlagGe */ export const enableAccessToHostTreeInFabric: Getter = createJavaScriptFlagGetter('enableAccessToHostTreeInFabric', false); -/** - * Enables Animated to skip non-allowlisted props and styles. - */ -export const enableAnimatedAllowlist: Getter = createJavaScriptFlagGetter('enableAnimatedAllowlist', true); - /** * Enables an experimental to use the proper clearIntermediate instead of calling the wrong clearTimeout and canceling another timer. */ export const enableAnimatedClearImmediateFix: Getter = createJavaScriptFlagGetter('enableAnimatedClearImmediateFix', true); -/** - * Enables Animated to analyze props to minimize invalidating `AnimatedProps`. - */ -export const enableAnimatedPropsMemo: Getter = createJavaScriptFlagGetter('enableAnimatedPropsMemo', true); - /** * Fixing an edge case where the current window size is not properly calculated with fast scrolling. Window size collapsed to 1 element even if windowSize more than the current amount of elements */ @@ -168,11 +155,6 @@ export const shouldUseRemoveClippedSubviewsAsDefaultOnIOS: Getter = cre */ export const shouldUseSetNativePropsInFabric: Getter = createJavaScriptFlagGetter('shouldUseSetNativePropsInFabric', true); -/** - * Changes construction of the animation graph to `useInsertionEffect` instead of `useLayoutEffect`. - */ -export const useInsertionEffectsForAnimations: Getter = createJavaScriptFlagGetter('useInsertionEffectsForAnimations', true); - /** * Enable a variant of TextInput that moves some state to refs to avoid unnecessary re-renders */