Skip to content

Commit f9350ca

Browse files
authored
Fix Reanimated subscriptions when replacing a mounted gesture (#4634)
## Description Replacing one worklet gesture with another on a mounted `GestureDetector` leaves the new gesture's callbacks disconnected. `NativeEventsManager` retains the component object created on mount, but its `props` still points to the first set of worklet handlers. Later `updateEvents` calls therefore compare against stale props, and cleanup also reads that initial set. Expose the latest committed worklet props through a getter on the retained object. Update the ref before calling `updateEvents`, so Reanimated can replace subscriptions while preserving the detector and its children. Set the current props and clear the previous props when the manager starts, including when React reconnects effects after an `Activity` becomes visible. The regression uses Reanimated's real `NativeEventsManager` with mocked worklet-handler registration methods. It covers repeated replacements, handler addition/removal, unchanged handlers, child state, and cleanup on unmount. React `Activity` tests also check that revealing a detector attaches its current handler exactly once. The [standalone reproduction](https://github.com/Elehiggle/rngh-reanimated-gesture-swap-repro/tree/2c320105aab4ba5e5241fa1d2b6d460d337cd32d) switches between two worklet pans on a retained detector and includes a remount control. ## Test plan - Android 16 / API 36 x86_64 emulator, Fabric and Hermes, React Native 0.88.0, Reanimated 4.7.1, Worklets 0.13.0: the standalone example fails on v3.4.0 and main. After selecting B and dragging, both B counters remain zero; remounting enables B. With this patch, B receives begin/update events without a remount. iOS has not been tested. - On v3.4.0 (`ff34dbf`), three tests fail: replacement, addition, and duplicate registration when a previously visible `Activity` is revealed. With the fix, all five pass. Main (`a83496b`) has the same detector source. - `yarn workspace react-native-gesture-handler test --runInBand`: 238 tests pass across 24 suites. - Package `ts-check`, `lint-js`, and `circular-dependency-check` pass. Existing lint/dependency warnings remain.
1 parent 96cdbc1 commit f9350ca

2 files changed

Lines changed: 229 additions & 1 deletion

File tree

Lines changed: 220 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,220 @@
1+
import { fireEvent, render } from '@testing-library/react-native';
2+
import React, { act, Activity, useEffect, useState } from 'react';
3+
import * as ReactNative from 'react-native';
4+
import type { ReactTestRenderer } from 'react-test-renderer';
5+
import { create } from 'react-test-renderer';
6+
7+
import type { NativeEventsManager } from '../handlers/gestures/reanimatedWrapper';
8+
import type { RNGestureHandlerDetectorNativeComponentProps } from '../v3/detectors/HostGestureDetector';
9+
import { ReanimatedNativeDetector } from '../v3/detectors/ReanimatedNativeDetector';
10+
11+
// Keep Reanimated's event diffing; replace only its native registration boundary.
12+
jest.mock('react-native-reanimated/src/WorkletEventHandler', () => ({
13+
WorkletEventHandler: class {
14+
registered = new Set<number>();
15+
registerForEvents = jest.fn((tag: number) => this.registered.add(tag));
16+
unregisterFromEvents = jest.fn((tag: number) =>
17+
this.registered.delete(tag)
18+
);
19+
},
20+
}));
21+
22+
jest.mock(
23+
'react-native-reanimated/src/platform-specific/findHostInstance',
24+
() => ({
25+
findHostInstance: jest.fn(() => {
26+
throw new Error('Expected the retained detector tag');
27+
}),
28+
})
29+
);
30+
31+
jest.mock(
32+
'react-native-reanimated/src/platformFunctions/findNodeHandle',
33+
() => ({
34+
findNodeHandle: jest.fn(() => 42),
35+
})
36+
);
37+
38+
jest.mock('../handlers/gestures/reanimatedWrapper', () => ({
39+
Reanimated: jest.requireActual<{ NativeEventsManager: NativeEventsManager }>(
40+
'react-native-reanimated/src/createAnimatedComponent/NativeEventsManager'
41+
),
42+
}));
43+
44+
type RecordedHandler = {
45+
registered: Set<number>;
46+
registerForEvents: jest.Mock;
47+
unregisterFromEvents: jest.Mock;
48+
};
49+
50+
// The wrapper mock always supplies NativeEventsManager.
51+
const Detector = ReanimatedNativeDetector as NonNullable<
52+
typeof ReanimatedNativeDetector
53+
>;
54+
55+
const { WorkletEventHandler } = jest.requireMock<{
56+
WorkletEventHandler: new () => RecordedHandler;
57+
}>('react-native-reanimated/src/WorkletEventHandler');
58+
59+
type WorkletProp =
60+
RNGestureHandlerDetectorNativeComponentProps['onGestureHandlerReanimatedEvent'];
61+
62+
const eventProp = (handler: RecordedHandler) =>
63+
({ workletEventHandler: handler }) as unknown as WorkletProp;
64+
65+
describe('ReanimatedNativeDetector event registrations', () => {
66+
let findNodeHandle: jest.SpyInstance;
67+
68+
beforeEach(() => {
69+
findNodeHandle = jest
70+
.spyOn(
71+
jest.requireActual<typeof ReactNative>('react-native'),
72+
'findNodeHandle'
73+
)
74+
.mockReturnValue(42);
75+
});
76+
77+
afterEach(() => {
78+
findNodeHandle.mockRestore();
79+
});
80+
81+
test('replaces handlers while preserving the host and child state', () => {
82+
const first = new WorkletEventHandler();
83+
const second = new WorkletEventHandler();
84+
const third = new WorkletEventHandler();
85+
const mounted = jest.fn();
86+
const unmounted = jest.fn();
87+
88+
function Child() {
89+
const [count, setCount] = useState(0);
90+
useEffect(() => {
91+
mounted();
92+
return () => {
93+
unmounted();
94+
};
95+
}, []);
96+
return (
97+
<ReactNative.Pressable
98+
testID="child"
99+
onPress={() => setCount(count + 1)}>
100+
<ReactNative.Text>{count}</ReactNative.Text>
101+
</ReactNative.Pressable>
102+
);
103+
}
104+
105+
const tree = (handler: RecordedHandler) => (
106+
<Detector
107+
handlerTags={[1]}
108+
virtualChildren={[]}
109+
testID="detector"
110+
onGestureHandlerReanimatedEvent={eventProp(handler)}>
111+
<Child />
112+
</Detector>
113+
);
114+
const view = render(tree(first));
115+
const host = view.getByTestId('detector');
116+
fireEvent.press(view.getByTestId('child'));
117+
expect([...first.registered]).toEqual([42]);
118+
119+
for (const handler of [second, first, second, third]) {
120+
view.rerender(tree(handler));
121+
for (const candidate of [first, second, third]) {
122+
expect([...candidate.registered]).toEqual(
123+
candidate === handler ? [42] : []
124+
);
125+
}
126+
expect(view.getByTestId('detector')).toBe(host);
127+
expect(view.getByText('1')).toBeDefined();
128+
expect(mounted).toHaveBeenCalledTimes(1);
129+
expect(unmounted).not.toHaveBeenCalled();
130+
}
131+
132+
view.unmount();
133+
expect([...third.registered]).toEqual([]);
134+
expect(third.unregisterFromEvents).toHaveBeenCalledTimes(1);
135+
expect(unmounted).toHaveBeenCalledTimes(1);
136+
});
137+
138+
test('adds and removes a handler on the retained host', () => {
139+
const handler = new WorkletEventHandler();
140+
const view = render(<Detector handlerTags={[1]} virtualChildren={[]} />);
141+
view.rerender(
142+
<Detector
143+
handlerTags={[1]}
144+
virtualChildren={[]}
145+
onGestureHandlerReanimatedEvent={eventProp(handler)}
146+
/>
147+
);
148+
expect([...handler.registered]).toEqual([42]);
149+
view.rerender(<Detector handlerTags={[1]} virtualChildren={[]} />);
150+
expect([...handler.registered]).toEqual([]);
151+
expect(handler.unregisterFromEvents).toHaveBeenCalledTimes(1);
152+
view.unmount();
153+
expect(handler.unregisterFromEvents).toHaveBeenCalledTimes(1);
154+
});
155+
156+
test('keeps an unchanged handler registered until unmount', () => {
157+
const handler = new WorkletEventHandler();
158+
const tree = (label: string) => (
159+
<Detector
160+
handlerTags={[1]}
161+
virtualChildren={[]}
162+
accessibilityLabel={label}
163+
onGestureHandlerReanimatedEvent={eventProp(handler)}
164+
/>
165+
);
166+
const view = render(tree('first'));
167+
view.rerender(tree('second'));
168+
expect(handler.registerForEvents).toHaveBeenCalledTimes(1);
169+
expect(handler.unregisterFromEvents).not.toHaveBeenCalled();
170+
view.unmount();
171+
expect(handler.unregisterFromEvents).toHaveBeenCalledTimes(1);
172+
expect([...handler.registered]).toEqual([]);
173+
});
174+
175+
test.each(['hidden', 'visible'] as const)(
176+
'attaches the current handler once when an initially %s Activity is revealed',
177+
async (initialMode) => {
178+
const first = new WorkletEventHandler();
179+
const second = new WorkletEventHandler();
180+
const tree = (mode: 'hidden' | 'visible', handler: RecordedHandler) => (
181+
<Activity mode={mode}>
182+
<Detector
183+
handlerTags={[1]}
184+
virtualChildren={[]}
185+
onGestureHandlerReanimatedEvent={eventProp(handler)}
186+
/>
187+
</Activity>
188+
);
189+
const options = {
190+
createNodeMock: () => null,
191+
unstable_isConcurrent: true,
192+
};
193+
let renderer!: ReactTestRenderer;
194+
await act(() => {
195+
renderer = create(tree(initialMode, first), options);
196+
return Promise.resolve();
197+
});
198+
expect(first.registerForEvents).toHaveBeenCalledTimes(
199+
initialMode === 'visible' ? 1 : 0
200+
);
201+
202+
await act(() => Promise.resolve(renderer.update(tree('hidden', first))));
203+
expect([...first.registered]).toEqual([]);
204+
await act(() => Promise.resolve(renderer.update(tree('hidden', second))));
205+
expect(second.registerForEvents).not.toHaveBeenCalled();
206+
207+
await act(() =>
208+
Promise.resolve(renderer.update(tree('visible', second)))
209+
);
210+
expect([...first.registered]).toEqual([]);
211+
expect([...second.registered]).toEqual([42]);
212+
expect(second.registerForEvents).toHaveBeenCalledTimes(1);
213+
214+
await act(() => Promise.resolve(renderer.unmount()));
215+
expect([...first.registered]).toEqual([]);
216+
expect([...second.registered]).toEqual([]);
217+
expect(second.unregisterFromEvents).toHaveBeenCalledTimes(1);
218+
}
219+
);
220+
});

‎packages/react-native-gesture-handler/src/v3/detectors/ReanimatedNativeDetector.tsx‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,14 +60,21 @@ function LeanReanimatedNativeDetector(
6060
]
6161
);
6262

63+
const committedProps = useRef(reaProps);
64+
6365
useEffect(() => {
66+
committedProps.current = reaProps;
67+
prevProps.current = null;
6468
const nativeTag = findNodeHandle(viewRef.current) ?? -1;
6569
// @ts-expect-error Reanimated expects __nativeTag to be present on the ref
6670
viewRef.__nativeTag = nativeTag;
6771
// @ts-expect-error NativeEventsManager should be defined here, if it isn't, we should
6872
// go the fallback way and use Reanimated's createAnimatedComponent
6973
eventManager.current = new NativeEventsManagerImpl({
70-
props: reaProps,
74+
// NativeEventsManager reads this object again when handlers change.
75+
get props() {
76+
return committedProps.current;
77+
},
7178
_componentRef: viewRef,
7279
_componentViewTag: nativeTag,
7380
getComponentViewTag: () => nativeTag,
@@ -80,6 +87,7 @@ function LeanReanimatedNativeDetector(
8087
}, []);
8188

8289
useEffect(() => {
90+
committedProps.current = reaProps;
8391
if (prevProps.current) {
8492
eventManager.current?.updateEvents(prevProps.current);
8593
}

0 commit comments

Comments
 (0)