Skip to content

Commit

Permalink
Add checks for the same instance of handler usage across multiple `Ge…
Browse files Browse the repository at this point in the history
…stureDetectors` (#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>
  • Loading branch information
m-bert authored Dec 19, 2023
1 parent cf45c38 commit 35ec17d
Show file tree
Hide file tree
Showing 4 changed files with 40 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
8 changes: 8 additions & 0 deletions apple/RNGestureHandlerManager.mm
Original file line number Diff line number Diff line change
Expand Up @@ -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, ^{
Expand Down
22 changes: 22 additions & 0 deletions docs/docs/gestures/gesture-detector.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
4 changes: 3 additions & 1 deletion src/web/tools/NodeManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down

0 comments on commit 35ec17d

Please sign in to comment.