From 57cc524e9345c418283d031d0a7f2ed1c0cdfc49 Mon Sep 17 00:00:00 2001 From: Nicola Corti Date: Fri, 18 Feb 2022 04:31:01 -0800 Subject: [PATCH] Gradle: Deprecate `reactRoot` in favor of `root` and `reactNativeDir` (#33142) Summary: Pull Request resolved: https://github.com/facebook/react-native/pull/33142 The `reactRoot` property was confusing as we were using it for both the root of the project and the react-native NPM Package root. I'm deprecating it and splitting it in two. I've added several warning in the codebase to tell the people how to migrate away from it. Moreover this is specifying default values that are more user-friendly. Users won't have to configure anything unless they are in a monorepo. Changelog: [Android] [Changed] - Gradle: Deprecate `reactRoot` in favor of `root` and `reactNativeDir` Reviewed By: ShikaSD Differential Revision: D34277050 fbshipit-source-id: fc7f45017452b086726516a9586cacd9a661c287 --- ReactAndroid/build.gradle | 2 +- .../com/facebook/react/ReactExtension.kt | 59 ++++++++++++++----- .../kotlin/com/facebook/react/ReactPlugin.kt | 28 +++++---- .../com/facebook/react/TaskConfiguration.kt | 9 +-- .../tasks/GenerateCodegenArtifactsTask.kt | 32 +++++++++- .../com/facebook/react/utils/PathUtils.kt | 5 +- .../tasks/GenerateCodegenArtifactsTaskTest.kt | 6 +- .../com/facebook/react/utils/PathUtilsTest.kt | 6 +- packages/rn-tester/android/app/build.gradle | 3 +- 9 files changed, 104 insertions(+), 46 deletions(-) diff --git a/ReactAndroid/build.gradle b/ReactAndroid/build.gradle index 3e0f804219c..5a384f23c12 100644 --- a/ReactAndroid/build.gradle +++ b/ReactAndroid/build.gradle @@ -406,7 +406,7 @@ react { // This should be changed to a more generic name, e.g. `ReactCoreSpec`. libraryName = "rncore" jsRootDir = file("../Libraries") - reactRoot = file("$projectDir/..") + reactNativeDir = file("$projectDir/..") useJavaGenerator = System.getenv("USE_CODEGEN_JAVAPOET")?.toBoolean() ?: false // We search for the codegen in either one of the `node_modules` folder or in the // root packages folder (that's for when we build from source without calling `yarn install`). 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 5c72bdb9838..7f1d914de46 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 @@ -29,13 +29,13 @@ abstract class ReactExtension @Inject constructor(project: Project) { val applyAppPlugin: Property = objects.property(Boolean::class.java).convention(false) /** - * The path to the react root folder. This is the path to the root folder where the `node_modules` - * folder is present. All the CLI commands will be invoked from this folder as working directory. + * The path to the root of your project. This is the path to where the `package.json` lives. All + * the CLI commands will be invoked from this folder as working directory. * - * Default: $projectDir/../../ + * Default: ${rootProject.dir}/../ */ - val reactRoot: DirectoryProperty = - objects.directoryProperty().convention(project.layout.projectDirectory.dir("../../")) + val root: DirectoryProperty = + objects.directoryProperty().convention(project.rootProject.layout.projectDirectory.dir("../")) /** * The path to the JS entry file. If not specified, the plugin will try to resolve it using a list @@ -45,7 +45,7 @@ abstract class ReactExtension @Inject constructor(project: Project) { /** * The path to the React Native CLI. If not specified, the plugin will try to resolve it looking - * for `react-native` CLI inside `node_modules` in [reactRoot]. + * for `react-native` CLI inside `node_modules` in [root]. */ val cliPath: Property = objects.property(String::class.java) @@ -56,9 +56,7 @@ abstract class ReactExtension @Inject constructor(project: Project) { val nodeExecutableAndArgs: ListProperty = objects.listProperty(String::class.java).convention(listOf("node")) - /** - * The command to use to invoke bundle. Default is `bundle` and will be invoked on [reactRoot]. - */ + /** The command to use to invoke bundle. Default is `bundle` and will be invoked on [root]. */ val bundleCommand: Property = objects.property(String::class.java).convention("bundle") /** @@ -190,19 +188,27 @@ abstract class ReactExtension @Inject constructor(project: Project) { /** Codegen Config */ /** - * The path to the react-native-codegen folder. + * The path to the react-native-codegen NPM package folder. * - * Default: $projectDir/../../node_modules/react-native-codegen + * Default: ${rootProject.dir}/../node_modules/react-native-codegen */ val codegenDir: DirectoryProperty = - objects.directoryProperty().convention(reactRoot.dir("node_modules/react-native-codegen")) + objects.directoryProperty().convention(root.dir("node_modules/react-native-codegen")) + + /** + * The path to the react-native NPM package folder. + * + * Default: ${rootProject.dir}/../node_modules/react-native-codegen + */ + val reactNativeDir: DirectoryProperty = + objects.directoryProperty().convention(root.dir("node_modules/react-native")) /** * The root directory for all JS files for the app. * - * Default: $projectDir/../../ + * Default: [root] (i.e. ${rootProject.dir}/../) */ - val jsRootDir: DirectoryProperty = objects.directoryProperty().convention(reactRoot.get()) + val jsRootDir: DirectoryProperty = objects.directoryProperty().convention(root.get()) /** * The library name that will be used for the codegen artifacts. @@ -222,4 +228,29 @@ abstract class ReactExtension @Inject constructor(project: Project) { /** Whether the Java Generator (based on Javapoet) should be used or not. Default: false */ val useJavaGenerator: Property = objects.property(Boolean::class.java).convention(false) + + /** + * The `reactRoot` property was confusing and should not be used. + * + * You should instead use either: + * - [root] to point to your root project (where the package.json lives) + * - [reactNativeDir] to point to the NPM package of react native. + * + * A valid configuration would look like: + * + * ``` + * react { + * root = rootProject.file("..") + * reactNativeDir = rootProject.file("../node_modules/react-native") + * } + * ``` + * + * Please also note that those are the default value and you most likely don't need those at all. + */ + @Deprecated( + "reactRoot was confusing and has been replace with root" + + "to point to your root project and reactNativeDir to point to " + + "the folder of the react-native NPM package", + replaceWith = ReplaceWith("reactNativeRoot")) + val reactRoot: DirectoryProperty = objects.directoryProperty() } 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 9f8759f0dbc..42490044ba8 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 @@ -23,25 +23,26 @@ import org.gradle.internal.jvm.Jvm class ReactPlugin : Plugin { override fun apply(project: Project) { - checkJvmVersion() + checkJvmVersion(project) val extension = project.extensions.create("react", ReactExtension::class.java, project) applyAppPlugin(project, extension) applyCodegenPlugin(project, extension) } - private fun checkJvmVersion() { + private fun checkJvmVersion(project: Project) { val jvmVersion = Jvm.current()?.javaVersion?.majorVersion if ((jvmVersion?.toIntOrNull() ?: 0) <= 8) { - println("\n\n\n") - println( - "**************************************************************************************************************") - println("\n\n") - println("ERROR: requires JDK11 or higher.") - println("Incompatible major version detected: '" + jvmVersion + "'") - println("\n\n") - println( - "**************************************************************************************************************") - println("\n\n\n") + project.logger.error( + """ + + ******************************************************************************** + + ERROR: requires JDK11 or higher. + Incompatible major version detected: '$jvmVersion' + + ******************************************************************************** + + """.trimIndent()) exitProcess(1) } } @@ -95,7 +96,8 @@ class ReactPlugin : Plugin { project.tasks.register( "generateCodegenArtifactsFromSchema", GenerateCodegenArtifactsTask::class.java) { it.dependsOn(generateCodegenSchemaTask) - it.reactRoot.set(extension.reactRoot) + it.reactNativeDir.set(extension.reactNativeDir) + it.deprecatedReactRoot.set(extension.reactRoot) it.nodeExecutableAndArgs.set(extension.nodeExecutableAndArgs) it.codegenDir.set(extension.codegenDir) it.useJavaGenerator.set(extension.useJavaGenerator) diff --git a/packages/react-native-gradle-plugin/src/main/kotlin/com/facebook/react/TaskConfiguration.kt b/packages/react-native-gradle-plugin/src/main/kotlin/com/facebook/react/TaskConfiguration.kt index 30625c53d37..4165d858458 100644 --- a/packages/react-native-gradle-plugin/src/main/kotlin/com/facebook/react/TaskConfiguration.kt +++ b/packages/react-native-gradle-plugin/src/main/kotlin/com/facebook/react/TaskConfiguration.kt @@ -17,7 +17,6 @@ import com.facebook.react.utils.detectedCliPath import com.facebook.react.utils.detectedEntryFile import com.facebook.react.utils.detectedHermesCommand import java.io.File -import java.util.* import org.gradle.api.Project import org.gradle.api.Task import org.gradle.api.tasks.Copy @@ -56,11 +55,9 @@ internal fun Project.configureReactTasks(variant: BaseVariant, config: ReactExte it.group = REACT_GROUP it.description = "create JS bundle and assets for $targetName." - it.reactRoot = config.reactRoot.get().asFile + it.reactRoot = config.root.get().asFile it.sources = - fileTree(config.reactRoot) { fileTree -> - fileTree.setExcludes(config.inputExcludes.get()) - } + fileTree(config.root) { fileTree -> fileTree.setExcludes(config.inputExcludes.get()) } it.execCommand = execCommand it.bundleCommand = config.bundleCommand.get() it.devEnabled = !config.disableDevForVariant(variant) @@ -98,7 +95,7 @@ internal fun Project.configureReactTasks(variant: BaseVariant, config: ReactExte it.group = REACT_GROUP it.description = "bundle hermes resources for $targetName" - it.reactRoot = config.reactRoot.get().asFile + it.reactRoot = config.root.get().asFile it.hermesCommand = detectedHermesCommand(config) it.hermesFlags = config.hermesFlagsForVariant(variant) it.jsBundleFile = jsBundleFile diff --git a/packages/react-native-gradle-plugin/src/main/kotlin/com/facebook/react/tasks/GenerateCodegenArtifactsTask.kt b/packages/react-native-gradle-plugin/src/main/kotlin/com/facebook/react/tasks/GenerateCodegenArtifactsTask.kt index e44f72954c1..f0bda7737be 100644 --- a/packages/react-native-gradle-plugin/src/main/kotlin/com/facebook/react/tasks/GenerateCodegenArtifactsTask.kt +++ b/packages/react-native-gradle-plugin/src/main/kotlin/com/facebook/react/tasks/GenerateCodegenArtifactsTask.kt @@ -20,7 +20,7 @@ import org.gradle.api.tasks.* abstract class GenerateCodegenArtifactsTask : Exec() { - @get:Internal abstract val reactRoot: DirectoryProperty + @get:Internal abstract val reactNativeDir: DirectoryProperty @get:Internal abstract val codegenDir: DirectoryProperty @@ -34,6 +34,9 @@ abstract class GenerateCodegenArtifactsTask : Exec() { @get:Input abstract val libraryName: Property + // We're keeping this just to fire a warning at the user should they use the `reactRoot` property. + @get:Internal abstract val deprecatedReactRoot: DirectoryProperty + @get:InputFile val combineJsToSchemaCli: Provider = codegenDir.file("lib/cli/combine/combine-js-to-schema-cli.js") @@ -46,6 +49,7 @@ abstract class GenerateCodegenArtifactsTask : Exec() { @get:OutputDirectory val generatedJniFiles: Provider = generatedSrcDir.dir("jni") override fun exec() { + checkForDeprecatedProperty() setupCommandLine() super.exec() if (useJavaGenerator.getOrElse(false)) { @@ -63,11 +67,35 @@ abstract class GenerateCodegenArtifactsTask : Exec() { } } + private fun checkForDeprecatedProperty() { + if (deprecatedReactRoot.isPresent) { + project.logger.error( + """ + ******************************************************************************** + The `reactRoot` property is deprecated and will be removed in + future versions of React Native. The property is currently ignored. + + You should instead use either: + - [root] to point to your root project (where the package.json lives) + - [reactNativeDir] to point to the NPM package of react native. + + You should be fine by just removing the `reactRoot` line entirely from + your build.gradle file. Otherwise a valid configuration would look like: + + react { + root = rootProject.file('..') + reactNativeDir = rootProject.file('../node_modules/react-native') + } + ******************************************************************************** + """.trimIndent()) + } + } + internal fun setupCommandLine() { commandLine( windowsAwareYarn( *nodeExecutableAndArgs.get().toTypedArray(), - reactRoot.file("scripts/generate-specs-cli.js").get().asFile.absolutePath, + reactNativeDir.file("scripts/generate-specs-cli.js").get().asFile.absolutePath, "--platform", "android", "--schemaPath", 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 eb965bc0602..302c1d83a7b 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 @@ -11,7 +11,6 @@ package com.facebook.react.utils import com.facebook.react.ReactExtension import java.io.File -import java.util.* import org.apache.tools.ant.taskdefs.condition.Os /** @@ -25,7 +24,7 @@ import org.apache.tools.ant.taskdefs.condition.Os */ internal fun detectedEntryFile(config: ReactExtension): File = detectEntryFile( - entryFile = config.entryFile.orNull?.asFile, reactRoot = config.reactRoot.get().asFile) + entryFile = config.entryFile.orNull?.asFile, reactRoot = config.root.get().asFile) /** * Computes the CLI location for React Native. The Algo follows this order: @@ -40,7 +39,7 @@ internal fun detectedCliPath( ): String = detectCliPath( projectDir = projectDir, - reactRoot = config.reactRoot.get().asFile, + reactRoot = config.root.get().asFile, preconfiguredCliPath = config.cliPath.orNull) /** diff --git a/packages/react-native-gradle-plugin/src/test/kotlin/com/facebook/react/tasks/GenerateCodegenArtifactsTaskTest.kt b/packages/react-native-gradle-plugin/src/test/kotlin/com/facebook/react/tasks/GenerateCodegenArtifactsTaskTest.kt index 81c40375c6d..900086e43c9 100644 --- a/packages/react-native-gradle-plugin/src/test/kotlin/com/facebook/react/tasks/GenerateCodegenArtifactsTaskTest.kt +++ b/packages/react-native-gradle-plugin/src/test/kotlin/com/facebook/react/tasks/GenerateCodegenArtifactsTaskTest.kt @@ -79,13 +79,13 @@ class GenerateCodegenArtifactsTaskTest { @Test @WithOs(OS.UNIX) fun setupCommandLine_withoutJavaGenerator_willSetupCorrectly() { - val reactRoot = tempFolder.newFolder("node_modules/react-native/") + val reactNativeDir = tempFolder.newFolder("node_modules/react-native/") val codegenDir = tempFolder.newFolder("codegen") val outputDir = tempFolder.newFolder("output") val task = createTestTask { - it.reactRoot.set(reactRoot) + it.reactNativeDir.set(reactNativeDir) it.codegenDir.set(codegenDir) it.generatedSrcDir.set(outputDir) it.nodeExecutableAndArgs.set(listOf("--verbose")) @@ -99,7 +99,7 @@ class GenerateCodegenArtifactsTaskTest { listOf( "yarn", "--verbose", - File(reactRoot, "scripts/generate-specs-cli.js").toString(), + File(reactNativeDir, "scripts/generate-specs-cli.js").toString(), "--platform", "android", "--schemaPath", 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 c87d5fa3c6e..debba45740b 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 @@ -33,7 +33,7 @@ class PathUtilsTest { @Test fun detectedEntryFile_withAndroidEntryPoint() { val extension = TestReactExtension(ProjectBuilder.builder().build()) - extension.reactRoot.set(tempFolder.root) + extension.root.set(tempFolder.root) tempFolder.newFile("index.android.js") val actual = detectedEntryFile(extension) @@ -44,7 +44,7 @@ class PathUtilsTest { @Test fun detectedEntryFile_withDefaultEntryPoint() { val extension = TestReactExtension(ProjectBuilder.builder().build()) - extension.reactRoot.set(tempFolder.root) + extension.root.set(tempFolder.root) val actual = detectedEntryFile(extension) @@ -80,7 +80,7 @@ class PathUtilsTest { fun detectedCliPath_withCliFromNodeModules() { val project = ProjectBuilder.builder().build() val extension = TestReactExtension(project) - extension.reactRoot.set(tempFolder.root) + extension.root.set(tempFolder.root) val expected = File(tempFolder.root, "node_modules/react-native/cli.js").apply { parentFile.mkdirs() diff --git a/packages/rn-tester/android/app/build.gradle b/packages/rn-tester/android/app/build.gradle index 648b4d88580..07e581e9ba4 100644 --- a/packages/rn-tester/android/app/build.gradle +++ b/packages/rn-tester/android/app/build.gradle @@ -80,7 +80,7 @@ react { cliPath = "../../../../cli.js" bundleAssetName = "RNTesterApp.android.bundle" entryFile = file("../../js/RNTesterApp.android.js") - reactRoot = rootDir + root = rootDir inputExcludes = ["android/**", "./**", ".gradle/**"] composeSourceMapsPath = "$rootDir/scripts/compose-source-maps.js" hermesCommand = "$rootDir/node_modules/hermes-engine/%OS-BIN%/hermesc" @@ -88,6 +88,7 @@ react { // Codegen Configs jsRootDir = file("$rootDir/packages/rn-tester") + reactNativeDir = rootDir libraryName = "rntester" useJavaGenerator = System.getenv("USE_CODEGEN_JAVAPOET")?.toBoolean() ?: false }