From ef812a1fcb40a65186e7e7b9ca40240fe7a20855 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20=C5=BBelawski?= <40713406+tjzel@users.noreply.github.com> Date: Tue, 19 Nov 2024 14:17:07 +0100 Subject: [PATCH] fix: Animation assignment with compiler-safe API (#6715) ## Summary When I added compiler-safe API for shared values, I accidentally forgot that animations exist and that their assignment to shared values has to be handled separately. Fixes - #6613 ## Test plan Added relevant runtime test suite and adjusted some others. --- .../RuntimeTests/RuntimeTestsExample.tsx | 2 + .../animations/withTiming/basic.test.tsx | 88 ++++++---- .../tests/core/useDerivedValue/basic.test.tsx | 162 ++++++++++-------- .../animationsCompilerApi.test.tsx | 153 +++++++++++++++++ .../src/animation/util.ts | 4 +- .../react-native-reanimated/src/mutables.ts | 8 +- 6 files changed, 316 insertions(+), 101 deletions(-) create mode 100644 apps/common-app/src/examples/RuntimeTests/tests/core/useSharedValue/animationsCompilerApi.test.tsx diff --git a/apps/common-app/src/examples/RuntimeTests/RuntimeTestsExample.tsx b/apps/common-app/src/examples/RuntimeTests/RuntimeTestsExample.tsx index 59da4a81f0d..6a460dbf4ef 100644 --- a/apps/common-app/src/examples/RuntimeTests/RuntimeTestsExample.tsx +++ b/apps/common-app/src/examples/RuntimeTests/RuntimeTestsExample.tsx @@ -49,6 +49,8 @@ export default function RuntimeTestsExample() { require('./tests/core/useAnimatedStyle/reuseAnimatedStyle.test'); require('./tests/core/useDerivedValue/basic.test'); require('./tests/core/useDerivedValue/chain.test'); + + require('./tests/core/useSharedValue/animationsCompilerApi.test'); }, }, { diff --git a/apps/common-app/src/examples/RuntimeTests/tests/animations/withTiming/basic.test.tsx b/apps/common-app/src/examples/RuntimeTests/tests/animations/withTiming/basic.test.tsx index d1192fdc43c..f7cd63ceb50 100644 --- a/apps/common-app/src/examples/RuntimeTests/tests/animations/withTiming/basic.test.tsx +++ b/apps/common-app/src/examples/RuntimeTests/tests/animations/withTiming/basic.test.tsx @@ -27,7 +27,15 @@ const WIDTH_COMPONENT_PASSIVE_REF = 'WidthComponentPassive'; type Width = number | `${number}%` | 'auto'; describe('withTiming animation of WIDTH', () => { - const WidthComponent = ({ startWidth, finalWidth }: { startWidth: Width; finalWidth: Width }) => { + const WidthComponent = ({ + startWidth, + finalWidth, + compilerApi, + }: { + startWidth: Width; + finalWidth: Width; + compilerApi: boolean; + }) => { const widthActiveSV = useSharedValue(startWidth); const widthPassiveSV = useSharedValue(startWidth); @@ -36,22 +44,30 @@ describe('withTiming animation of WIDTH', () => { const styleActive = useAnimatedStyle(() => { return { - width: withTiming(widthActiveSV.value, { duration: 500 }), + width: withTiming(compilerApi ? widthActiveSV.get() : widthActiveSV.value, { duration: 500 }), }; }); const stylePassive = useAnimatedStyle(() => { return { - width: widthPassiveSV.value, + width: compilerApi ? widthPassiveSV.get() : widthPassiveSV.value, }; }); useEffect(() => { - widthActiveSV.value = finalWidth; - }, [widthActiveSV, finalWidth]); + if (compilerApi) { + widthActiveSV.set(finalWidth); + } else { + widthActiveSV.value = finalWidth; + } + }, [widthActiveSV, finalWidth, compilerApi]); useEffect(() => { - widthPassiveSV.value = withTiming(finalWidth, { duration: 500 }); - }, [widthPassiveSV, finalWidth]); + if (compilerApi) { + widthPassiveSV.set(withTiming(finalWidth, { duration: 500 })); + } else { + widthPassiveSV.value = withTiming(finalWidth, { duration: 500 }); + } + }, [widthPassiveSV, finalWidth, compilerApi]); return ( @@ -69,31 +85,41 @@ describe('withTiming animation of WIDTH', () => { finalWidth: Width; finalWidthInPixels: number; description: string; + compilerApi: boolean; } - test.each([ - { startWidth: 0, finalWidth: 100, finalWidthInPixels: 100, description: 'width in pixels' }, - { - startWidth: '0%', - finalWidth: '100%', - finalWidthInPixels: Dimensions.get('window').width, - description: 'width in percents', - }, - { - startWidth: '0%', - finalWidth: '75%', - finalWidthInPixels: Dimensions.get('window').width * 0.75, - description: 'width in percents', - }, - { - startWidth: 20, - finalWidth: '40%', - finalWidthInPixels: Dimensions.get('window').width * 0.4, - description: 'width from pixels to percents (not supported)', - }, - ] as Array)( + test.each( + [ + { startWidth: 0, finalWidth: 100, finalWidthInPixels: 100, description: 'width in pixels' }, + { + startWidth: '0%', + finalWidth: '100%', + finalWidthInPixels: Dimensions.get('window').width, + description: 'width in percents', + }, + { + startWidth: '0%', + finalWidth: '75%', + finalWidthInPixels: Dimensions.get('window').width * 0.75, + description: 'width in percents', + }, + { + startWidth: 20, + finalWidth: '40%', + finalWidthInPixels: Dimensions.get('window').width * 0.4, + description: 'width from pixels to percents (not supported)', + }, + ].reduce( + (acc, element) => [ + ...acc, + { ...element, compilerApi: false }, + { ...element, compilerApi: true, description: `${element.description} (compiler API)` }, + ], + [] as Record[], + ) as unknown as Array, + )( '${description}, from ${startWidth} to ${finalWidth}', - async ({ startWidth, finalWidth, finalWidthInPixels }: TestCase) => { - await render(); + async ({ startWidth, finalWidth, finalWidthInPixels, compilerApi }: TestCase) => { + await render(); const componentActive = getTestComponent(WIDTH_COMPONENT_ACTIVE_REF); const WidthComponentPassive = getTestComponent(WIDTH_COMPONENT_PASSIVE_REF); await wait(1000); @@ -103,7 +129,7 @@ describe('withTiming animation of WIDTH', () => { ); test('Width from percent to pixels is NOT handled correctly', async () => { - await render(); + await render(); const componentActive = getTestComponent(WIDTH_COMPONENT_ACTIVE_REF); const WidthComponentPassive = getTestComponent(WIDTH_COMPONENT_PASSIVE_REF); await wait(1000); diff --git a/apps/common-app/src/examples/RuntimeTests/tests/core/useDerivedValue/basic.test.tsx b/apps/common-app/src/examples/RuntimeTests/tests/core/useDerivedValue/basic.test.tsx index 71e7a247cab..fb4cbdce871 100644 --- a/apps/common-app/src/examples/RuntimeTests/tests/core/useDerivedValue/basic.test.tsx +++ b/apps/common-app/src/examples/RuntimeTests/tests/core/useDerivedValue/basic.test.tsx @@ -43,12 +43,14 @@ describe('Test useDerivedValue changing width', () => { animate, animationType, deriveFunction, + compilerApi, }: { startWidth: number; finalWidth: number; animate: AnimationLocation; animationType: AnimationType; deriveFunction: (a: number) => number; + compilerApi: boolean; }) => { const basicValue = useSharedValue(startWidth); const componentRef = useTestRef(WIDTH_COMPONENT); @@ -60,20 +62,30 @@ describe('Test useDerivedValue changing width', () => { if (animate === AnimationLocation.ANIMATED_STYLE) { return { width: - animationType === AnimationType.TIMING ? withTiming(derivedValue.value) : withSpring(derivedValue.value), + animationType === AnimationType.TIMING + ? withTiming(compilerApi ? derivedValue.get() : derivedValue.value) + : withSpring(compilerApi ? derivedValue.get() : derivedValue.value), }; } else { - return { width: derivedValue.value }; + return { width: compilerApi ? derivedValue.get() : derivedValue.value }; } }); useEffect(() => { if (animate === AnimationLocation.USE_EFFECT) { - basicValue.value = animationType === AnimationType.TIMING ? withTiming(finalWidth) : withSpring(finalWidth); + if (compilerApi) { + basicValue.set(animationType === AnimationType.TIMING ? withTiming(finalWidth) : withSpring(finalWidth)); + } else { + basicValue.value = animationType === AnimationType.TIMING ? withTiming(finalWidth) : withSpring(finalWidth); + } } else { - basicValue.value = finalWidth; + if (compilerApi) { + basicValue.set(finalWidth); + } else { + basicValue.value = finalWidth; + } } - }, [basicValue, finalWidth, animate, animationType]); + }, [basicValue, finalWidth, animate, animationType, compilerApi]); return ( @@ -108,6 +120,7 @@ describe('Test useDerivedValue changing width', () => { finalWidth: number, animate: AnimationLocation, animationType: AnimationType, + compilerApi: boolean, ) { await mockAnimationTimer(); const updatesContainerActive = await recordAnimationUpdates(); @@ -118,6 +131,7 @@ describe('Test useDerivedValue changing width', () => { animate={animate} animationType={animationType} deriveFunction={derivedFun} + compilerApi={compilerApi} />, ); const testComponent = getTestComponent(WIDTH_COMPONENT); @@ -129,68 +143,81 @@ describe('Test useDerivedValue changing width', () => { return [updates, naiveUpdates]; } - test.each([ - { - startWidth: 0, - finalWidth: 100, - animate: AnimationLocation.USE_EFFECT, - animationType: AnimationType.TIMING, - }, - { - startWidth: 0, - finalWidth: 100, - animate: AnimationLocation.ANIMATED_STYLE, - animationType: AnimationType.TIMING, - }, - { - startWidth: 0, - finalWidth: 100, - animate: AnimationLocation.ANIMATED_STYLE, - animationType: AnimationType.SPRING, - }, - { - startWidth: 0, - finalWidth: 100, - animate: AnimationLocation.USE_EFFECT, - animationType: AnimationType.SPRING, - }, - { - startWidth: 0, - finalWidth: 100, - animate: AnimationLocation.NONE, - animationType: AnimationType.NONE, - }, - { - startWidth: 100, - finalWidth: 20, - animate: AnimationLocation.USE_EFFECT, - animationType: AnimationType.TIMING, - }, - { - startWidth: 400, - finalWidth: 300, - animate: AnimationLocation.ANIMATED_STYLE, - animationType: AnimationType.TIMING, - }, - { - startWidth: 20, - finalWidth: 100, - animate: AnimationLocation.ANIMATED_STYLE, - animationType: AnimationType.SPRING, - }, - { - startWidth: 55.5, - finalWidth: 155.5, - animate: AnimationLocation.USE_EFFECT, - animationType: AnimationType.SPRING, - }, - { - startWidth: 300, - finalWidth: 33, - animate: AnimationLocation.NONE, - animationType: AnimationType.NONE, - }, - ])( + test.each( + [ + { + startWidth: 0, + finalWidth: 100, + animate: AnimationLocation.USE_EFFECT, + animationType: AnimationType.TIMING, + }, + { + startWidth: 0, + finalWidth: 100, + animate: AnimationLocation.ANIMATED_STYLE, + animationType: AnimationType.TIMING, + }, + { + startWidth: 0, + finalWidth: 100, + animate: AnimationLocation.ANIMATED_STYLE, + animationType: AnimationType.SPRING, + }, + { + startWidth: 0, + finalWidth: 100, + animate: AnimationLocation.USE_EFFECT, + animationType: AnimationType.SPRING, + }, + { + startWidth: 0, + finalWidth: 100, + animate: AnimationLocation.NONE, + animationType: AnimationType.NONE, + }, + { + startWidth: 100, + finalWidth: 20, + animate: AnimationLocation.USE_EFFECT, + animationType: AnimationType.TIMING, + }, + { + startWidth: 400, + finalWidth: 300, + animate: AnimationLocation.ANIMATED_STYLE, + animationType: AnimationType.TIMING, + }, + { + startWidth: 20, + finalWidth: 100, + animate: AnimationLocation.ANIMATED_STYLE, + animationType: AnimationType.SPRING, + }, + { + startWidth: 55.5, + finalWidth: 155.5, + animate: AnimationLocation.USE_EFFECT, + animationType: AnimationType.SPRING, + }, + { + startWidth: 300, + finalWidth: 33, + animate: AnimationLocation.NONE, + animationType: AnimationType.NONE, + }, + ].reduce( + (acc, element) => { + return [...acc, { ...element, compilerApi: false }, { ...element, compilerApi: true }]; + }, + [] as { + startWidth: number; + finalWidth: number; + animate: AnimationLocation; + animationType: AnimationType; + compilerApi: boolean; + }[], + ), + )( 'Animate from ${startWidth} to ${finalWidth}, ${animationType} ${animate}', async ({ startWidth, finalWidth, animate, animationType }) => { const snapshotIdPerType = { @@ -212,6 +239,7 @@ describe('Test useDerivedValue changing width', () => { finalWidth, animate, animationType, + false, ); expect(updates).toMatchSnapshots(snapshot[snapshotName]); diff --git a/apps/common-app/src/examples/RuntimeTests/tests/core/useSharedValue/animationsCompilerApi.test.tsx b/apps/common-app/src/examples/RuntimeTests/tests/core/useSharedValue/animationsCompilerApi.test.tsx new file mode 100644 index 00000000000..59ed30b4673 --- /dev/null +++ b/apps/common-app/src/examples/RuntimeTests/tests/core/useSharedValue/animationsCompilerApi.test.tsx @@ -0,0 +1,153 @@ +import React, { useEffect } from 'react'; +import type { SharedValue } from 'react-native-reanimated'; +import { + useSharedValue, + withClamp, + withDecay, + withDelay, + withRepeat, + withSequence, + withSpring, + withTiming, +} from 'react-native-reanimated'; +import { + describe, + test, + expect, + render, + registerValue, + getRegisteredValue, + wait, +} from '../../../ReJest/RuntimeTestsApi'; +import { ComparisonMode } from '../../../ReJest/types'; +import { ProgressBar } from './components'; + +const SHARED_VALUE_REF = 'SHARED_VALUE_REF'; + +describe(`Test animation assignments on Shared Value using compiler API`, () => { + const WithTiming = ({ progress }: { progress: number }) => { + const sharedValue = useSharedValue(0); + registerValue(SHARED_VALUE_REF, sharedValue as SharedValue); + + useEffect(() => { + sharedValue.set(withTiming(100)); + }); + return ; + }; + + const WithClamp = ({ progress }: { progress: number }) => { + const sharedValue = useSharedValue(0); + registerValue(SHARED_VALUE_REF, sharedValue as SharedValue); + + useEffect(() => { + sharedValue.set(withClamp({ min: 0, max: 100 }, withTiming(200))); + }); + return ; + }; + + const WithDecay = ({ progress }: { progress: number }) => { + const sharedValue = useSharedValue(0); + registerValue(SHARED_VALUE_REF, sharedValue as SharedValue); + + useEffect(() => { + sharedValue.set(withDecay({})); + }); + return ; + }; + + const WithDelay = ({ progress }: { progress: number }) => { + const sharedValue = useSharedValue(0); + registerValue(SHARED_VALUE_REF, sharedValue as SharedValue); + + useEffect(() => { + sharedValue.set(withDelay(100, withTiming(100))); + }); + return ; + }; + + const WithSpring = ({ progress }: { progress: number }) => { + const sharedValue = useSharedValue(0); + registerValue(SHARED_VALUE_REF, sharedValue as SharedValue); + + useEffect(() => { + sharedValue.set(withSpring(100, { duration: 250 })); + }); + return ; + }; + + const WithRepeat = ({ progress }: { progress: number }) => { + const sharedValue = useSharedValue(0); + registerValue(SHARED_VALUE_REF, sharedValue as SharedValue); + + useEffect(() => { + sharedValue.set(withRepeat(withTiming(100), 2, true)); + }); + return ; + }; + + const WithSequence = ({ progress }: { progress: number }) => { + const sharedValue = useSharedValue(0); + registerValue(SHARED_VALUE_REF, sharedValue as SharedValue); + + useEffect(() => { + sharedValue.set(withSequence(withTiming(100), withTiming(200))); + }); + return ; + }; + + test('WithTiming', async () => { + await render(); + await wait(300); + const sharedValue = await getRegisteredValue(SHARED_VALUE_REF); + expect(sharedValue.onJS).toBe(100, ComparisonMode.NUMBER); + expect(sharedValue.onUI).toBe(100, ComparisonMode.NUMBER); + }); + + test('WithClamp', async () => { + await render(); + await wait(300); + const sharedValue = await getRegisteredValue(SHARED_VALUE_REF); + expect(sharedValue.onJS).toBe(100, ComparisonMode.NUMBER); + expect(sharedValue.onUI).toBe(100, ComparisonMode.NUMBER); + }); + + test('WithDecay', async () => { + await render(); + await wait(300); + const sharedValue = await getRegisteredValue(SHARED_VALUE_REF); + expect(sharedValue.onJS).toBe(0, ComparisonMode.NUMBER); + expect(sharedValue.onUI).toBe(0, ComparisonMode.NUMBER); + }); + + test('WithDelay', async () => { + await render(); + await wait(400); + const sharedValue = await getRegisteredValue(SHARED_VALUE_REF); + expect(sharedValue.onJS).toBe(100, ComparisonMode.NUMBER); + expect(sharedValue.onUI).toBe(100, ComparisonMode.NUMBER); + }); + + test('WithSpring', async () => { + await render(); + await wait(300); + const sharedValue = await getRegisteredValue(SHARED_VALUE_REF); + expect(sharedValue.onJS).toBe(100, ComparisonMode.NUMBER); + expect(sharedValue.onUI).toBe(100, ComparisonMode.NUMBER); + }); + + test('WithRepeat', async () => { + await render(); + await wait(600); + const sharedValue = await getRegisteredValue(SHARED_VALUE_REF); + expect(sharedValue.onJS).toBe(0, ComparisonMode.NUMBER); + expect(sharedValue.onUI).toBe(0, ComparisonMode.NUMBER); + }); + + test('WithSequence', async () => { + await render(); + await wait(600); + const sharedValue = await getRegisteredValue(SHARED_VALUE_REF); + expect(sharedValue.onJS).toBe(200, ComparisonMode.NUMBER); + expect(sharedValue.onUI).toBe(200, ComparisonMode.NUMBER); + }); +}); diff --git a/packages/react-native-reanimated/src/animation/util.ts b/packages/react-native-reanimated/src/animation/util.ts index bda7c4e4e4a..73cb013e3d0 100644 --- a/packages/react-native-reanimated/src/animation/util.ts +++ b/packages/react-native-reanimated/src/animation/util.ts @@ -528,7 +528,9 @@ export function defineAnimation< if (_WORKLET || SHOULD_BE_USE_WEB) { return create(); } - // @ts-ignore: eslint-disable-line + create.__isAnimationDefinition = true; + + // @ts-expect-error it's fine return create; } diff --git a/packages/react-native-reanimated/src/mutables.ts b/packages/react-native-reanimated/src/mutables.ts index 617c1853f2f..4a05b56b103 100644 --- a/packages/react-native-reanimated/src/mutables.ts +++ b/packages/react-native-reanimated/src/mutables.ts @@ -58,10 +58,14 @@ function addCompilerSafeGetAndSet(mutable: PartialMutable): void { }, set: { value(newValue: Value | ((value: Value) => Value)) { - if (typeof newValue === 'function') { + if ( + typeof newValue === 'function' && + // If we have an animation definition, we don't want to call it here. + !(newValue as Record).__isAnimationDefinition + ) { mutable.value = (newValue as (value: Value) => Value)(mutable.value); } else { - mutable.value = newValue; + mutable.value = newValue as Value; } }, configurable: false,