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

React Native New Architecture support #1944

Closed
mrweiss opened this issue Sep 18, 2024 · 12 comments
Closed

React Native New Architecture support #1944

mrweiss opened this issue Sep 18, 2024 · 12 comments
Labels
enhancement New feature or request

Comments

@mrweiss
Copy link

mrweiss commented Sep 18, 2024

Feature Request

React Native new architecture compatibility - is this a plan? what are the main reasons the package is not compatible?

Why it is needed

It's the direction React Native is going forward https://reactnative.dev/docs/the-new-architecture/landing-page

(https://reactnative.directory/?search=gorhom%2Fbottom-sheet)

image

Possible implementation

A technical assessment would have to be done to align with New Architecture requirements

Code sample

Not possible until technical assessment

@mrweiss mrweiss added the enhancement New feature or request label Sep 18, 2024
@daves28
Copy link

daves28 commented Sep 19, 2024

The New Architecture is for Expo, not for React, but yes I agree a hugely popular library like this should consider becoming compatible.

@mrweiss mrweiss changed the title React New Architecture support React Native New Architecture support Sep 19, 2024
@mrweiss
Copy link
Author

mrweiss commented Sep 19, 2024

The New Architecture is for Expo, not for React, but yes I agree a hugely popular library like this should consider becoming compatible.

sorry my bad! changed the title to React Native New Architecture :) not just Expo, though!

@iliapnmrv
Copy link

Hey! This library does not have any native part so it does support New Architecture by default. I've just created a new app with new architecture and rn-bottom-sheet works fine for me

@daves28
Copy link

daves28 commented Sep 30, 2024

That makes sense. This site is incorrectly reporting that this library is incompatible then https://reactnative.directory/?search=bottom-sheet

@iliapnmrv
Copy link

react-native-community/directory#1198 this pr set value to false
@0xFA11 Hey! Are you sure this library does not support New Architecture?

@0xFA11
Copy link

0xFA11 commented Oct 1, 2024

I didn't test recently but I'd say please use this thread for proper testing on v5: #1823

Attempted fix didn't really fix it for me in the past (I still had issues with Expo51 + Reanimated3 + bottom sheet scrollview).

I'd suggest testing v5 thoroughly with all components, including bottomsheet scrollview etc and potentially using a simple reanimated3 based view with bottomsheet's exposed animIndex somewhere too.

other than that, if everything looks good then let's call it newArch-supported.

@mateoabrbt
Copy link

I just try it out it seems i little bit buggy even with v5 sometime the modal would just not open i'm gonna try to see what could cause that.

@qnrjs42
Copy link

qnrjs42 commented Oct 18, 2024

I just try it out it seems i little bit buggy even with v5 sometime the modal would just not open i'm gonna try to see what could cause that.

same.

@bahadiraraz
Copy link

Based on my testing results, bottom sheets are not functioning correctly in v5 when using React Native's new architecture - they simply don't open. While bottom sheet v4 is still working, it generates multiple console warnings during operation. The issue appears to be specifically related to the bottom sheet animation and mounting behavior in conjunction with the new architecture.

@mateoabrbt
Copy link

I would add that i create a custom component passing multiple children inside it and it simply didn't open but using a more simple approach works. It wasn't working for me on V4 also. I would suggest to try out the present method for opening the modal on new architecture.

@gorhom
Copy link
Owner

gorhom commented Oct 26, 2024

the library been tested on new arch and it is working , feel free to cut a another ticket if you discover new issues

@gorhom gorhom closed this as completed Oct 26, 2024
@the-simian
Copy link

the-simian commented Oct 28, 2024

@gorhom I still get

The following issues were found when validating your dependencies against React Native Directory:
Unsupported on New Architecture: @gorhom/bottom-sheet

with expo-doctor@latest,
do we need to update https://reactnative.directory/?search=bottom-sheet to say its compatible now?

edit: I've made a PR to update it react-native-community/directory#1340 so it reflects your testing

the-simian added a commit to the-simian/directory that referenced this issue Oct 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

9 participants