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

Add support for RN 0.71.0 #4

Merged
merged 1 commit into from
Mar 4, 2023
Merged

Conversation

mmmoussa
Copy link
Contributor

This PR is based on mrousavy/react-native-mmkv#480 and mrousavy/react-native-mmkv#499. Tested it in my own app and confirmed it's working. Closes #3.

@jahglow
Copy link

jahglow commented Feb 10, 2023

@mateioprea please merge this PR ASAP

@mateioprea
Copy link
Owner

Hi @mmmoussa and @jahglow. I've been away for the past week. Let me review this quickly and merge it.

@mateioprea
Copy link
Owner

Sorry, but your PR doesn't work on older versions of RN. You can test it with the example app. Do you want to fix these issues?

@jahglow
Copy link

jahglow commented Feb 13, 2023

't work on older versions of RN. You can test it with the example app.

@mmmoussa ?

@jahglow
Copy link

jahglow commented Feb 13, 2023

@mateioprea I'm not good at native code, maybe major upgrade will do the thing and have a division for pre 0.71 react and post? That's what is usually done to resolve backward incompatibility especially there were no changes to the lib for a year.

@a-eid
Copy link

a-eid commented Feb 25, 2023

@mateioprea

Sorry, but your PR doesn't work on older versions of RN. You can test it with the example app. Do you want to fix these issues?

I don't believe it's trivial to support 71 as well as prior RN versions, as most libs do you should note in the readme which versions support which RN version.

does this PR fixes this error ?
yes.

Screenshot 2023-02-25 at 13 56 27

I tried patching the package but it doesn't seem to be working.
patching the package worked.

@hirbod
Copy link

hirbod commented Feb 27, 2023

I can confirm that a lot of libs that support RN 71 have dropped support for older versions. I'd bump the major version and drop a deprecation warning. Users can still use a lower version when they need support for < 0.71 imho.

@a-eid
Copy link

a-eid commented Feb 27, 2023

@hirbod exactly.

@mateioprea
Copy link
Owner

I can confirm that a lot of libs that support RN 71 have dropped support for older versions. I'd bump the major version and drop a deprecation warning. Users can still use a lower version when they need support for < 0.71 imho.

@hirbod and that means that unless I’m using new arch, I won’t be able to get latest updates?

@hirbod
Copy link

hirbod commented Feb 27, 2023

No, @mateioprea, when you follow the PR like done in react-native-mmkv, or this open PR (which does base on the mmkv one) mrousavy/react-native-blob-jsi-helper#11 - your library will still work on Paper, but it requires RN 0.71.

The template and the build steps got much better and faster with 0.71 and supporting older versions is just tedious work.

@mateioprea
Copy link
Owner

Sorry, but this doesn't work for me. I get this error: java.lang.UnsatisfiedLinkError: couldn't find DSO to load: libhermes.so caused by: dlopen failed: cannot locate symbol

I don't know if its me or the new implementation, but i won't release this until i can run it successfully.

@mateioprea
Copy link
Owner

Found the issue. I'm gonna release this later today.

@mateioprea mateioprea merged commit 06c08eb into mateioprea:main Mar 4, 2023
@mateioprea
Copy link
Owner

mateioprea commented Mar 4, 2023

@mmmoussa @jahglow released under v2.0.0: https://www.npmjs.com/package/react-native-random-values-jsi-helper.

Thank you for your contributions! 🥇

@mmmoussa mmmoussa deleted the rn-0.71.0 branch March 16, 2023 15:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
5 participants