Skip to content
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: don't crash on iOS because of an onClickBlock in react-native 0.73.2 #31

Conversation

siddarthkay
Copy link

@siddarthkay siddarthkay commented Jan 24, 2024

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 -> status-im/status-mobile#18563 (comment)

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

@siddarthkay siddarthkay force-pushed the fix-nested-component-react-native-73 branch from 0f6cc81 to de2cf12 Compare January 24, 2024 15:23
@stephenkopylov
Copy link
Member

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
Copy link
Author

siddarthkay commented Jan 25, 2024

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
Copy link
Author

more information on the crash is here : status-im/status-mobile#18563 (comment)

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!

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.

2 participants