ibitcy / react-native-hole-view

✂️ React-Native component to cut a touch-through holes anywhere you want. Perfect solution for tutorial overlay
395 stars 23 forks source link

fix: don't crash on iOS because of an `onClickBlock` in react-native 0.73.2 #31

Closed siddarthkay closed 8 months ago

siddarthkay commented 8 months ago

When upgrading react-native to 0.73.2 for our project we faced this crash which did not happen in react-native version 0.72.5 ref -> https://github.com/status-im/status-mobile/pull/18563#issuecomment-1907414008

This PR fixes that crash by modifying RNHoleViewImpl interface to accept setOnClick This is a very basic implementation just so that we avoid the crash without having to change the way we use react-native-hole-view in our codebase.

If you would like this to be done some other way I would be happy to incorporate any feedback you have.

Looking forward to your thoughts on this @stephenkopylov

stephenkopylov commented 8 months ago

Hi @siddarthkay and thanks for your attention to our library!

Frankly I don't think we really need this code to be merged - I don't see any reason why react-native-hole-view should handle any user interactions (gestures, clicks, etc)

My opinion - it's better to use patch-package to eliminate the problem until RN-team fix it

But if this setter (setOnClick) is a necessary part of any View-based module - we could merge it, but I can't find any mention about it in RN Modules documentation

siddarthkay commented 8 months ago

Hello! @stephenkopylov

Your reasoning sounds good, This could be a bug in RN or an undocumented change, I intend to create few examples of code that worked in react-native 0.72 which now crashes in react-native 0.73 with this error in javascript/typescript.

Screenshot 2024-01-24 at 9 43 25 AM

We have many examples in our project but they're in clojurescript and I am sure they would not be easy to understand for someone that does not write clojure.

ref -> https://github.com/status-im/status-mobile/blob/develop/src/quo/components/profile/profile_card/view.cljs#L39-L84

siddarthkay commented 8 months ago

more information on the crash is here : https://github.com/status-im/status-mobile/pull/18563#issuecomment-1911511881

This kind of crash happened because of usage of react-native-hole-view inside a TouchableWithoutFeedback component and the onPress of TouchableWithoutFeedback was causing the crash. We fixed it by swapping TouchableWithoutFeedback with Pressable component from react native.

This PR is un-necessary & hence closing. Thanks again @stephenkopylov for the good work on this library!