Skip to content

[Web] Do not detach gestures from other detectors#4286

Open
m-bert wants to merge 2 commits into
mainfrom
@mbert/web-reattach
Open

[Web] Do not detach gestures from other detectors#4286
m-bert wants to merge 2 commits into
mainfrom
@mbert/web-reattach

Conversation

@m-bert

@m-bert m-bert commented Jun 26, 2026

Copy link
Copy Markdown
Collaborator

Description

Fixes reattaching a v3 gesture between GestureDetectors on web. Moving a gesture from one detector to another would, after a cycle or two, stop working and then crash with Expected delegate view to be HTMLElement.

On web a gesture handler is shared per handler tag, so during a reattach both the old and the new detector touch the same handler. The new detector binds the handler to its view, but the old detector's cleanup then ran an unconditional detach — tearing down the binding the new detector had just established (and eventually calling detach on an already-cleared view, which threw). This mirrors the native ownership model on web.

Test plan

Tested on the following code:
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,
  },
});

Copilot AI review requested due to automatic review settings June 26, 2026 09:02

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes a web-specific v3 reattach scenario where moving a single gesture instance between GestureDetectors could eventually stop working and crash due to one detector detaching a handler that had already been rebound by another detector. The approach mirrors a native-like ownership model on web by associating an attached handler with the “host detector” and only allowing that host to detach it.

Changes:

  • Introduces a HostDetector ref type and threads it through web handler initialization to track which detector currently owns a handler binding.
  • Updates the web HostGestureDetector to pass its viewRef as the host identity when attaching and detaching handlers, preventing old detectors from tearing down new bindings.
  • Extends NodeManager / RNGestureHandlerModule.web detach/attach APIs to accept the host identity and apply ownership checks.

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
packages/react-native-gesture-handler/src/web/tools/NodeManager.ts Adds host-aware detachment to avoid detaching handlers owned by another detector.
packages/react-native-gesture-handler/src/web/interfaces.ts Defines HostDetector type used as the detector ownership identity.
packages/react-native-gesture-handler/src/web/handlers/RotationGestureHandler.ts Threads hostDetector through handler init override.
packages/react-native-gesture-handler/src/web/handlers/PinchGestureHandler.ts Threads hostDetector through handler init override.
packages/react-native-gesture-handler/src/web/handlers/NativeViewGestureHandler.ts Threads hostDetector through handler init override.
packages/react-native-gesture-handler/src/web/handlers/LongPressGestureHandler.ts Threads hostDetector through handler init override.
packages/react-native-gesture-handler/src/web/handlers/IGestureHandler.ts Extends the web handler interface with hostDetectorView and a typed init signature.
packages/react-native-gesture-handler/src/web/handlers/GestureHandler.ts Stores host detector identity on init to support ownership-based detach decisions.
packages/react-native-gesture-handler/src/v3/detectors/HostGestureDetector.web.tsx Passes a stable viewRef as both observation owner and detach/attach host identity.
packages/react-native-gesture-handler/src/RNGestureHandlerModule.web.ts Extends attachGestureHandler/detachGestureHandler signatures to accept host identity on web.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread packages/react-native-gesture-handler/src/web/tools/NodeManager.ts
Skip handler.detach() when the handler isn't attached, so a repeated
detachGestureHandler is a safe no-op instead of re-running delegate
cleanup on an already-cleared view (which would throw).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@m-bert m-bert requested a review from j-piasecki June 26, 2026 09:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants