-
-
Notifications
You must be signed in to change notification settings - Fork 983
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix for #320, "Unsupported top level event type "onGestureHandlerEvent"" #792
Conversation
…stureHandlerEvent"" hook up one NativeEventEmitter to whitelist our event types
I'm also seeing the same behavior for |
Hello everyone :-), honest question: has anyone done a pure test on this without the workaround of importing react-native-gesture-handler early, and can reliably crash, then apply this fix and never crash? Just curious if this has been tested rigorously. If not, I could try that, and hopefully it could at least be provisionally merged until there was a good solution, as I'd like to remove the workaround in my own project and it would help new users I'm sure. @paddlefish I assume this nailed the problem for you? @kmagiera @solidfox have you tried it for onGestureHandlerStateChange? |
@mikehardy I gave this a shot by using this fix locally on react-native 0.61.3 and gesture handler 1.5.0 but replacing
|
@mikehardy I also tried adding listeners for both events and still got the error. |
I think there is a deeper problem for sure. I thought maybe this workaround would be a band aid... but perhaps just like real band aids they sometimes come off :-/ I did notice that the core react-native code has special case handling for gesture handler events. Maybe there is a newer better way events are supposed to be sent that would mean that this workaround isn’t needed AND that react-native doesn’t need to have special cases either. |
We are currently upgrading to React Native 0.61.3 and as part of that have upgraded to react-native-gesture-handler 1.5.0 to fix facebook/react-native#26626 (comment). I only started seeing this 'Unsupported top level event type "onGestureHandlerEvent"' crash after upgrading from react-native-gesture-handler 1.4.1 to 1.5.0. I can't offer any more repro steps than have already been mentioned in the various tickets but maybe this helps to narrow it down...? Edit: very anecdotally, but moving |
Hey @paddlefish – Thanks for working on this. Have you maybe tried your workaround when running on iOS in release mode? I think currently the main issue is with the release setup as otherwise the gesture handler events are whitelisted in RN core. |
I checked this PR and it appears not to be fixing the issue in release mode. Can you double check or confirm whether you tried release mode on iOS? |
I think I managed to find yet another workaround and submitted #845 |
I discovered it only in release builds (TestFlight). After my patch my TestFlight builds have been working. 🤷 |
Hey, for what it's worth, I can confirm I only see this issue in iOS release mode, and see it in an Expo ejected 36 app despite using 1.5.3 JS, while it's supposed to be fixed in 1.5.1 in https://github.com/software-mansion/react-native-gesture-handler/pull/845/files I guess Expo might use 1.5.0 native so upgrading just the JS is not enough (but at least does not crash anyway) |
@slorber for versioning gesture-handler libs we use this scheme where patch numbers are only JS updates. This is specifically to make it possible for expo users to receive some updates that does not require native code. It also means that 1.5.1 is compatible with 1.5.0 native code. Also note that the fix from #845 did not include any native changes |
Ah I see, I thought I had seen some native in the PR when I looked. Was already on 1.5.3 (with Expo SDK36), and still had the problem, and that was fixed by importing RNGH at the entry point in the app. My app is perhaps particular in that it's like uber driver + uber customer in one app, so I require("./uberDriverApp") only after knowing the user type to avoid initializing js unnecessary, that may be a reason. |
Thanks @omnikron!
Can confirm, adding RN 0.59 |
I'll close this PR as from what I understand, RNGH events are whitelisted in RN core. |
hook up one NativeEventEmitter to whitelist our event types
This fixes #320