From 7d61b9d81a2b99b50a00ecd5f8ac91f85cdce8ce Mon Sep 17 00:00:00 2001 From: Nicola Corti Date: Wed, 26 Oct 2022 10:41:02 -0700 Subject: [PATCH] RNGP - Remove enableCodegenInApps and infer it from package.json (#35083) Summary: Pull Request resolved: https://github.com/facebook/react-native/pull/35083 We don't need to `enableCodegenInApps` keys but we can instead rely on the existence of the `codegenConfig` key inside the `package.json` to decide if Codegen should run in App modules or not. Changelog: [Internal] [Changed] - RNGP - Remove enableCodegenInApps and infer it from package.json Reviewed By: cipolleschi Differential Revision: D40687079 fbshipit-source-id: cd4a6c67caa19c1d199ae75388a0551339f876a0 --- .../com/facebook/react/ReactExtension.kt | 7 --- .../kotlin/com/facebook/react/ReactPlugin.kt | 17 +++-- .../com/facebook/react/utils/PathUtils.kt | 14 +++++ .../com/facebook/react/utils/ProjectUtils.kt | 11 ++++ .../com/facebook/react/utils/PathUtilsTest.kt | 57 +++++++++++++++++ .../facebook/react/utils/ProjectUtilsTest.kt | 62 +++++++++++++++++++ packages/rn-tester/android/app/build.gradle | 1 - 7 files changed, 157 insertions(+), 12 deletions(-) diff --git a/packages/react-native-gradle-plugin/src/main/kotlin/com/facebook/react/ReactExtension.kt b/packages/react-native-gradle-plugin/src/main/kotlin/com/facebook/react/ReactExtension.kt index 4e757387654..08350da88d9 100644 --- a/packages/react-native-gradle-plugin/src/main/kotlin/com/facebook/react/ReactExtension.kt +++ b/packages/react-native-gradle-plugin/src/main/kotlin/com/facebook/react/ReactExtension.kt @@ -156,11 +156,4 @@ abstract class ReactExtension @Inject constructor(project: Project) { */ val codegenJavaPackageName: Property = objects.property(String::class.java).convention("com.facebook.fbreact.specs") - - /** - * Toggles the Codegen for App modules. By default we enable the codegen only for library modules. - * If you want to enable codegen in your app, you will have to set this to true. Default: false - */ - val enableCodegenInApps: Property = - objects.property(Boolean::class.java).convention(false) } diff --git a/packages/react-native-gradle-plugin/src/main/kotlin/com/facebook/react/ReactPlugin.kt b/packages/react-native-gradle-plugin/src/main/kotlin/com/facebook/react/ReactPlugin.kt index 24790daa83c..8538eaa9941 100644 --- a/packages/react-native-gradle-plugin/src/main/kotlin/com/facebook/react/ReactPlugin.kt +++ b/packages/react-native-gradle-plugin/src/main/kotlin/com/facebook/react/ReactPlugin.kt @@ -19,6 +19,7 @@ import com.facebook.react.utils.DependencyUtils.configureRepositories import com.facebook.react.utils.DependencyUtils.readVersionString import com.facebook.react.utils.JsonUtils import com.facebook.react.utils.NdkConfiguratorUtils.configureReactNativeNdk +import com.facebook.react.utils.ProjectUtils.needsCodegenFromPackageJson import com.facebook.react.utils.findPackageJsonFile import java.io.File import kotlin.system.exitProcess @@ -91,7 +92,11 @@ class ReactPlugin : Plugin { it.codegenDir.set(extension.codegenDir) val bashWindowsHome = project.findProperty("REACT_WINDOWS_BASH") as String? it.bashWindowsHome.set(bashWindowsHome) - it.onlyIf { isLibrary || extension.enableCodegenInApps.get() } + + // Please note that appNeedsCodegen is triggering a read of the package.json at + // configuration time as we need to feed the onlyIf condition of this task. + // Therefore, the appNeedsCodegen needs to be invoked inside this lambda. + it.onlyIf { isLibrary || project.needsCodegenFromPackageJson(extension) } } // We create the task to produce schema from JS files. @@ -102,10 +107,9 @@ class ReactPlugin : Plugin { it.nodeExecutableAndArgs.set(extension.nodeExecutableAndArgs) it.codegenDir.set(extension.codegenDir) it.generatedSrcDir.set(generatedSrcDir) - it.onlyIf { isLibrary || extension.enableCodegenInApps.get() } // We're reading the package.json at configuration time to properly feed - // the `jsRootDir` @Input property of this task. Therefore, the + // the `jsRootDir` @Input property of this task & the onlyIf. Therefore, the // parsePackageJson should be invoked inside this lambda. val packageJson = findPackageJsonFile(project, extension) val parsedPackageJson = packageJson?.let { JsonUtils.fromCodegenJson(it) } @@ -116,6 +120,7 @@ class ReactPlugin : Plugin { } else { it.jsRootDir.set(extension.jsRootDir) } + it.onlyIf { isLibrary || project.needsCodegenFromPackageJson(parsedPackageJson) } } // We create the task to generate Java code from schema. @@ -130,7 +135,11 @@ class ReactPlugin : Plugin { it.packageJsonFile.set(findPackageJsonFile(project, extension)) it.codegenJavaPackageName.set(extension.codegenJavaPackageName) it.libraryName.set(extension.libraryName) - it.onlyIf { isLibrary || extension.enableCodegenInApps.get() } + + // Please note that appNeedsCodegen is triggering a read of the package.json at + // configuration time as we need to feed the onlyIf condition of this task. + // Therefore, the appNeedsCodegen needs to be invoked inside this lambda. + it.onlyIf { isLibrary || project.needsCodegenFromPackageJson(extension) } } // We update the android configuration to include the generated sources. diff --git a/packages/react-native-gradle-plugin/src/main/kotlin/com/facebook/react/utils/PathUtils.kt b/packages/react-native-gradle-plugin/src/main/kotlin/com/facebook/react/utils/PathUtils.kt index b7e039909ea..41130173eda 100644 --- a/packages/react-native-gradle-plugin/src/main/kotlin/com/facebook/react/utils/PathUtils.kt +++ b/packages/react-native-gradle-plugin/src/main/kotlin/com/facebook/react/utils/PathUtils.kt @@ -10,6 +10,7 @@ package com.facebook.react.utils import com.facebook.react.ReactExtension +import com.facebook.react.model.ModelPackageJson import java.io.File import org.gradle.api.Project @@ -206,6 +207,19 @@ internal fun findPackageJsonFile(project: Project, extension: ReactExtension): F extension.root.file("package.json").orNull?.asFile } +/** + * Function to look for the `package.json` and parse it. It returns a [ModelPackageJson] if found or + * null others. + * + * Please note that this function access the [ReactExtension] field properties and calls .get() on + * them, so calling this during apply() of the ReactPlugin is not recommended. It should be invoked + * inside lazy lambdas or at execution time. + */ +internal fun readPackageJsonFile(project: Project, extension: ReactExtension): ModelPackageJson? { + val packageJson = findPackageJsonFile(project, extension) + return packageJson?.let { JsonUtils.fromCodegenJson(it) } +} + private const val HERMESC_IN_REACT_NATIVE_DIR = "node_modules/react-native/sdks/hermesc/%OS-BIN%/" private const val HERMESC_BUILT_FROM_SOURCE_DIR = "node_modules/react-native/ReactAndroid/hermes-engine/build/hermes/bin/" diff --git a/packages/react-native-gradle-plugin/src/main/kotlin/com/facebook/react/utils/ProjectUtils.kt b/packages/react-native-gradle-plugin/src/main/kotlin/com/facebook/react/utils/ProjectUtils.kt index f70e14cbcca..c18946a0cd0 100644 --- a/packages/react-native-gradle-plugin/src/main/kotlin/com/facebook/react/utils/ProjectUtils.kt +++ b/packages/react-native-gradle-plugin/src/main/kotlin/com/facebook/react/utils/ProjectUtils.kt @@ -7,6 +7,8 @@ package com.facebook.react.utils +import com.facebook.react.ReactExtension +import com.facebook.react.model.ModelPackageJson import org.gradle.api.Project internal object ProjectUtils { @@ -32,4 +34,13 @@ internal object ProjectUtils { } else { HERMES_FALLBACK } + + internal fun Project.needsCodegenFromPackageJson(extension: ReactExtension): Boolean { + val parsedPackageJson = readPackageJsonFile(this, extension) + return needsCodegenFromPackageJson(parsedPackageJson) + } + + internal fun Project.needsCodegenFromPackageJson(model: ModelPackageJson?): Boolean { + return model?.codegenConfig != null + } } diff --git a/packages/react-native-gradle-plugin/src/test/kotlin/com/facebook/react/utils/PathUtilsTest.kt b/packages/react-native-gradle-plugin/src/test/kotlin/com/facebook/react/utils/PathUtilsTest.kt index 784d596a89d..b0f7474cd23 100644 --- a/packages/react-native-gradle-plugin/src/test/kotlin/com/facebook/react/utils/PathUtilsTest.kt +++ b/packages/react-native-gradle-plugin/src/test/kotlin/com/facebook/react/utils/PathUtilsTest.kt @@ -13,6 +13,7 @@ import com.facebook.react.tests.OS import com.facebook.react.tests.OsRule import com.facebook.react.tests.WithOs import java.io.File +import java.io.FileNotFoundException import org.gradle.testfixtures.ProjectBuilder import org.junit.Assert.* import org.junit.Assume.assumeTrue @@ -299,4 +300,60 @@ class PathUtilsTest { assertEquals(localFile, findPackageJsonFile(project, extension)) } + + @Test(expected = FileNotFoundException::class) + fun readPackageJsonFile_withMissingFile_returnsNull() { + val moduleFolder = tempFolder.newFolder("awesome-module") + val project = ProjectBuilder.builder().withProjectDir(moduleFolder).build() + project.plugins.apply("com.android.library") + project.plugins.apply("com.facebook.react") + val extension = + project.extensions.getByType(ReactExtension::class.java).apply { root.set(moduleFolder) } + + val actual = readPackageJsonFile(project, extension) + + assertNull(actual) + } + + @Test + fun readPackageJsonFile_withFileConfiguredInExtension_andMissingCodegenConfig_returnsNullCodegenConfig() { + val moduleFolder = tempFolder.newFolder("awesome-module") + File(moduleFolder, "package.json").apply { writeText("{}") } + val project = ProjectBuilder.builder().withProjectDir(moduleFolder).build() + project.plugins.apply("com.android.library") + project.plugins.apply("com.facebook.react") + val extension = + project.extensions.getByType(ReactExtension::class.java).apply { root.set(moduleFolder) } + + val actual = readPackageJsonFile(project, extension) + + assertNotNull(actual) + assertNull(actual!!.codegenConfig) + } + + @Test + fun readPackageJsonFile_withFileConfiguredInExtension_andHavingCodegenConfig_returnsValidCodegenConfig() { + val moduleFolder = tempFolder.newFolder("awesome-module") + File(moduleFolder, "package.json").apply { + writeText( + // language=json + """ + { + "name": "a-library", + "codegenConfig": {} + } + """ + .trimIndent()) + } + val project = ProjectBuilder.builder().withProjectDir(moduleFolder).build() + project.plugins.apply("com.android.library") + project.plugins.apply("com.facebook.react") + val extension = + project.extensions.getByType(ReactExtension::class.java).apply { root.set(moduleFolder) } + + val actual = readPackageJsonFile(project, extension) + + assertNotNull(actual) + assertNotNull(actual!!.codegenConfig) + } } diff --git a/packages/react-native-gradle-plugin/src/test/kotlin/com/facebook/react/utils/ProjectUtilsTest.kt b/packages/react-native-gradle-plugin/src/test/kotlin/com/facebook/react/utils/ProjectUtilsTest.kt index c41cc65a27c..d918c285d1c 100644 --- a/packages/react-native-gradle-plugin/src/test/kotlin/com/facebook/react/utils/ProjectUtilsTest.kt +++ b/packages/react-native-gradle-plugin/src/test/kotlin/com/facebook/react/utils/ProjectUtilsTest.kt @@ -7,15 +7,24 @@ package com.facebook.react.utils +import com.facebook.react.TestReactExtension +import com.facebook.react.model.ModelCodegenConfig +import com.facebook.react.model.ModelPackageJson import com.facebook.react.tests.createProject import com.facebook.react.utils.ProjectUtils.isHermesEnabled import com.facebook.react.utils.ProjectUtils.isNewArchEnabled +import com.facebook.react.utils.ProjectUtils.needsCodegenFromPackageJson +import java.io.File import org.junit.Assert.assertFalse import org.junit.Assert.assertTrue +import org.junit.Rule import org.junit.Test +import org.junit.rules.TemporaryFolder class ProjectUtilsTest { + @get:Rule val tempFolder = TemporaryFolder() + @Test fun isNewArchEnabled_returnsFalseByDefault() { assertFalse(createProject().isNewArchEnabled) @@ -99,4 +108,57 @@ class ProjectUtilsTest { project.extensions.extraProperties.set("react", extMap) assertTrue(project.isHermesEnabled) } + + @Test + fun needsCodegenFromPackageJson_withCodegenConfigInPackageJson_returnsTrue() { + val project = createProject() + val extension = TestReactExtension(project) + File(tempFolder.root, "package.json").apply { + writeText( + // language=json + """ + { + "name": "a-library", + "codegenConfig": {} + } + """ + .trimIndent()) + } + extension.root.set(tempFolder.root) + assertTrue(project.needsCodegenFromPackageJson(extension)) + } + + @Test + fun needsCodegenFromPackageJson_withMissingCodegenConfigInPackageJson_returnsFalse() { + val project = createProject() + val extension = TestReactExtension(project) + File(tempFolder.root, "package.json").apply { + writeText( + // language=json + """ + { + "name": "a-library" + } + """ + .trimIndent()) + } + extension.root.set(tempFolder.root) + assertFalse(project.needsCodegenFromPackageJson(extension)) + } + + @Test + fun needsCodegenFromPackageJson_withCodegenConfigInModel_returnsTrue() { + val project = createProject() + val model = ModelPackageJson(ModelCodegenConfig(null, null, null, null)) + + assertTrue(project.needsCodegenFromPackageJson(model)) + } + + @Test + fun needsCodegenFromPackageJson_withMissingCodegenConfigInModel_returnsFalse() { + val project = createProject() + val model = ModelPackageJson(null) + + assertFalse(project.needsCodegenFromPackageJson(model)) + } } diff --git a/packages/rn-tester/android/app/build.gradle b/packages/rn-tester/android/app/build.gradle index e0fc0dcf655..c26d4444cf3 100644 --- a/packages/rn-tester/android/app/build.gradle +++ b/packages/rn-tester/android/app/build.gradle @@ -88,7 +88,6 @@ react { // Codegen Configs reactNativeDir = rootDir codegenDir = new File(rootDir, "node_modules/react-native-codegen") - enableCodegenInApps = true } /**