search

maxkomarychev / react-native-ultimate-config

Config that works
MIT License
260 stars 31 forks source link

feat: support for both react-native >= 0.71 and < 0.71 versions #128

Closed vagnerlandio closed 1 year ago

vagnerlandio commented 1 year ago

Hi @maxkomarychev ,

I've made some changes to the react-native-ultimate-config package that I would like to submit for review 🤝 . I've added support for the new react-native 0.71 🚀 and also fixed an issue with the maven publishing not working properly 🔧.

I've also included extras contributor notes, example updated with react-native 0.71, example-web updated with reactjs 18.2 and test cases to ensure that these changes do not break any existing functionality. I've tested these changes on multiple devices and it's working as expected 💻📱.

I think these changes would be a valuable addition to the package and I would appreciate it if it could be reviewed and merged 🙏.

Thank you! 🤗

maxkomarychev commented 1 year ago

Impressive. I need some time to digest this. thanks!

maxkomarychev commented 1 year ago

I've tested these changes on multiple devices and it's working as expected 💻📱.

could you please provide more details on how you have actually tested it?

thanks!

vagnerlandio commented 1 year ago

could you please provide more details on how you have actually tested it?

I tested the example app on iOS 16 Simulator and on Android 13 Emulator.

I also added the package with these changes in a private app that is running in production (running react-native 0.70.6, pnpm workspaces with react-native and react-native-ultimate-config in the root of the monorepo node_modules and sharing dependencies between projects)

  • what versions of ios/android you have used?

Tested by installing the package on android app with minSdkVersion 23, targetSdkVersion 31 and installed on physical devices running android 10, 11 and 12

  • have you used new architecture?

in the example project configured with newArchEnabled=true in gradle.properties execute the app without problems.

In the submitted code, despite having configured the android and ios folders adding support for the new architecture with backward-compatibility, is recommended convert the js files to typescript or flow and implement the TurboModule instead of the NativeModule methods.

maxkomarychev commented 1 year ago

@vagnerlandio this looks legit and I'm going to merge it. Thank you for your contribution!!

maxkomarychev commented 1 year ago

@vagnerlandio your change has been pushed as version 4.1.0-alpha.0 .

This is rather massive change and I am a bit hesitating to push it as a real version for now. I would give it a week time to get some testing (on my end as well).

That said: the amount of work you have done is awesome. Thank you!! <3

vagnerlandio commented 1 year ago

Thank you very much @maxkomarychev

An alpha version is great, I could leave it in alpha even for more than a single week. the more people who test it will be even better.

We are very grateful to you for creating this excellent tool.

maxkomarychev commented 1 year ago

FYI this change has introduced a breaking change for monorepo - the app I'm working on required extra set up. I have updated docs and pushed new version - 5 (pls see latest commits). Changelog entry: https://github.com/maxkomarychev/react-native-ultimate-config/blob/master/packages/react-native-ultimate-config/CHANGELOG.md#500-2023-01-26