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: disable dummy codegen of rncore #488

Closed
wants to merge 1 commit into from

Conversation

Sunbreak
Copy link
Contributor

Summary

Without react.jsRootDir, react-native-gradle-plugin would generate rncore related code inside library's buildDir when newArchEnabled is true. And then duplicate class definition inside ReactAndroid like lottie-react-native/lottie-react-native#1053

This impacts library's compatiblitiy with 0.68/0.69/0.70 because of direct dependent on source code of ReactAndroid

Reproduce

  • Generate my-lib legacy module and initialize dependency
npx create-react-native-library cpp-lib --react-native-version 0.70.13
✔ What is the name of the npm package? … react-native-cpp-lib
✔ What is the description for the package? … C++ library
✔ What is the name of package author? … Sunbreak
✔ What is the email address for the package author?[email protected]
✔ What is the URL for the package author? … https://github.com/Sunbreak
✔ What is the URL for the repository? … https://github.com/Sunbreak/react-native-cpp-lib
✔ What type of library do you want to develop? › Native module
✔ Which languages do you want to use? › C++ for Android & iOS
ℹ Using [email protected] for the example
✔ Project created successfully at cpp-lib!

$ yarn
  • set newArchEnabled=true in example/andorid
  • compile android
$ yarn example android
...
ERROR:/Users/sunbreak/w/temp/cpp-lib/android/build/.transforms/bf9e6bfef630a6f77f22238c2792ba6f/transformed/classes/classes.dex: D8: Type com.facebook.react.viewmanagers.ActivityIndicatorViewManagerInterface is defined multiple times: /Users/sunbreak/w/temp/cpp-lib/android/build/.transforms/bf9e6bfef630a6f77f22238c2792ba6f/transformed/classes/classes.dex, /Users/sunbreak/w/temp/cpp-lib/example/node_modules/react-native/ReactAndroid/build/.transforms/eb9621efbac0f35e55ba3279bc90538a/transformed/classes/classes.dex

Test plan

@atlj
Copy link
Collaborator

atlj commented Nov 24, 2023

Hey, thanks a lot for the PR! From what I understand from the description, the legacy templates don't work with the new architecture. I feel like this should be the expected behavior since we have backward-compatible module templates that work with both architectures. Supporting the new architecture on legacy templates besides the latest React Native version does not make sense to me. Could you verify if my understanding is correct? 🙏

@Sunbreak
Copy link
Contributor Author

Yep. We could close this issue by design then?

@satya164 satya164 closed this Feb 2, 2024
@Sunbreak Sunbreak deleted the no-rncore-codegen branch February 5, 2024 03:32
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.

3 participants