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

feat: new arch support on android #730

Merged
merged 24 commits into from Mar 17, 2023
Merged

feat: new arch support on android #730

merged 24 commits into from Mar 17, 2023

Conversation

vonovak
Copy link
Member

@vonovak vonovak commented Feb 25, 2023

Summary

this adds support for turbomodules on android

With this, the New Architecture is supported by this module

enabling the new architecture requires RN 0.71.4 or newer
if you don't use new arch I suppose RN 69 and newer is supported

Test Plan

Android e2e tests build and pass with new arch enabled

What's required for testing (prerequisites)?

What are the steps to reproduce (after prerequisites)?

Compatibility

OS Implemented
iOS
Android

Checklist

  • I have tested this on a device and a simulator
  • I added the documentation in README.md
  • I updated the typed files (TS and Flow)
  • I added a sample use of the API in the example project (example/App.js)
  • I have added automated tests, either in JS or e2e tests, as applicable

@vonovak vonovak changed the title Feat/new arch feat: new arch support on android Feb 25, 2023
@WoLewicki WoLewicki mentioned this pull request Mar 2, 2023
@vonovak vonovak marked this pull request as ready for review March 17, 2023 21:54
@vonovak vonovak merged commit 17c28e7 into master Mar 17, 2023
@vonovak vonovak deleted the feat/new-arch branch March 17, 2023 21:58
@vonovak vonovak mentioned this pull request Mar 17, 2023
5 tasks
@vonovak
Copy link
Member Author

vonovak commented Mar 18, 2023

🎉 This issue has been resolved in version 7.0.0 🎉

If this package helps you, consider sponsoring us! 🚀

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see you reverted the name of the file to include .android.js. It makes the codegen return wrong filenames and it does not compile on my machine with RN 0.70. Did you test it @vonovak ?

Copy link
Member Author

@vonovak vonovak Apr 3, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, I tested and e2e tests are passing - but with RN71. According to some comment I remember reading in discord, the .android suffix should not cause issues even with RN 70 but if it does, it's best to drop it. Feel free to open a PR, otherwise I'll get to it sometime next week. thank you 🙂

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@vonovak here you are: #748

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I could check on a clean project, but if we want to support the RN 0.70 and there are cases where it fails, it seems fine to just change it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants