Skip to content

Commit bbb49c3

Browse files
authored
Pass empty callbacks to UI when runOnJS is true (#4326)
## Description Currently we call [`useReanimatedEventHandler`](https://github.com/software-mansion/react-native-gesture-handler/blob/f922b3b23c23ae9e5d77e6d716f7d6f8c02fe9b5/packages/react-native-gesture-handler/src/v3/hooks/useGestureCallbacks.ts#L57) with original callbacks from config. However, objects captured in the callbacks' closures will be frozen by Reanimated as they will be passed to UI, even if `runOnJS` is set to `true`. This triggers warning from worklets, as well as prevents modification of objects in closure. ``` [Worklets] Tried to modify key `current` of an object which has been already passed to a worklet. ``` To fix that, we pass empty callbacks when `runOnJS` is `true`. ## Test plan <details> <summary>Tested on the following code:</summary> ```tsx import React, { useCallback, useMemo, useState } from 'react'; import { Pressable, StyleSheet, Text, View } from 'react-native'; import { GestureDetector, usePanGesture } from 'react-native-gesture-handler'; import { useSharedValue } from 'react-native-reanimated'; import { scheduleOnRN } from 'react-native-worklets'; // Empirical test matrix for the `runOnJS` noop-substitution fix. // // Pad A - static `runOnJS: true`. Callbacks mutate a plain `session` object; // before the fix dev builds froze it ("0 updates in ~1.7T ms" + warnings). // Expected: real numbers, no `Tried to modify key` warnings. // // Pad B - `runOnJS` driven by React state, toggled by the button below it. // Callbacks report which runtime executed them. Expected: "JS runtime" // while ON, then after toggling OFF the real callbacks must be re-registered // on the UI runtime - a drag must report "UI runtime". If the lazy swap // were broken, the UI path would fire the registered noops and the status // would never update. // // Pad C - `runOnJS` as a SharedValue, flipped by writing `sv.value` directly // (no re-render). Expected: "JS runtime" while true, "UI runtime" after the // flip - proving the SharedValue path still eagerly registers the real // callbacks and toggles without React involvement. function runtimeName() { 'worklet'; return (globalThis as { _WORKLET?: boolean })._WORKLET ? 'UI runtime' : 'JS runtime'; } export default function EmptyExample() { // --- Pad A: static runOnJS: true, plain mutable object --- const [summaryA, setSummaryA] = useState('drag to measure'); const session = useMemo(() => ({ startTime: 0, updateCount: 0 }), []); const panA = usePanGesture({ onActivate: () => { session.startTime = Date.now(); session.updateCount = 0; }, onUpdate: () => { session.updateCount += 1; }, onDeactivate: () => { const duration = Date.now() - session.startTime; setSummaryA(`${session.updateCount} updates in ${duration} ms`); }, runOnJS: true, }); // --- Pad B: runOnJS toggled through React state --- const [jsModeB, setJsModeB] = useState(true); const [statusB, setStatusB] = useState('drag to check'); const reportB = useCallback((runtime: string) => { setStatusB(`activated on ${runtime}`); }, []); const panB = usePanGesture({ onActivate: () => { scheduleOnRN(reportB, runtimeName()); }, runOnJS: jsModeB, }); // --- Pad C: runOnJS as a SharedValue, flipped without a re-render --- const jsModeC = useSharedValue(true); const [statusC, setStatusC] = useState('drag to check'); const reportC = useCallback((runtime: string) => { setStatusC(`activated on ${runtime}`); }, []); const panC = usePanGesture({ onActivate: () => { scheduleOnRN(reportC, runtimeName()); }, runOnJS: jsModeC, }); return ( <View style={styles.container}> <GestureDetector gesture={panA}> <View style={styles.pad}> <Text style={styles.padLabel}>Pad A - static true</Text> </View> </GestureDetector> <Text style={styles.status}>{summaryA}</Text> <GestureDetector gesture={panB}> <View style={styles.pad}> <Text style={styles.padLabel}>Pad B - state toggle</Text> </View> </GestureDetector> <Text style={styles.status}>{statusB}</Text> <Pressable style={styles.button} onPress={() => setJsModeB((prev) => !prev)}> <Text style={styles.buttonLabel}> {`B: runOnJS is ${jsModeB ? 'ON' : 'OFF'} - toggle`} </Text> </Pressable> <GestureDetector gesture={panC}> <View style={styles.pad}> <Text style={styles.padLabel}>Pad C - SharedValue toggle</Text> </View> </GestureDetector> <Text style={styles.status}>{statusC}</Text> <Pressable style={styles.button} onPress={() => { jsModeC.value = !jsModeC.value; }}> <Text style={styles.buttonLabel}> C: flip SharedValue (no rerender) </Text> </Pressable> </View> ); } const styles = StyleSheet.create({ container: { flex: 1, justifyContent: 'center', padding: 20, }, pad: { alignSelf: 'stretch', height: 110, borderRadius: 12, backgroundColor: '#eef', borderWidth: StyleSheet.hairlineWidth, borderColor: '#99a', justifyContent: 'center', alignItems: 'center', }, padLabel: { color: '#667', fontSize: 15, }, status: { marginTop: 6, marginBottom: 12, textAlign: 'center', color: '#666', }, button: { alignSelf: 'center', paddingHorizontal: 16, paddingVertical: 8, borderRadius: 8, backgroundColor: '#dde', marginBottom: 16, }, buttonLabel: { color: '#334', }, }); ``` </details>
1 parent ed6e159 commit bbb49c3

2 files changed

Lines changed: 137 additions & 2 deletions

File tree

Lines changed: 123 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,123 @@
1+
import { renderHook } from '@testing-library/react-native';
2+
3+
import { useTapGesture } from '../v3/hooks/gestures';
4+
import type { SharedValue } from '../v3/types';
5+
6+
jest.mock('../handlers/gestures/reanimatedWrapper', () => {
7+
const Reanimated = {
8+
useHandler: jest.fn(() => ({
9+
doDependenciesDiffer: false,
10+
context: { lastUpdateEvent: undefined },
11+
})),
12+
useEvent: jest.fn(() => jest.fn()),
13+
isSharedValue: (value: unknown): boolean =>
14+
value !== null && typeof value === 'object' && 'value' in value,
15+
isWorkletFunction: (value: unknown): boolean =>
16+
typeof value === 'function' && '__workletHash' in value,
17+
makeMutable: <T,>(value: T) => ({ value }),
18+
runOnUI: () => jest.fn(),
19+
runOnJS:
20+
(fn: (...args: unknown[]) => unknown) =>
21+
(...args: unknown[]) =>
22+
fn(...args),
23+
useSharedValue: <T,>(value: T) => ({ value }),
24+
setGestureState: jest.fn(),
25+
};
26+
27+
return { Reanimated };
28+
});
29+
30+
const { Reanimated: MockedReanimated } = jest.requireMock(
31+
'../handlers/gestures/reanimatedWrapper'
32+
) as unknown as { Reanimated: { useHandler: jest.Mock } };
33+
34+
const lastHandlers = () =>
35+
MockedReanimated.useHandler.mock.calls.at(-1)?.[0] as Record<string, unknown>;
36+
37+
const makeFakeSharedValue = (value: boolean) =>
38+
({
39+
value,
40+
addListener: jest.fn(),
41+
removeListener: jest.fn(),
42+
modify: jest.fn(),
43+
}) as unknown as SharedValue<boolean>;
44+
45+
// When `runOnJS` is statically `true`, the Reanimated event path can never
46+
// receive events, so user callbacks must not be serialized to the UI runtime
47+
// (in dev, Worklets freezes every plain object captured in their closures,
48+
// silently dropping later writes). These tests guard the substitution in
49+
// `useGestureCallbacks` and the re-registration when `runOnJS` changes.
50+
describe('[API v3] Reanimated handler registration vs runOnJS', () => {
51+
beforeEach(() => {
52+
MockedReanimated.useHandler.mockClear();
53+
});
54+
55+
test('registers an empty handler set when runOnJS is statically true', () => {
56+
const onActivate = jest.fn();
57+
58+
renderHook(() => useTapGesture({ onActivate, runOnJS: true }));
59+
60+
expect(MockedReanimated.useHandler).toHaveBeenCalled();
61+
for (const call of MockedReanimated.useHandler.mock.calls) {
62+
expect(Object.keys(call[0] as object)).toHaveLength(0);
63+
}
64+
});
65+
66+
test('registers real callbacks when runOnJS is not set', () => {
67+
const onActivate = jest.fn();
68+
69+
renderHook(() => useTapGesture({ onActivate }));
70+
71+
expect(lastHandlers().onActivate).toBe(onActivate);
72+
});
73+
74+
test('registers real callbacks when runOnJS is false', () => {
75+
const onActivate = jest.fn();
76+
77+
renderHook(() => useTapGesture({ onActivate, runOnJS: false }));
78+
79+
expect(lastHandlers().onActivate).toBe(onActivate);
80+
});
81+
82+
test('re-registers real callbacks when runOnJS changes from true to false', () => {
83+
const onActivate = jest.fn();
84+
85+
const { rerender } = renderHook(
86+
({ runOnJS }: { runOnJS: boolean }) =>
87+
useTapGesture({ onActivate, runOnJS }),
88+
{ initialProps: { runOnJS: true } }
89+
);
90+
91+
expect(Object.keys(lastHandlers())).toHaveLength(0);
92+
93+
rerender({ runOnJS: false });
94+
95+
expect(lastHandlers().onActivate).toBe(onActivate);
96+
});
97+
98+
test('swaps back to the empty handler set when runOnJS returns to true', () => {
99+
const onActivate = jest.fn();
100+
101+
const { rerender } = renderHook(
102+
({ runOnJS }: { runOnJS: boolean }) =>
103+
useTapGesture({ onActivate, runOnJS }),
104+
{ initialProps: { runOnJS: false } }
105+
);
106+
107+
expect(lastHandlers().onActivate).toBe(onActivate);
108+
109+
rerender({ runOnJS: true });
110+
111+
expect(Object.keys(lastHandlers())).toHaveLength(0);
112+
});
113+
114+
test('keeps real callbacks when runOnJS is a SharedValue holding true', () => {
115+
const onActivate = jest.fn();
116+
117+
renderHook(() =>
118+
useTapGesture({ onActivate, runOnJS: makeFakeSharedValue(true) })
119+
);
120+
121+
expect(lastHandlers().onActivate).toBe(onActivate);
122+
});
123+
});

packages/react-native-gesture-handler/src/v3/hooks/useGestureCallbacks.ts

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,8 @@ import {
1313
useMemoizedGestureCallbacks,
1414
} from './utils';
1515

16+
const EMPTY_UI_CALLBACKS = {};
17+
1618
function guardJSAnimatedEvent(handler: (...args: unknown[]) => void) {
1719
return (...args: unknown[]) => {
1820
try {
@@ -51,12 +53,22 @@ export function useGestureCallbacks<
5153
let reanimatedEventHandler;
5254

5355
if (!config.disableReanimated) {
56+
// Passing callbacks to UI runtime when `runOnJS` is true would freeze
57+
// the closure, so we pass an empty object instead.
58+
//
59+
// This check is true only when `runOnJS` is a constant or a React state.
60+
// When `runOnJS` is a SharedValue, we pass original callbacks.
61+
const uiCallbacks =
62+
config.runOnJS === true
63+
? (EMPTY_UI_CALLBACKS as typeof callbacks)
64+
: callbacks;
65+
5466
// eslint-disable-next-line react-hooks/rules-of-hooks
55-
const reanimatedHandler = Reanimated?.useHandler(callbacks);
67+
const reanimatedHandler = Reanimated?.useHandler(uiCallbacks);
5668
// eslint-disable-next-line react-hooks/rules-of-hooks
5769
reanimatedEventHandler = useReanimatedEventHandler(
5870
handlerTag,
59-
callbacks,
71+
uiCallbacks,
6072
reanimatedHandler,
6173
config.changeEventCalculator,
6274
config.fillInDefaultValues

0 commit comments

Comments
 (0)