From 35ec17d636220c1ad4226456aa3b03c846ba16a8 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Micha=C5=82=20Bert?=
 <63123542+m-bert@users.noreply.github.com>
Date: Tue, 19 Dec 2023 09:40:38 +0100
Subject: [PATCH] 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>
---
 .../react/RNGestureHandlerModule.kt           |  7 ++++++
 apple/RNGestureHandlerManager.mm              |  8 +++++++
 docs/docs/gestures/gesture-detector.md        | 22 +++++++++++++++++++
 src/web/tools/NodeManager.ts                  |  4 +++-
 4 files changed, 40 insertions(+), 1 deletion(-)

diff --git a/android/src/main/java/com/swmansion/gesturehandler/react/RNGestureHandlerModule.kt b/android/src/main/java/com/swmansion/gesturehandler/react/RNGestureHandlerModule.kt
index d786284053..d242eb6f11 100644
--- a/android/src/main/java/com/swmansion/gesturehandler/react/RNGestureHandlerModule.kt
+++ b/android/src/main/java/com/swmansion/gesturehandler/react/RNGestureHandlerModule.kt
@@ -340,6 +340,13 @@ class RNGestureHandlerModule(reactContext: ReactApplicationContext?) :
     handlerTag: Int,
     config: ReadableMap,
   ) {
+
+    if (registry.getHandler(handlerTag) !== null) {
+      throw IllegalStateException(
+        "Handler with tag $handlerTag already exists. Please ensure that no Gesture instance is used across multiple GestureDetectors."
+      )
+    }
+
     for (handlerFactory in handlerFactories as Array<HandlerFactory<T>>) {
       if (handlerFactory.name == handlerName) {
         val handler = handlerFactory.create(reactApplicationContext).apply {
diff --git a/apple/RNGestureHandlerManager.mm b/apple/RNGestureHandlerManager.mm
index d5158826f6..83d9058762 100644
--- a/apple/RNGestureHandlerManager.mm
+++ b/apple/RNGestureHandlerManager.mm
@@ -71,6 +71,14 @@ - (instancetype)initWithUIManager:(RCTUIManager *)uiManager eventDispatcher:(RCT
 
 - (void)createGestureHandler:(NSString *)handlerName tag:(NSNumber *)handlerTag config:(NSDictionary *)config
 {
+  if ([_registry handlerWithTag:handlerTag] != nullptr) {
+    NSString *errorMessage = [NSString
+        stringWithFormat:
+            @"Handler with tag %@ already exists. Please ensure that no Gesture instance is used across multiple GestureDetectors.",
+            handlerTag];
+    @throw [NSException exceptionWithName:@"HandlerAlreadyRegistered" reason:errorMessage userInfo:nil];
+  }
+
   static NSDictionary *map;
   static dispatch_once_t mapToken;
   dispatch_once(&mapToken, ^{
diff --git a/docs/docs/gestures/gesture-detector.md b/docs/docs/gestures/gesture-detector.md
index bab53b6b11..335672f22d 100644
--- a/docs/docs/gestures/gesture-detector.md
+++ b/docs/docs/gestures/gesture-detector.md
@@ -47,3 +47,25 @@ This parameter allows to specify which `userSelect` property should be applied t
 - 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:
   <FunctionalComponents />
   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.
+
+- Using the same instance of a gesture across multiple Gesture Detectors is not possible. Have a look at the code below:
+
+  ```jsx
+  export default function Example() {
+    const pan = Gesture.Pan();
+
+    return (
+      <View>
+        <GestureDetector gesture={pan}>
+          <View>
+            <GestureDetector gesture={pan}> {/* Don't do this! */}
+              <View />
+            </GestureDetector>
+          </View>
+        </GestureDetector>
+      </View>
+    );
+  }
+  ```
+
+  This example will throw an error, becuse we try to use the same instance of `Pan` in two different Gesture Detectors.
diff --git a/src/web/tools/NodeManager.ts b/src/web/tools/NodeManager.ts
index 825f4d0f84..fd4a9c630c 100644
--- a/src/web/tools/NodeManager.ts
+++ b/src/web/tools/NodeManager.ts
@@ -21,7 +21,9 @@ export default abstract class NodeManager {
     handler: InstanceType<ValueOf<typeof Gestures>>
   ): void {
     if (handlerTag in this.gestures) {
-      throw new Error(`Handler with tag ${handlerTag} already exists`);
+      throw new Error(
+        `Handler with tag ${handlerTag} already exists. Please ensure that no Gesture instance is used across multiple GestureDetectors.`
+      );
     }
 
     this.gestures[handlerTag] = handler;