From 8bc324fd34337ab159e2e21e213a6c5b06c548da 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 d391da8576cef9..6067a8802178c7 100644 --- a/ReactAndroid/build.gradle +++ b/ReactAndroid/build.gradle @@ -407,7 +407,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 5c72bdb9838891..7f1d914de461c5 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 9f8759f0dbc624..42490044ba89f0 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 30625c53d37d7c..4165d85845887a 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 e44f72954c12ef..f0bda7737beabb 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 eb965bc0602e6e..302c1d83a7b412 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 81c40375c6d752..900086e43c919e 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 c87d5fa3c6e676..debba45740b3b0 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 648b4d8858071f..07e581e9ba4cbf 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 }