Skip to content

Commit 856661f

Browse files
authored
fix: disconnect AnimatedProps before attaching a new native view
## Summary: When an animated component replaces its underlying native view, `AnimatedProps.setNativeView()` connects the existing animated props node to the replacement without disconnecting the previous view. On Android, this causes a crash: ```text Animated node X is already attached to a view: Y ``` Disconnect the previous target before connecting the replacement. ## Changelog: [ANDROID] [FIXED] - Prevent native Animated crashes when an animated component replaces its underlying native view. ## Test Plan: You can use this code to reproduce a crash: ```tsx import React, { forwardRef, useState } from "react"; import { Animated, Button, StyleSheet, Text, View } from "react-native"; import type { ViewProps } from "react-native"; // Keep the Animated wrapper mounted while replacing its native View. const ReplacingView = Animated.createAnimatedComponent( forwardRef<View, ViewProps & { generation: number }>(function ReplacingView( { generation, ...props }, ref, ) { return <View {...props} key={generation} ref={ref} />; }), ); const styles = StyleSheet.create({ screen: { flex: 1, padding: 24, paddingTop: 64, backgroundColor: "white" }, }); export default function Issue1604() { const [generation, setGeneration] = useState(0); const [opacity] = useState( () => new Animated.Value(1, { useNativeDriver: true }), ); const replaceView = () => setGeneration((value) => value + 1); return ( <View style={styles.screen}> <Text>Native Animated crash repro</Text> <Button title="Replace view" onPress={replaceView} /> <ReplacingView generation={generation} style={{ opacity }}> <Text testID="issue1604-generation">View {generation}</Text> </ReplacingView> </View> ); } ``` |Before|After| |-------|-----| |<video src="https://github.com/user-attachments/assets/40f5b75f-e250-4c06-b00b-7084a2011064">|<video src="https://github.com/user-attachments/assets/a2524013-3923-4b8f-ac01-d0ede0f8edc2">| Differential Revision: D128344147 Pull Request resolved: #59099
1 parent a9790c7 commit 856661f

2 files changed

Lines changed: 96 additions & 0 deletions

File tree

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -268,6 +268,9 @@ export default class AnimatedProps extends AnimatedNode {
268268
if (this._target?.instance === instance) {
269269
return;
270270
}
271+
if (this.__isNative && this._target != null) {
272+
this.#disconnectAnimatedView(this._target);
273+
}
271274
const target = (this._target = {instance, connectedViewTag: null});
272275
if (this.__isNative) {
273276
this.#connectAnimatedView(target);

‎packages/react-native/src/private/animated/__tests__/AnimatedNative-test.js‎

Lines changed: 93 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,11 @@ describe('Native Animated', () => {
3131
};
3232
}
3333

34+
function createViewInstance(): React.ElementRef<React.ElementType> {
35+
// $FlowFixMe[incompatible-type]
36+
return {};
37+
}
38+
3439
beforeEach(() => {
3540
jest.resetModules();
3641
jest.restoreAllMocks();
@@ -1620,6 +1625,94 @@ describe('Native Animated', () => {
16201625
);
16211626
});
16221627

1628+
it('disconnects the previous native view before connecting a replacement', () => {
1629+
const findNodeHandle = jest
1630+
.fn()
1631+
.mockReturnValueOnce(11)
1632+
.mockReturnValueOnce(22);
1633+
jest.setMock('../../../../Libraries/ReactNative/RendererProxy', {
1634+
findNodeHandle,
1635+
});
1636+
const {Animated} = importModules();
1637+
const AnimatedProps =
1638+
require('../../../../Libraries/Animated/nodes/AnimatedProps').default;
1639+
const firstView = createViewInstance();
1640+
const secondView = createViewInstance();
1641+
const opacity = new Animated.Value(1);
1642+
const propsNode = new AnimatedProps({style: {opacity}}, jest.fn());
1643+
propsNode.__attach();
1644+
propsNode.setNativeView(firstView);
1645+
propsNode.__makeNative();
1646+
const propsTag = propsNode.__getNativeTag();
1647+
1648+
propsNode.setNativeView(secondView);
1649+
1650+
expect(
1651+
NativeAnimatedModule.connectAnimatedNodeToView,
1652+
).toHaveBeenNthCalledWith(1, propsTag, 11);
1653+
expect(
1654+
NativeAnimatedModule.disconnectAnimatedNodeFromView,
1655+
).toHaveBeenCalledWith(propsTag, 11);
1656+
expect(
1657+
NativeAnimatedModule.connectAnimatedNodeToView,
1658+
).toHaveBeenNthCalledWith(2, propsTag, 22);
1659+
expect(
1660+
// $FlowFixMe[prop-missing]
1661+
NativeAnimatedModule.disconnectAnimatedNodeFromView.mock
1662+
.invocationCallOrder[0],
1663+
).toBeLessThan(
1664+
// $FlowFixMe[prop-missing]
1665+
NativeAnimatedModule.connectAnimatedNodeToView.mock
1666+
.invocationCallOrder[1],
1667+
);
1668+
1669+
propsNode.setNativeView(secondView);
1670+
expect(
1671+
NativeAnimatedModule.connectAnimatedNodeToView,
1672+
).toHaveBeenCalledTimes(2);
1673+
expect(
1674+
NativeAnimatedModule.disconnectAnimatedNodeFromView,
1675+
).toHaveBeenCalledTimes(1);
1676+
1677+
propsNode.__detach();
1678+
expect(
1679+
NativeAnimatedModule.disconnectAnimatedNodeFromView,
1680+
).toHaveBeenNthCalledWith(2, propsTag, 22);
1681+
expect(findNodeHandle).toHaveBeenCalledTimes(2);
1682+
});
1683+
1684+
it('only connects the latest view when switching to the native driver', () => {
1685+
jest.setMock('../../../../Libraries/ReactNative/RendererProxy', {
1686+
findNodeHandle: () => 22,
1687+
});
1688+
const {Animated} = importModules();
1689+
const AnimatedProps =
1690+
require('../../../../Libraries/Animated/nodes/AnimatedProps').default;
1691+
const opacity = new Animated.Value(1);
1692+
const propsNode = new AnimatedProps({style: {opacity}}, jest.fn());
1693+
propsNode.__attach();
1694+
propsNode.setNativeView(createViewInstance());
1695+
propsNode.setNativeView(createViewInstance());
1696+
expect(
1697+
NativeAnimatedModule.connectAnimatedNodeToView,
1698+
).not.toHaveBeenCalled();
1699+
expect(
1700+
NativeAnimatedModule.disconnectAnimatedNodeFromView,
1701+
).not.toHaveBeenCalled();
1702+
1703+
propsNode.__makeNative();
1704+
expect(
1705+
NativeAnimatedModule.connectAnimatedNodeToView,
1706+
).toHaveBeenCalledTimes(1);
1707+
expect(
1708+
NativeAnimatedModule.connectAnimatedNodeToView,
1709+
).toHaveBeenCalledWith(propsNode.__getNativeTag(), 22);
1710+
expect(
1711+
NativeAnimatedModule.disconnectAnimatedNodeFromView,
1712+
).not.toHaveBeenCalled();
1713+
propsNode.__detach();
1714+
});
1715+
16231716
it('creates new props, style and transform nodes at update', async () => {
16241717
const {Animated} = importModules();
16251718

0 commit comments

Comments
 (0)