Skip to content

Commit 5c8c538

Browse files
yungstersfacebook-github-bot
authored andcommitted
RN: Fix Shadowing of Animated Styles (#51719)
Summary: Pull Request resolved: #51719 When `Animated` traverses props for instances of `AnimatedNode`, it flattens `props.style` before traversing it so that we correctly ignore any `AnimatedNode` instances that may be shadowed by static values: ``` { style: [ {transform: [{translateX: new Animated.Value(0)}]}, {transform: [{translateX: 100}]}, ], } ``` However, there is a bug that occurs when *every* `AnimatedNode` instance is shadowed. In this case, `AnimatedProps` assumes that there are *no* `AnimatedNode` instances in the entire `props.style`. It then incorrectly operates on the unflattened `props.style`, which *does* have `AnimatedNode` instances. When this is passed to `View`, the `AnimatedNode` instances are encountered and can cause a crash in `processTransform`, as reported by: #51395 The fix for this was originally attempted by riteshshukla04 in #51442. This diff reuses the same unit test case, but it applies a different fix that does not involve re-traversing the `props.style` object. The fix is gated behind a feature flag, `alwaysFlattenAnimatedStyles`. This will enable us to validate correctness of the new behavior before enabling it for everyone. (Beyond fixing the bug described above, this also causes styles to flatten more aggressively, so production testing is important to ensure stability.) Changelog: [General][Changed] - Creates a feature flag that changes Animated to no longer produce invalid `props.style` if every `AnimatedNode` instance is shadowed via style flattening. Reviewed By: javache Differential Revision: D75723284 fbshipit-source-id: 504f63e8edf836243d615783e119137a920ad271
1 parent 3353579 commit 5c8c538

File tree

6 files changed

+116
-43
lines changed

6 files changed

+116
-43
lines changed

packages/react-native/Libraries/Animated/__tests__/Animated-test.js

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -245,6 +245,27 @@ describe('Animated', () => {
245245
expect(callback).toBeCalled();
246246
});
247247

248+
it('renders animated and primitive style correctly', () => {
249+
ReactNativeFeatureFlags.override({
250+
alwaysFlattenAnimatedStyles: () => true,
251+
});
252+
253+
const anim = new Animated.Value(0);
254+
const staticProps = {
255+
style: [
256+
{transform: [{translateX: anim}]},
257+
{transform: [{translateX: 100}]},
258+
],
259+
};
260+
const staticPropsWithoutAnim = {
261+
style: {transform: [{translateX: 100}]},
262+
};
263+
const node = new AnimatedProps(staticProps, jest.fn());
264+
expect(node.__getValueWithStaticProps(staticProps)).toStrictEqual(
265+
staticPropsWithoutAnim,
266+
);
267+
});
268+
248269
it('send toValue when a critically damped spring stops', () => {
249270
const anim = new Animated.Value(0);
250271
const listener = jest.fn();

packages/react-native/Libraries/Animated/nodes/AnimatedProps.js

Lines changed: 36 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,9 @@ import type {AnimatedNodeConfig} from './AnimatedNode';
1313
import type {AnimatedStyleAllowlist} from './AnimatedStyle';
1414

1515
import NativeAnimatedHelper from '../../../src/private/animated/NativeAnimatedHelper';
16+
import * as ReactNativeFeatureFlags from '../../../src/private/featureflags/ReactNativeFeatureFlags';
1617
import {findNodeHandle} from '../../ReactNative/RendererProxy';
18+
import flattenStyle from '../../StyleSheet/flattenStyle';
1719
import {AnimatedEvent} from '../AnimatedEvent';
1820
import AnimatedNode from './AnimatedNode';
1921
import AnimatedObject from './AnimatedObject';
@@ -43,18 +45,30 @@ function createAnimatedProps(
4345
for (let ii = 0, length = keys.length; ii < length; ii++) {
4446
const key = keys[ii];
4547
const value = inputProps[key];
48+
let staticValue = value;
4649

4750
if (allowlist == null || hasOwn(allowlist, key)) {
4851
let node;
4952
if (key === 'style') {
50-
node = AnimatedStyle.from(value, allowlist?.style);
53+
// Ignore `style` if it is not an object (or array).
54+
if (typeof value === 'object' && value != null) {
55+
// Even if we do not find any `AnimatedNode` values in `style`, we
56+
// still need to use the flattened `style` object because static
57+
// values can shadow `AnimatedNode` values. We need to make sure that
58+
// we propagate the flattened `style` object to the `props` object.
59+
const flatStyle = flattenStyle(value as $FlowFixMe);
60+
node = AnimatedStyle.from(flatStyle, allowlist?.style, value);
61+
if (ReactNativeFeatureFlags.alwaysFlattenAnimatedStyles()) {
62+
staticValue = flatStyle;
63+
}
64+
}
5165
} else if (value instanceof AnimatedNode) {
5266
node = value;
5367
} else {
5468
node = AnimatedObject.from(value);
5569
}
5670
if (node == null) {
57-
props[key] = value;
71+
props[key] = staticValue;
5872
} else {
5973
nodeKeys.push(key);
6074
nodes.push(node);
@@ -134,8 +148,26 @@ export default class AnimatedProps extends AnimatedNode {
134148
const key = keys[ii];
135149
const maybeNode = this.#props[key];
136150

137-
if (key === 'style' && maybeNode instanceof AnimatedStyle) {
138-
props[key] = maybeNode.__getValueWithStaticStyle(staticProps.style);
151+
if (key === 'style') {
152+
const staticStyle = staticProps.style;
153+
const flatStaticStyle = flattenStyle(staticStyle);
154+
if (maybeNode instanceof AnimatedStyle) {
155+
const mutableStyle: {[string]: mixed} =
156+
flatStaticStyle == null
157+
? {}
158+
: flatStaticStyle === staticStyle
159+
? // Copy the input style, since we'll mutate it below.
160+
{...flatStaticStyle}
161+
: // Reuse `flatStaticStyle` if it is a newly created object.
162+
flatStaticStyle;
163+
164+
maybeNode.__replaceAnimatedNodeWithValues(mutableStyle);
165+
props[key] = maybeNode.__getValueForStyle(mutableStyle);
166+
} else {
167+
if (ReactNativeFeatureFlags.alwaysFlattenAnimatedStyles()) {
168+
props[key] = flatStaticStyle;
169+
}
170+
}
139171
} else if (maybeNode instanceof AnimatedNode) {
140172
props[key] = maybeNode.__getValue();
141173
} else if (maybeNode instanceof AnimatedEvent) {

packages/react-native/Libraries/Animated/nodes/AnimatedStyle.js

Lines changed: 35 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@ import type {AnimatedNodeConfig} from './AnimatedNode';
1313

1414
import {validateStyles} from '../../../src/private/animated/NativeAnimatedValidation';
1515
import * as ReactNativeFeatureFlags from '../../../src/private/featureflags/ReactNativeFeatureFlags';
16-
import flattenStyle from '../../StyleSheet/flattenStyle';
1716
import Platform from '../../Utilities/Platform';
1817
import AnimatedNode from './AnimatedNode';
1918
import AnimatedObject from './AnimatedObject';
@@ -22,19 +21,22 @@ import AnimatedWithChildren from './AnimatedWithChildren';
2221

2322
export type AnimatedStyleAllowlist = $ReadOnly<{[string]: true}>;
2423

24+
type FlatStyle = {[string]: mixed};
25+
type FlatStyleForWeb<TStyle: FlatStyle> = [mixed, TStyle];
26+
2527
function createAnimatedStyle(
26-
inputStyle: {[string]: mixed},
28+
flatStyle: FlatStyle,
2729
allowlist: ?AnimatedStyleAllowlist,
2830
keepUnanimatedValues: boolean,
2931
): [$ReadOnlyArray<string>, $ReadOnlyArray<AnimatedNode>, {[string]: mixed}] {
3032
const nodeKeys: Array<string> = [];
3133
const nodes: Array<AnimatedNode> = [];
3234
const style: {[string]: mixed} = {};
3335

34-
const keys = Object.keys(inputStyle);
36+
const keys = Object.keys(flatStyle);
3537
for (let ii = 0, length = keys.length; ii < length; ii++) {
3638
const key = keys[ii];
37-
const value = inputStyle[key];
39+
const value = flatStyle[key];
3840

3941
if (allowlist == null || hasOwn(allowlist, key)) {
4042
let node;
@@ -62,10 +64,10 @@ function createAnimatedStyle(
6264
// WARNING: This is a potentially expensive check that we should only
6365
// do in development. Without this check in development, it might be
6466
// difficult to identify which styles need to be allowlisted.
65-
if (AnimatedObject.from(inputStyle[key]) != null) {
67+
if (AnimatedObject.from(flatStyle[key]) != null) {
6668
console.error(
67-
`AnimatedStyle: ${key} is not allowlisted for animation, but it ` +
68-
'contains AnimatedNode values; styles allowing animation: ',
69+
`AnimatedStyle: ${key} is not allowlisted for animation, but ` +
70+
'it contains AnimatedNode values; styles allowing animation: ',
6971
allowlist,
7072
);
7173
}
@@ -80,7 +82,7 @@ function createAnimatedStyle(
8082
}
8183

8284
export default class AnimatedStyle extends AnimatedWithChildren {
83-
#inputStyle: any;
85+
#originalStyleForWeb: ?mixed;
8486
#nodeKeys: $ReadOnlyArray<string>;
8587
#nodes: $ReadOnlyArray<AnimatedNode>;
8688
#style: {[string]: mixed};
@@ -90,10 +92,10 @@ export default class AnimatedStyle extends AnimatedWithChildren {
9092
* Otherwise, returns `null`.
9193
*/
9294
static from(
93-
inputStyle: any,
95+
flatStyle: ?FlatStyle,
9496
allowlist: ?AnimatedStyleAllowlist,
97+
originalStyleForWeb: ?mixed,
9598
): ?AnimatedStyle {
96-
const flatStyle = flattenStyle(inputStyle);
9799
if (flatStyle == null) {
98100
return null;
99101
}
@@ -105,24 +107,31 @@ export default class AnimatedStyle extends AnimatedWithChildren {
105107
if (nodes.length === 0) {
106108
return null;
107109
}
108-
return new AnimatedStyle(nodeKeys, nodes, style, inputStyle);
110+
return new AnimatedStyle(nodeKeys, nodes, style, originalStyleForWeb);
109111
}
110112

111113
constructor(
112114
nodeKeys: $ReadOnlyArray<string>,
113115
nodes: $ReadOnlyArray<AnimatedNode>,
114116
style: {[string]: mixed},
115-
inputStyle: any,
117+
originalStyleForWeb: ?mixed,
116118
config?: ?AnimatedNodeConfig,
117119
) {
118120
super(config);
119121
this.#nodeKeys = nodeKeys;
120122
this.#nodes = nodes;
121123
this.#style = style;
122-
this.#inputStyle = inputStyle;
124+
125+
if ((Platform.OS as string) === 'web') {
126+
// $FlowIgnore[cannot-write] - Intentional shadowing.
127+
this.__getValueForStyle = resultStyle => [
128+
originalStyleForWeb,
129+
resultStyle,
130+
];
131+
}
123132
}
124133

125-
__getValue(): Object | Array<Object> {
134+
__getValue(): FlatStyleForWeb<FlatStyle> | FlatStyle {
126135
const style: {[string]: mixed} = {};
127136

128137
const keys = Object.keys(this.#style);
@@ -137,27 +146,23 @@ export default class AnimatedStyle extends AnimatedWithChildren {
137146
}
138147
}
139148

140-
/* $FlowFixMe[incompatible-type] Error found due to incomplete typing of
141-
* Platform.flow.js */
142-
return Platform.OS === 'web' ? [this.#inputStyle, style] : style;
149+
return this.__getValueForStyle(style);
143150
}
144151

145152
/**
146-
* Creates a new `style` object that contains the same style properties as
147-
* the supplied `staticStyle` object, except with animated nodes for any
148-
* style properties that were created by this `AnimatedStyle` instance.
153+
* See the constructor, where this is shadowed on web platforms.
149154
*/
150-
__getValueWithStaticStyle(staticStyle: Object): Object | Array<Object> {
151-
const flatStaticStyle = flattenStyle(staticStyle);
152-
const style: {[string]: mixed} =
153-
flatStaticStyle == null
154-
? {}
155-
: flatStaticStyle === staticStyle
156-
? // Copy the input style, since we'll mutate it below.
157-
{...flatStaticStyle}
158-
: // Reuse `flatStaticStyle` if it is a newly created object.
159-
flatStaticStyle;
155+
__getValueForStyle<TStyle: FlatStyle>(
156+
style: TStyle,
157+
): FlatStyleForWeb<TStyle> | TStyle {
158+
return style;
159+
}
160160

161+
/**
162+
* Mutates the supplied `style` object such that animated nodes are replaced
163+
* with rasterized values.
164+
*/
165+
__replaceAnimatedNodeWithValues(style: {[string]: mixed}): void {
161166
const keys = Object.keys(style);
162167
for (let ii = 0, length = keys.length; ii < length; ii++) {
163168
const key = keys[ii];
@@ -175,10 +180,6 @@ export default class AnimatedStyle extends AnimatedWithChildren {
175180
style[key] = maybeNode.__getValue();
176181
}
177182
}
178-
179-
/* $FlowFixMe[incompatible-type] Error found due to incomplete typing of
180-
* Platform.flow.js */
181-
return Platform.OS === 'web' ? [this.#inputStyle, style] : style;
182183
}
183184

184185
__getAnimatedValue(): Object {

packages/react-native/Libraries/__tests__/__snapshots__/public-api-test.js.snap

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -925,16 +925,19 @@ declare export default class AnimatedProps extends AnimatedNode {
925925

926926
exports[`public API should not change unintentionally Libraries/Animated/nodes/AnimatedStyle.js 1`] = `
927927
"export type AnimatedStyleAllowlist = $ReadOnly<{ [string]: true }>;
928+
type FlatStyle = { [string]: mixed };
929+
type FlatStyleForWeb<TStyle: FlatStyle> = [mixed, TStyle];
928930
declare export default class AnimatedStyle extends AnimatedWithChildren {
929931
static from(
930-
inputStyle: any,
931-
allowlist: ?AnimatedStyleAllowlist
932+
flatStyle: ?FlatStyle,
933+
allowlist: ?AnimatedStyleAllowlist,
934+
originalStyleForWeb: ?mixed
932935
): ?AnimatedStyle;
933936
constructor(
934937
nodeKeys: $ReadOnlyArray<string>,
935938
nodes: $ReadOnlyArray<AnimatedNode>,
936939
style: { [string]: mixed },
937-
inputStyle: any,
940+
originalStyleForWeb: ?mixed,
938941
config?: ?AnimatedNodeConfig
939942
): void;
940943
}

packages/react-native/scripts/featureflags/ReactNativeFeatureFlags.config.js

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -621,7 +621,17 @@ const definitions: FeatureFlagDefinitions = {
621621

622622
jsOnly: {
623623
...testDefinitions.jsOnly,
624-
624+
alwaysFlattenAnimatedStyles: {
625+
defaultValue: false,
626+
metadata: {
627+
dateAdded: '2025-06-02',
628+
description:
629+
'Changes `Animated` to always flatten style, fixing a bug with shadowed `AnimatedNode` instances.',
630+
expectedReleaseValue: true,
631+
purpose: 'experimentation',
632+
},
633+
ossReleaseStage: 'none',
634+
},
625635
animatedShouldDebounceQueueFlush: {
626636
defaultValue: false,
627637
metadata: {

packages/react-native/src/private/featureflags/ReactNativeFeatureFlags.js

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
* This source code is licensed under the MIT license found in the
55
* LICENSE file in the root directory of this source tree.
66
*
7-
* @generated SignedSource<<7fe2abc6b638728b09b0194988f0264c>>
7+
* @generated SignedSource<<a4b4716b9b0e9f84c38d83c991ecfba7>>
88
* @flow strict
99
* @noformat
1010
*/
@@ -29,6 +29,7 @@ import {
2929

3030
export type ReactNativeFeatureFlagsJsOnly = $ReadOnly<{
3131
jsOnlyTestFlag: Getter<boolean>,
32+
alwaysFlattenAnimatedStyles: Getter<boolean>,
3233
animatedShouldDebounceQueueFlush: Getter<boolean>,
3334
animatedShouldUseSingleOp: Getter<boolean>,
3435
avoidStateUpdateInAnimatedPropsMemo: Getter<boolean>,
@@ -111,6 +112,11 @@ export type ReactNativeFeatureFlags = $ReadOnly<{
111112
*/
112113
export const jsOnlyTestFlag: Getter<boolean> = createJavaScriptFlagGetter('jsOnlyTestFlag', false);
113114

115+
/**
116+
* Changes `Animated` to always flatten style, fixing a bug with shadowed `AnimatedNode` instances.
117+
*/
118+
export const alwaysFlattenAnimatedStyles: Getter<boolean> = createJavaScriptFlagGetter('alwaysFlattenAnimatedStyles', false);
119+
114120
/**
115121
* Enables an experimental flush-queue debouncing in Animated.js.
116122
*/

0 commit comments

Comments
 (0)