Skip to content

Commit 1a713d0

Browse files
m-bertCopilotclaude
authored
Ensure no gesture is used across multiple detectors (#4285)
## Description Currently there's no error when the same gesture is used in multiple detectors. This may results in a bug, e.g. on Android and iOS only last detector captures gestures, on web all of them. This PR adds JS side check whether instance of a gesture was passed into more than one detector. > [!NOTE] > The example below crashes on web, in reattaching scenario. However, this seems to be unrelated to this PR, so I left that for a follow-up. ## Test plan <details> <summary>Tested on the following code:</summary> ```tsx import { useRef, useState } from 'react'; import { Pressable, StyleSheet, Text, View } from 'react-native'; import { GestureDetector, useTapGesture } from 'react-native-gesture-handler'; import type { FeedbackHandle } from '../../../common'; import { COLORS, commonStyles, Feedback } from '../../../common'; type Mode = 'separate' | 'reattach' | 'shared'; // Demonstrates the guard that forbids attaching a single gesture instance to // more than one GestureDetector at a time. // // - "Separate gestures (OK)" -> each detector gets its own gesture. // - "Reattach mode (OK)" -> tapping the ⭐ box moves the SAME gesture to // the other detector. The detector losing it // releases it before the other claims it, so // it never throws. The ⭐ badge and its tap // counter travel with the instance, so you can // see it is literally the same gesture moving. // - "Share one gesture (throws)" -> the SAME gesture is rendered in BOTH // detectors at once. This is the misuse we // catch - it throws on the second detector. export default function SharedGestureExample() { const [mode, setMode] = useState<Mode>('separate'); // In reattach mode, which detector currently holds the shared gesture. const [sharedOnTop, setSharedOnTop] = useState(true); // Tap count of the shared gesture - kept in a ref so incrementing it does not // trigger a second state update (the setSharedOnTop re-render reads it fresh). const sharedTapsRef = useRef(0); const feedbackRef = useRef<FeedbackHandle>(null); const sharedGesture = useTapGesture({ onActivate: () => { sharedTapsRef.current += 1; // Tapping the box that holds the shared gesture moves (reattaches) it to // the other detector - exactly one detector ever holds it, so no throw. setSharedOnTop((prev) => !prev); feedbackRef.current?.showMessage('⭐ shared gesture tapped → moving'); }, runOnJS: true, }); const topOwnGesture = useTapGesture({ onActivate: () => feedbackRef.current?.showMessage('top gesture tapped'), runOnJS: true, }); const bottomOwnGesture = useTapGesture({ onActivate: () => feedbackRef.current?.showMessage('bottom gesture tapped'), runOnJS: true, }); // Single placeholder used by whichever box does NOT currently hold the shared // gesture in reattach mode (mirrors the existing `reattaching` example). const placeholderGesture = useTapGesture({ onActivate: () => feedbackRef.current?.showMessage('inert box tapped'), runOnJS: true, }); // Decide which gesture each detector receives, and whether it's the shared one. let topGesture = topOwnGesture; let bottomGesture = bottomOwnGesture; let topHasShared = false; let bottomHasShared = false; if (mode === 'shared') { // Same instance in both detectors at the same time -> throws. topGesture = sharedGesture; bottomGesture = sharedGesture; topHasShared = true; bottomHasShared = true; } else if (mode === 'reattach') { // Exactly one detector holds the shared gesture at any moment -> allowed. topGesture = sharedOnTop ? sharedGesture : placeholderGesture; bottomGesture = sharedOnTop ? placeholderGesture : sharedGesture; topHasShared = sharedOnTop; bottomHasShared = !sharedOnTop; } return ( <View style={commonStyles.centerView}> <View style={styles.controls}> <Button label="Separate gestures (OK)" onPress={() => setMode('separate')} /> <Button label="Reattach mode (OK)" onPress={() => { setMode('reattach'); setSharedOnTop(true); }} /> <Button label="Share one gesture (throws)" danger onPress={() => setMode('shared')} /> </View> <Text style={styles.status}> mode: {mode} {mode === 'reattach' ? ` · tap the ⭐ box to move the shared gesture` : ''} </Text> <GestureDetector gesture={topGesture}> <Box title="Top" color={COLORS.NAVY} hasShared={topHasShared} sharedTaps={sharedTapsRef.current} /> </GestureDetector> <GestureDetector gesture={bottomGesture}> <Box title="Bottom" color={COLORS.GREEN} hasShared={bottomHasShared} sharedTaps={sharedTapsRef.current} /> </GestureDetector> <Feedback ref={feedbackRef} /> </View> ); } function Box({ title, color, hasShared, sharedTaps, }: { title: string; color: string; hasShared: boolean; sharedTaps: number; }) { return ( <View style={[ commonStyles.box, { backgroundColor: color }, hasShared && styles.boxWithShared, ]}> <Text style={styles.boxText}>{title}</Text> {hasShared ? ( <Text style={styles.badge}> ⭐ shared{'\n'} {sharedTaps} taps </Text> ) : ( <Text style={styles.boxSubText}>own gesture</Text> )} </View> ); } function Button({ label, onPress, danger, }: { label: string; onPress: () => void; danger?: boolean; }) { return ( <Pressable onPress={onPress} style={[styles.button, danger && styles.buttonDanger]}> <Text style={styles.buttonText}>{label}</Text> </Pressable> ); } const styles = StyleSheet.create({ controls: { gap: 8, marginBottom: 24, width: '100%', paddingHorizontal: 16, }, button: { backgroundColor: COLORS.NAVY, paddingVertical: 12, borderRadius: 10, alignItems: 'center', }, buttonDanger: { backgroundColor: COLORS.RED, }, buttonText: { color: 'white', fontWeight: '600', }, status: { marginBottom: 16, color: COLORS.NAVY, fontWeight: '600', }, boxWithShared: { borderWidth: 4, borderColor: COLORS.KINDA_YELLOW, }, boxText: { color: 'white', fontWeight: '700', fontSize: 18, }, boxSubText: { color: 'white', opacity: 0.8, marginTop: 4, }, badge: { color: COLORS.KINDA_YELLOW, fontWeight: '700', textAlign: 'center', marginTop: 4, }, }); ``` </details> --------- Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent ede0f4d commit 1a713d0

5 files changed

Lines changed: 160 additions & 5 deletions

File tree

packages/react-native-gesture-handler/src/__tests__/Errors.test.tsx

Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,94 @@ describe('VirtualDetector', () => {
8383
});
8484
});
8585

86+
describe('Sharing a gesture across detectors', () => {
87+
beforeEach(() => (findNodeHandle as jest.Mock).mockReturnValue(123));
88+
89+
test('throws when the same gesture is attached to two detectors', () => {
90+
function SameGestureTwoDetectors() {
91+
const tap = useTapGesture({});
92+
return (
93+
<GestureHandlerRootView>
94+
<GestureDetector gesture={tap}>
95+
<View />
96+
</GestureDetector>
97+
<GestureDetector gesture={tap}>
98+
<View />
99+
</GestureDetector>
100+
</GestureHandlerRootView>
101+
);
102+
}
103+
104+
expect(() => render(<SameGestureTwoDetectors />)).toThrow(
105+
'Using the same gesture instance across multiple GestureDetectors is not possible. Create a separate gesture for each detector.'
106+
);
107+
});
108+
109+
test('does not throw when the same gesture is reattached to another detector', () => {
110+
// Changing the key unmounts the previous detector and mounts a new one with
111+
// the same gesture - the old detector must release the gesture before the
112+
// new one claims it.
113+
function ReattachedGesture({ detectorKey }: { detectorKey: string }) {
114+
const tap = useTapGesture({});
115+
return (
116+
<GestureHandlerRootView>
117+
<GestureDetector key={detectorKey} gesture={tap}>
118+
<View />
119+
</GestureDetector>
120+
</GestureHandlerRootView>
121+
);
122+
}
123+
124+
const { rerender } = render(<ReattachedGesture detectorKey="a" />);
125+
expect(() => rerender(<ReattachedGesture detectorKey="b" />)).not.toThrow();
126+
});
127+
128+
test('does not throw when gestures are swapped between two mounted detectors', () => {
129+
// Prop-swap reattach: both detectors stay mounted and the two gestures
130+
// trade places. The detector losing a gesture must release it (effect
131+
// cleanup) before the detector gaining it claims it (effect setup).
132+
function SwappedGestures({ swapped }: { swapped: boolean }) {
133+
const tapA = useTapGesture({});
134+
const tapB = useTapGesture({});
135+
return (
136+
<GestureHandlerRootView>
137+
<GestureDetector gesture={swapped ? tapB : tapA}>
138+
<View />
139+
</GestureDetector>
140+
<GestureDetector gesture={swapped ? tapA : tapB}>
141+
<View />
142+
</GestureDetector>
143+
</GestureHandlerRootView>
144+
);
145+
}
146+
147+
const { rerender } = render(<SwappedGestures swapped={false} />);
148+
expect(() => rerender(<SwappedGestures swapped />)).not.toThrow();
149+
});
150+
151+
test('throws when the same gesture is attached to two virtual detectors', () => {
152+
function SameGestureTwoVirtualDetectors() {
153+
const tap = useTapGesture({});
154+
return (
155+
<GestureHandlerRootView>
156+
<InterceptingGestureDetector>
157+
<VirtualDetector gesture={tap}>
158+
<View />
159+
</VirtualDetector>
160+
<VirtualDetector gesture={tap}>
161+
<View />
162+
</VirtualDetector>
163+
</InterceptingGestureDetector>
164+
</GestureHandlerRootView>
165+
);
166+
}
167+
168+
expect(() => render(<SameGestureTwoVirtualDetectors />)).toThrow(
169+
'Using the same gesture instance across multiple GestureDetectors is not possible. Create a separate gesture for each detector.'
170+
);
171+
});
172+
});
173+
86174
describe('Check if descendant of root view', () => {
87175
test('gesture detector', () => {
88176
function GestureDetectorNoRootView() {

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import type { NativeDetectorProps } from './common';
77
import { AnimatedNativeDetector, nativeDetectorStyles } from './common';
88
import HostGestureDetector from './HostGestureDetector';
99
import { ReanimatedNativeDetector } from './ReanimatedNativeDetector';
10+
import { useDetectorAttachmentGuard } from './useDetectorAttachmentGuard';
1011
import { useGestureRelationsUpdater } from './useGestureRelationsUpdater';
1112
import { ensureNativeDetectorComponent } from './utils';
1213

@@ -38,6 +39,8 @@ export function NativeDetector<
3839
: [gesture.handlerTag];
3940
}, [gesture]);
4041

42+
useDetectorAttachmentGuard(handlerTags);
43+
4144
// On web, we're triggering Reanimated callbacks ourselves, based on the type.
4245
// To handle this properly, we need to provide all three callbacks, so we set
4346
// all three to the Reanimated event handler.

packages/react-native-gesture-handler/src/v3/detectors/VirtualDetector/InterceptingGestureDetector.tsx

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import type { InterceptingGestureDetectorProps } from '../common';
1717
import { AnimatedNativeDetector, nativeDetectorStyles } from '../common';
1818
import HostGestureDetector from '../HostGestureDetector';
1919
import { ReanimatedNativeDetector } from '../ReanimatedNativeDetector';
20+
import { useDetectorAttachmentGuard } from '../useDetectorAttachmentGuard';
2021
import { useEnsureGestureHandlerRootView } from '../useEnsureGestureHandlerRootView';
2122
import { useGestureRelationsUpdater } from '../useGestureRelationsUpdater';
2223
import { ensureNativeDetectorComponent } from '../utils';
@@ -233,6 +234,8 @@ export function InterceptingGestureDetector<
233234
return [];
234235
}, [gesture]);
235236

237+
useDetectorAttachmentGuard(handlerTags);
238+
236239
// On web, we're triggering Reanimated callbacks ourselves, based on the type.
237240
// To handle this properly, we need to provide all three callbacks, so we set
238241
// all three to the Reanimated event handler.

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

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,12 @@
1-
import { useCallback, useEffect, useRef, useState } from 'react';
1+
import { useCallback, useEffect, useMemo, useRef, useState } from 'react';
22
import { findNodeHandle, Platform } from 'react-native';
33

44
import { Wrap } from '../../../handlers/gestures/GestureDetector/Wrap';
55
import { tagMessage } from '../../../utils';
66
import { isComposedGesture } from '../../hooks/utils/relationUtils';
77
import type { DetectorCallbacks, VirtualChild } from '../../types';
88
import type { VirtualDetectorProps } from '../common';
9+
import { useDetectorAttachmentGuard } from '../useDetectorAttachmentGuard';
910
import { useGestureRelationsUpdater } from '../useGestureRelationsUpdater';
1011
import { useNativeGestureRole } from '../useNativeGestureRole';
1112
import {
@@ -57,15 +58,21 @@ export function VirtualDetector<
5758

5859
useNativeGestureRole(viewRef, props.children);
5960

61+
const handlerTags = useMemo(
62+
() =>
63+
isComposedGesture(props.gesture)
64+
? props.gesture.handlerTags
65+
: [props.gesture.handlerTag],
66+
[props.gesture]
67+
);
68+
69+
useDetectorAttachmentGuard(handlerTags);
70+
6071
useEffect(() => {
6172
if (viewTag === -1) {
6273
return;
6374
}
6475

65-
const handlerTags = isComposedGesture(props.gesture)
66-
? props.gesture.handlerTags
67-
: [props.gesture.handlerTag];
68-
6976
if (props.gesture.config.dispatchesAnimatedEvents) {
7077
throw new Error(
7178
tagMessage(
@@ -96,6 +103,7 @@ export function VirtualDetector<
96103
}, [
97104
viewTag,
98105
props.gesture,
106+
handlerTags,
99107
props.userSelect,
100108
props.touchAction,
101109
props.enableContextMenu,
Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
import { useEffect, useRef } from 'react';
2+
3+
import { tagMessage } from '../../utils';
4+
5+
// Maps a gesture's handlerTag to the id of the detector currently rendering it.
6+
// Ownership is claimed in an effect and released in its cleanup (on unmount or
7+
// when handlerTags change), so moving a gesture instance between detectors works
8+
// as long as it isn't rendered by more than one detector at the same time.
9+
const detectorByHandlerTag = new Map<number, number>();
10+
11+
let nextDetectorId = 0;
12+
13+
export function useDetectorAttachmentGuard(handlerTags: number[]) {
14+
if (!__DEV__) {
15+
return;
16+
}
17+
18+
// eslint-disable-next-line react-hooks/rules-of-hooks
19+
const detectorId = useRef(-1);
20+
21+
if (detectorId.current === -1) {
22+
detectorId.current = nextDetectorId++;
23+
}
24+
25+
// eslint-disable-next-line react-hooks/rules-of-hooks
26+
useEffect(() => {
27+
const id = detectorId.current;
28+
29+
for (const tag of handlerTags) {
30+
const owner = detectorByHandlerTag.get(tag);
31+
32+
if (owner !== undefined && owner !== id) {
33+
throw new Error(
34+
tagMessage(
35+
'Using the same gesture instance across multiple GestureDetectors is not possible. Create a separate gesture for each detector.'
36+
)
37+
);
38+
}
39+
}
40+
41+
for (const tag of handlerTags) {
42+
detectorByHandlerTag.set(tag, id);
43+
}
44+
45+
return () => {
46+
for (const tag of handlerTags) {
47+
if (detectorByHandlerTag.get(tag) === id) {
48+
detectorByHandlerTag.delete(tag);
49+
}
50+
}
51+
};
52+
}, [handlerTags]);
53+
}

0 commit comments

Comments
 (0)