Skip to content

Conversation

erksch
Copy link
Contributor

@erksch erksch commented Sep 4, 2022

I found that applyNativeModulesAppBuildGradle in native_modules.gradle makes assumptions about the isNewArchitectureEnabled function being defined somehow, which in a normal setup is defined in the app/build.gradle of RN boilerplate. Although we may know the setup in which native_modules.gradle will be included there should be no such assumptions in the gradle script.

We use a build.gradle.kts instead of build.gradle which as far as I know does not allow for the magic exposure of the isNewArchitectureEnabled function to the native_modules.gradle script.

Replacing the function with a direct for "newArchEnabled".

Fixes #1697

@thymikee
Copy link
Member

thymikee commented Sep 5, 2022

@cortinico could you advise here please?

@erksch
Copy link
Contributor Author

erksch commented Sep 5, 2022

Another option would be to directly read newArchEnabled from the project properties.

@cortinico
Copy link
Member

Another option would be to directly read newArchEnabled from the project properties.

This is the preferred approach (and is actually the most robust one). The CLI is currently assuming the isNewArchitectureEnabled is defined, which could not be the case everywhere.
Reading the Gradle property should be the preferred way here.

@erksch
Copy link
Contributor Author

erksch commented Sep 6, 2022

Ok I'll implement that! It then will also not be a breaking change.

@thymikee
Copy link
Member

thymikee commented Sep 7, 2022

Thanks @cortinico @erksch !

@adamTrz
Copy link
Collaborator

adamTrz commented Sep 7, 2022

Think this should also fix #1697 💪

@erksch erksch force-pushed the new-architecture-flag-source branch 2 times, most recently from 9bc2af0 to 0b29193 Compare September 7, 2022 11:02
@erksch erksch force-pushed the new-architecture-flag-source branch from 0b29193 to 74485f0 Compare September 7, 2022 11:02
@erksch erksch changed the title Move isNewArchitectureEnabled flag to applyNativeModulesAppBuildGradle args Remove dependency on existence of isNewArchitectureEnabled() Sep 7, 2022
@erksch
Copy link
Contributor Author

erksch commented Sep 7, 2022

@adamTrz not exactly a fix :D people should still configure their build.gradle correctly when updating. But now at least the native_modules.gradle script will not care.

@adamTrz
Copy link
Collaborator

adamTrz commented Sep 7, 2022

@adamTrz not exactly a fix :D people should still configure their build.gradle correctly when updating. But now at least the native_modules.gradle script will not care.

Yeah, but at least it shouldn't crash the build :)

@thymikee thymikee merged commit dda08b3 into react-native-community:main Sep 7, 2022
@thymikee
Copy link
Member

thymikee commented Sep 7, 2022

Published v9.0.1 with this. Please check the upgrade guide for the @react-native-community/cli or invalidate the whole lock file.

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.

Build failed on Android after upgrading to react-native@0.70.0

4 participants