Skip to content

Commit

Permalink
fix: Animation assignment with compiler-safe API (#6715)
Browse files Browse the repository at this point in the history
## 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.
  • Loading branch information
tjzel authored Nov 19, 2024
1 parent 1eed1a9 commit ef812a1
Show file tree
Hide file tree
Showing 6 changed files with 316 additions and 101 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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');
},
},
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand All @@ -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 (
<View style={styles.container}>
Expand All @@ -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<TestCase>)(
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<string, unknown>[],
) as unknown as Array<TestCase>,
)(
'${description}, from ${startWidth} to ${finalWidth}',
async ({ startWidth, finalWidth, finalWidthInPixels }: TestCase) => {
await render(<WidthComponent startWidth={startWidth} finalWidth={finalWidth} />);
async ({ startWidth, finalWidth, finalWidthInPixels, compilerApi }: TestCase) => {
await render(<WidthComponent startWidth={startWidth} finalWidth={finalWidth} compilerApi={compilerApi} />);
const componentActive = getTestComponent(WIDTH_COMPONENT_ACTIVE_REF);
const WidthComponentPassive = getTestComponent(WIDTH_COMPONENT_PASSIVE_REF);
await wait(1000);
Expand All @@ -103,7 +129,7 @@ describe('withTiming animation of WIDTH', () => {
);

test('Width from percent to pixels is NOT handled correctly', async () => {
await render(<WidthComponent startWidth={'20%'} finalWidth={100} />);
await render(<WidthComponent startWidth={'20%'} finalWidth={100} compilerApi={false} />);
const componentActive = getTestComponent(WIDTH_COMPONENT_ACTIVE_REF);
const WidthComponentPassive = getTestComponent(WIDTH_COMPONENT_PASSIVE_REF);
await wait(1000);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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 (
<View style={styles.container}>
Expand Down Expand Up @@ -108,6 +120,7 @@ describe('Test useDerivedValue changing width', () => {
finalWidth: number,
animate: AnimationLocation,
animationType: AnimationType,
compilerApi: boolean,
) {
await mockAnimationTimer();
const updatesContainerActive = await recordAnimationUpdates();
Expand All @@ -118,6 +131,7 @@ describe('Test useDerivedValue changing width', () => {
animate={animate}
animationType={animationType}
deriveFunction={derivedFun}
compilerApi={compilerApi}
/>,
);
const testComponent = getTestComponent(WIDTH_COMPONENT);
Expand All @@ -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 = {
Expand All @@ -212,6 +239,7 @@ describe('Test useDerivedValue changing width', () => {
finalWidth,
animate,
animationType,
false,
);

expect(updates).toMatchSnapshots(snapshot[snapshotName]);
Expand Down
Loading

0 comments on commit ef812a1

Please sign in to comment.