Skip to content

Commit

Permalink
feat: prevent flattening style arrays (#7021)
Browse files Browse the repository at this point in the history
<!-- Thanks for submitting a pull request! We appreciate you spending
the time to work on these changes. Please follow the template so that
the reviewers can easily understand what the code changes affect. -->

## Summary

### Why

While integrating `Unistyles` with `Reanimated`, there is one clash that
I would love to resolve.
We encourage developers to merge styles using array syntax, as it makes
it easier to detect the order of merging and persist the C++ state
attached to styles.

Reanimated flattens arrays passed to the `style` prop, making it
impossible to distinguish which props are animated and which are
provided by `Unistyles`.

### How  

To fix this, we just need to remove ~two~ one occurrence~s~ of array
flattening. I tested it locally with `Unistyles`, and I can correctly
retrieve the `style` prop as-is.

`Unistyles` have to retrieve `ref` + `style` prop while [borrowing
ref](https://www.unistyl.es/v3/other/babel-plugin#3-component-factory-borrowing-ref).

Additionally, with this change, I can treat `Reanimated` styles as
inline styles. Since inline styles are not processed by `Unistyles`
algorithm, this can completely eliminate issues such as preventing
animations when switching themes or changing device orientation.

<!-- Explain the motivation for this PR. Include "Fixes #<number>" if
applicable. -->

## Test plan

<!-- Provide a minimal but complete code snippet that can be used to
test out this change along with instructions how to run it and a
description of the expected behavior. -->

I tested it locally and I'm happy with the outcome. But I do understand
that you have own test suites and regression tests, so I hope this won't
break anything.
  • Loading branch information
jpudysz authored Feb 19, 2025
1 parent e5a0cdf commit 9b99fbd
Show file tree
Hide file tree
Showing 5 changed files with 30 additions and 23 deletions.
4 changes: 2 additions & 2 deletions packages/react-native-reanimated/__tests__/Animation.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ describe('Tests of animations', () => {
const { getByTestId } = render(<AnimatedComponent />);
const view = getByTestId('view');
const button = getByTestId('button');
expect(view.props.style.width).toBe(0);
expect(view.props.style).toEqual([getDefaultStyle(), { width: 0 }]);
expect(view).toHaveAnimatedStyle(style);
fireEvent.press(button);
jest.advanceTimersByTime(600);
Expand All @@ -92,7 +92,7 @@ describe('Tests of animations', () => {
const view = getByTestId('view');
const button = getByTestId('button');

expect(view.props.style.width).toBe(0);
expect(view.props.style).toEqual([getDefaultStyle(), { width: 0 }]);
expect(view).toHaveAnimatedStyle(style);

fireEvent.press(button);
Expand Down
26 changes: 17 additions & 9 deletions packages/react-native-reanimated/__tests__/props.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,12 @@ import type { ViewStyle } from 'react-native';
import { Pressable, Text, View } from 'react-native';

import Animated, {
getAnimatedStyle,
interpolate,
interpolateColor,
useAnimatedStyle,
useSharedValue,
} from '../src';
import { getAnimatedStyle } from '../src/jestUtils';
import { processBoxShadow } from '../src/processBoxShadow';

const AnimatedPressable = Animated.createAnimatedComponent(Pressable);
Expand Down Expand Up @@ -86,9 +86,12 @@ describe('Test of boxShadow prop', () => {
const { getByTestId } = render(<AnimatedComponent />);
const pressable = getByTestId('pressable');

expect(pressable.props.style.boxShadow).toBe(
'0px 4px 10px 0px rgba(255, 0, 0, 1)'
);
expect(pressable.props.style).toEqual([
{
boxShadow: '0px 4px 10px 0px rgba(255, 0, 0, 1)',
},
getDefaultStyle(),
]);
expect(pressable).toHaveAnimatedStyle(style);
fireEvent.press(pressable);
jest.advanceTimersByTime(600);
Expand All @@ -112,13 +115,18 @@ describe('Test of boxShadow prop', () => {
const { getByTestId } = render(<AnimatedComponent />);
const pressable = getByTestId('pressable');

expect(pressable.props.style.boxShadow).toBe(
'0px 4px 10px 0px rgba(255, 0, 0, 1)'
);
expect(pressable.props.style).toEqual([
{
boxShadow: '0px 4px 10px 0px rgba(255, 0, 0, 1)',
},
getDefaultStyle(),
]);

const unprocessedStyle = getAnimatedStyle(pressable) as ViewStyle;

processBoxShadow(pressable.props.style);
processBoxShadow(unprocessedStyle);

expect(pressable.props.style.boxShadow).toEqual([
expect(unprocessedStyle.boxShadow).toEqual([
{
offsetX: 0,
offsetY: 4,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -460,10 +460,12 @@ export default class AnimatedComponent
filteredProps.entering &&
!getReducedMotionFromConfig(filteredProps.entering as CustomConfig)
) {
filteredProps.style = {
...(filteredProps.style ?? {}),
visibility: 'hidden', // Hide component until `componentDidMount` triggers
};
filteredProps.style = Array.isArray(filteredProps.style)
? filteredProps.style.concat([{ visibility: 'hidden' }])
: {
...(filteredProps.style ?? {}),
visibility: 'hidden', // Hide component until `componentDidMount` triggers
};
}

const platformProps = Platform.select({
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
'use strict';

import { StyleSheet } from 'react-native';

import { initialUpdaterRun } from '../animation';
import type { StyleProps } from '../commonTypes';
import { isSharedValue } from '../isSharedValue';
Expand Down Expand Up @@ -53,7 +51,9 @@ export class PropsFilter implements IPropsFilter {
return style;
}
});
props[key] = StyleSheet.flatten(processedStyle);
// keep styles as they were passed by the user
// it will help other libs to interpret styles correctly
props[key] = processedStyle;
} else if (key === 'animatedProps') {
const animatedProp = inputProps.animatedProps as Partial<
AnimatedComponentProps<AnimatedProps>
Expand Down
7 changes: 2 additions & 5 deletions packages/react-native-reanimated/src/jestUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -62,8 +62,6 @@ const getCurrentStyle = (component: TestComponent): DefaultStyle => {
...style,
};
});

return currentStyle;
}

const jestInlineStyles = component.props.jestInlineStyle as JestInlineStyle;
Expand All @@ -84,7 +82,6 @@ const getCurrentStyle = (component: TestComponent): DefaultStyle => {
}

currentStyle = {
...styleObject,
...currentStyle,
...jestAnimatedStyleValue,
};
Expand All @@ -95,8 +92,8 @@ const getCurrentStyle = (component: TestComponent): DefaultStyle => {
const inlineStyles = getStylesFromObject(jestInlineStyles);

currentStyle = isEmpty(jestAnimatedStyleValue as object)
? { ...styleObject, ...inlineStyles }
: { ...styleObject, ...jestAnimatedStyleValue };
? { ...inlineStyles }
: { ...jestAnimatedStyleValue };

return currentStyle;
};
Expand Down

0 comments on commit 9b99fbd

Please sign in to comment.