Skip to content

Commit 35ec17d

Browse files
authored
Add checks for the same instance of handler usage across multiple GestureDetectors (#2694)
## Description Some of our users incorrectly use gesture handlers - they pass the same gesture handler instance into multiple `GestureDetectors`. It very often leads to unexpected behavior, like described in #2688. Currently web version of our library throws error `Handler with tag x already exists`. However, there are 2 problems with that: 1. This error message is not really helpful in case of fixing that problem. 2. Native platforms do not perform this check, so people don't know that they're using our handlers in a wrong way. This PR: - Improves error message - Adds check on native platforms - Adds information about error in `GestureDetector` documentation in remarks section ## Test plan Tested with code below on all platforms. <details> <summary>Code that throws error</summary> ```jsx import React from 'react'; import { View } from 'react-native'; import { Gesture, GestureDetector } from 'react-native-gesture-handler'; export default function Example() { const pan = Gesture.Pan(); return ( <View> <GestureDetector gesture={pan}> <View> <GestureDetector gesture={pan}> <View /> </GestureDetector> </View> </GestureDetector> </View> ); } ``` </details>
1 parent cf45c38 commit 35ec17d

File tree

4 files changed

+40
-1
lines changed

4 files changed

+40
-1
lines changed

android/src/main/java/com/swmansion/gesturehandler/react/RNGestureHandlerModule.kt

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -340,6 +340,13 @@ class RNGestureHandlerModule(reactContext: ReactApplicationContext?) :
340340
handlerTag: Int,
341341
config: ReadableMap,
342342
) {
343+
344+
if (registry.getHandler(handlerTag) !== null) {
345+
throw IllegalStateException(
346+
"Handler with tag $handlerTag already exists. Please ensure that no Gesture instance is used across multiple GestureDetectors."
347+
)
348+
}
349+
343350
for (handlerFactory in handlerFactories as Array<HandlerFactory<T>>) {
344351
if (handlerFactory.name == handlerName) {
345352
val handler = handlerFactory.create(reactApplicationContext).apply {

apple/RNGestureHandlerManager.mm

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,14 @@ - (instancetype)initWithUIManager:(RCTUIManager *)uiManager eventDispatcher:(RCT
7171

7272
- (void)createGestureHandler:(NSString *)handlerName tag:(NSNumber *)handlerTag config:(NSDictionary *)config
7373
{
74+
if ([_registry handlerWithTag:handlerTag] != nullptr) {
75+
NSString *errorMessage = [NSString
76+
stringWithFormat:
77+
@"Handler with tag %@ already exists. Please ensure that no Gesture instance is used across multiple GestureDetectors.",
78+
handlerTag];
79+
@throw [NSException exceptionWithName:@"HandlerAlreadyRegistered" reason:errorMessage userInfo:nil];
80+
}
81+
7482
static NSDictionary *map;
7583
static dispatch_once_t mapToken;
7684
dispatch_once(&mapToken, ^{

docs/docs/gestures/gesture-detector.md

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,3 +47,25 @@ This parameter allows to specify which `userSelect` property should be applied t
4747
- Gesture Detector will use first native view in its subtree to recognize gestures, however if this view is used only to group its children it may get automatically [collapsed](https://reactnative.dev/docs/view#collapsable-android). Consider this example:
4848
<FunctionalComponents />
4949
If we were to remove the collapsable prop from the View, the gesture would stop working because it would be attached to a view that is not present in the view hierarchy. Gesture Detector adds this prop automatically to its direct child but it's impossible to do automatically for more complex view trees.
50+
51+
- Using the same instance of a gesture across multiple Gesture Detectors is not possible. Have a look at the code below:
52+
53+
```jsx
54+
export default function Example() {
55+
const pan = Gesture.Pan();
56+
57+
return (
58+
<View>
59+
<GestureDetector gesture={pan}>
60+
<View>
61+
<GestureDetector gesture={pan}> {/* Don't do this! */}
62+
<View />
63+
</GestureDetector>
64+
</View>
65+
</GestureDetector>
66+
</View>
67+
);
68+
}
69+
```
70+
71+
This example will throw an error, becuse we try to use the same instance of `Pan` in two different Gesture Detectors.

src/web/tools/NodeManager.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,9 @@ export default abstract class NodeManager {
2121
handler: InstanceType<ValueOf<typeof Gestures>>
2222
): void {
2323
if (handlerTag in this.gestures) {
24-
throw new Error(`Handler with tag ${handlerTag} already exists`);
24+
throw new Error(
25+
`Handler with tag ${handlerTag} already exists. Please ensure that no Gesture instance is used across multiple GestureDetectors.`
26+
);
2527
}
2628

2729
this.gestures[handlerTag] = handler;

0 commit comments

Comments
 (0)