Pass reanimatedEventHandler only when the Reanimated detector is used - #4429
Merged
m-bert merged 2 commits intoAug 18, 2026
Merged
Conversation
The handler is built whenever `disableReanimated` is unset, but `shouldUseReanimatedDetector` additionally requires worklet callbacks. A gesture with none renders the plain host component, which forwards props verbatim, so it received Reanimated's event handler object under `onGestureHandlerReanimatedEvent`, a codegen DirectEventHandler prop. React then throws out of `getListener` instead of dispatching.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Included review availability: Your plan includes up to 8 reviews per rolling hour; 7 remain after this review. 📝 WalkthroughSummary by CodeRabbit
WalkthroughChangesNative detector event handling
Merge Risk: ⚪ Minimal · up to The change limits the Reanimated event handler to the detector that uses it, preventing an invalid handler object from being passed to plain gesture detectors. No actionable merge-blocking risk remains beyond normal checks and review. 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes a New Architecture crash where NativeDetector could forward Reanimated’s { workletEventHandler } object into a codegen DirectEventHandler prop (onGestureHandlerReanimatedEvent) when the Reanimated detector is not in use, causing React to throw when resolving listeners.
Changes:
- Gate
onGestureHandlerReanimatedEventso it’s only provided whengesture.config.shouldUseReanimatedDetectoris true (native path), otherwise passundefinedto avoid an invalid listener type. - Add an in-file comment documenting why the guard is necessary and how the mismatch occurs.
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
m-bert
reviewed
Aug 17, 2026
m-bert
left a comment
m-bert
left a comment
Collaborator
There was a problem hiding this comment.
Hi @antFrancon! Thank you for submitting this PR! Could you please also add the same guard for the web part? I know that it should be safe since we have forReanimated, but I'd like to keep these in sync 😅
Comment on lines
+65
to
+68
| // `reanimatedEventHandler` is built whenever `disableReanimated` is unset, but | ||
| // `shouldUseReanimatedDetector` additionally requires worklet callbacks. When it is | ||
| // false we render the plain host component, which forwards props verbatim, so passing | ||
| // the handler would put a non-function on a codegen `DirectEventHandler` prop. |
Collaborator
There was a problem hiding this comment.
I don't think we need this
Suggested change
| // `reanimatedEventHandler` is built whenever `disableReanimated` is unset, but | |
| // `shouldUseReanimatedDetector` additionally requires worklet callbacks. When it is | |
| // false we render the plain host component, which forwards props verbatim, so passing | |
| // the handler would put a non-function on a codegen `DirectEventHandler` prop. |
Web is safe today because the handler only emits Reanimated events when `forReanimated` is set, which itself comes from `dispatchesReanimatedEvents` and so already requires `shouldUseReanimatedDetector`. Hoisting the guard keeps both branches in sync rather than relying on that.
Contributor
Author
|
Done, pushed. I hoisted the guard into a single And you are right that web is already safe:
|
m-bert
approved these changes
Aug 17, 2026
m-bert
added a commit
that referenced
this pull request
Aug 19, 2026
## Description `RNGestureHandlerDetector` detaches its handlers and cancels registry observations only in `willMoveToWindow:` when the new window is `nil`. `UIKit` sends that callback only when the view's window actually changes, so a detector that is unmounted while its ancestor is already detached from the window (e.g. an inactive native-stack screen) never receives it. The view then enters Fabric's recycle pool still carrying the recognizers of live handlers and their `hostDetectorView` bindings - `prepareForRecycle` only reset the bookkeeping sets, and the base `RCTViewComponentView` implementation doesn't remove gesture recognizers. When such a view is reused for a different `GestureDetector`, the stale handler's events are emitted through the new detector's event emitter. If the new detector is a plain one, this throws ``` Expected onGestureHandlerReanimatedEvent listener to be a function, instead got a value of 'object' type ``` on every gesture frame (the visible half of #4428, see also #4429 which addresses the invalid prop itself). If the new detector is a Reanimated one, the foreign events are silently misrouted instead. This PR moves the cleanup into `detachAndCleanupHandlers` and calls it from both `willMoveToWindow:` and `prepareForRecycle`. The method is idempotent and skips views that were never configured (`moduleId == -1`), so the common path where `willMoveToWindow:` already ran is a no-op. This also makes iOS consistent with Android, where `onDropViewInstance` already calls `detachAllHandlers()` on unmount regardless of window state, which is why Android is not affected. ## Test plan - Ran the repro above on the iPhone 17 Pro simulator (iOS 26.4, expo-example, Fabric): before the change the error is thrown on every gesture frame, after the change the flow is clean in repeated runs. - Checked the regular paths on the same build: Fling and Tap examples, screen push/pop (the `willMoveToWindow:` detach/reattach cycle), Pressable rows and ScrollView on the examples list. <details> <summary>Tested on the following code:</summary> ```tsx import React, { useEffect, useRef, useState } from 'react'; import { Button, StyleSheet, Text, View } from 'react-native'; import { NavigationContainer, NavigationIndependentTree, useNavigation, } from '@react-navigation/native'; import { createNativeStackNavigator } from '@react-navigation/native-stack'; import { GestureDetector, usePanGesture, useTapGesture, } from 'react-native-gesture-handler'; // Repro for the visible half of #4428: a handler emitting through a detector // that is not its own. A worklet pan detector is mounted and unmounted while // its screen is detached from the window (inactive native-stack screen), so // RNGestureHandlerDetector's willMoveToWindow:nil cleanup never runs. The // detector view goes to Fabric's recycle pool still carrying the pan // recognizer and hostDetectorView binding. A plain-gesture detector mounted // afterwards recycles that view; panning on it should emit // onGestureHandlerReanimatedEvent into the plain HostGestureDetector, whose // prop is Reanimated's handler object -> "Expected onGestureHandlerReanimatedEvent // listener to be a function" on every frame. const Stack = createNativeStackNavigator(); function ReproScreen() { const navigation = useNavigation<any>(); const [phase, setPhase] = useState('idle'); const [showPan, setShowPan] = useState(false); const [showTarget, setShowTarget] = useState(false); const timers = useRef<ReturnType<typeof setTimeout>[]>([]); // Worklet callback -> shouldUseReanimatedDetector=true, // dispatchesReanimatedEvents=true on the native handler. const pan = usePanGesture({ onUpdate: (e) => { 'worklet'; console.log('pan onUpdate (worklet)', e.translationX); }, }); // No callbacks: any callback here gets auto-workletized by babel (even when // passed by reference), which would flip this to ReanimatedNativeDetector. // Callback-less tap keeps the plain HostGestureDetector with the object prop, // same as the issue's useNativeGesture() case. const tap = useTapGesture({}); useEffect(() => { return () => timers.current.forEach(clearTimeout); }, []); const at = (ms: number, fn: () => void) => { timers.current.push(setTimeout(fn, ms)); }; const start = () => { setShowPan(false); setShowTarget(false); setPhase('pushed cover screen'); navigation.navigate('Cover'); at(800, () => { setPhase('pan detector mounted (detached)'); setShowPan(true); }); at(1600, () => { setPhase('pan detector unmounted (detached) -> dirty pool'); setShowPan(false); }); at(2400, () => { setPhase('popped back'); navigation.goBack(); }); at(3200, () => { setPhase('target mounted - PAN ON THE BLUE BOX'); setShowTarget(true); }); }; return ( <View style={styles.container}> <Button title="Start repro" onPress={start} /> <Text style={styles.status}>{phase}</Text> {showPan && ( <GestureDetector gesture={pan}> <View style={[styles.box, styles.red]} /> </GestureDetector> )} {showTarget && ( <GestureDetector gesture={tap}> <View style={[styles.box, styles.blue]} /> </GestureDetector> )} </View> ); } function CoverScreen() { return ( <View style={styles.container}> <Text style={styles.status}> Cover screen - the repro screen is now detached from the window. </Text> </View> ); } export default function EmptyExample() { return ( <NavigationIndependentTree> <NavigationContainer> <Stack.Navigator> <Stack.Screen name="Repro" component={ReproScreen} /> <Stack.Screen name="Cover" component={CoverScreen} /> </Stack.Navigator> </NavigationContainer> </NavigationIndependentTree> ); } const styles = StyleSheet.create({ container: { flex: 1, alignItems: 'center', gap: 20, paddingTop: 40, }, status: { fontSize: 16, paddingHorizontal: 20, textAlign: 'center', }, box: { width: 220, height: 220, borderRadius: 12, }, red: { backgroundColor: 'crimson', }, blue: { backgroundColor: 'steelblue', }, }); ``` </details>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
NativeDetectoralways passesreanimatedEventHandlerto the native component, even when the gesture does not use the Reanimated detector. Two different conditions decide whether the handler exists and whether the Reanimated detector is used.useGestureCallbacksbuilds the handler wheneverdisableReanimatedis unset:shouldUseReanimatedDetectoradditionally requires worklet callbacks:So a gesture with no worklet callbacks gets
shouldUseReanimatedDetector === false, renders the plainHostGestureDetector, which forwards props verbatim, and receives Reanimated's{ workletEventHandler }object underonGestureHandlerReanimatedEvent. That prop is a codegenDirectEventHandler, so React throws out ofgetListenerinstead of dispatching:It stays latent most of the time, because a handler on a plain detector gets
dispatchesReanimatedEvents: shouldUseReanimatedDetector && !runOnJS, which is false, so it never emits the event. It surfaces once a handler ends up attached to a detector that is not its own. The prop is invalid either way.This passes the handler only when the Reanimated detector is actually used. The event prop is part of the static view config, so
undefinedonly means there is no JS listener, and native still emits the event.I left the web branch untouched, since I have not tested whether the same mismatch applies there. Happy to extend it if you think it does.
Fixes #4428
Test plan
yarn ts-check,yarn lint:jsandyarn testinpackages/react-native-gesture-handler.NativeDetectorpicks and thetypeofof the handler it receives.useNativeGesture(),usePanGesture()anduseTapGesture({ onActivate })all renderHostGestureDetectorwithtypeof reanimatedEventHandler === 'object'.undefined, and the worklet control still gets the handler onReanimatedNativeDetector.