feat: add generate_local_modules_build_files flag to yarn_install and npm_install rules (#2449)

mistic · web-flow · commit a6449b7c7a4d · 2021-02-18T17:19:23.000-05:00
diff --git a/WORKSPACE b/WORKSPACE
@@ -203,6 +203,7 @@ yarn_install(
     environment = {
         "SOME_USER_ENV": "yarn is great!",
     },
+    generate_local_modules_build_files = False,
     included_files = [
         "",
         ".js",
@@ -226,6 +227,7 @@ npm_install(
     environment = {
         "SOME_USER_ENV": "npm is cool!",
     },
+    generate_local_modules_build_files = False,
     included_files = [
         "",
         ".js",
diff --git a/internal/npm_install/generate_build_file.ts b/internal/npm_install/generate_build_file.ts
@@ -57,8 +57,9 @@ const WORKSPACE_ROOT_PREFIX = args[4];
 const WORKSPACE_ROOT_BASE = WORKSPACE_ROOT_PREFIX ?.split('/')[0];
 const STRICT_VISIBILITY = args[5]?.toLowerCase() === 'true';
 const INCLUDED_FILES = args[6] ? args[6].split(',') : [];
-const BAZEL_VERSION = args[7];
-const PACKAGE_PATH = args[8];
+const GENERATE_LOCAL_MODULES_BUILD_FILES = (`${args[7]}`.toLowerCase()) === 'true';
+const BAZEL_VERSION = args[8];
+const PACKAGE_PATH = args[9];
 
 const PUBLIC_VISIBILITY = '//visibility:public';
 const LIMITED_VISIBILITY = `@${WORKSPACE}//:__subpackages__`;
@@ -221,18 +222,12 @@ function generatePackageBuildFiles(pkg: Dep) {
   // install a node_module with link:)
   const isPkgDirASymlink =
       fs.existsSync(nodeModulesPkgDir) && fs.lstatSync(nodeModulesPkgDir).isSymbolicLink();
-  // Check if the current package is also written inside the workspace
-  // NOTE: It's a corner case but fs.realpathSync(.) will not be the root of
-  // the workspace if symlink_node_modules = False as yarn & npm are run in the root of the
-  // external repository and  anything linked with a relative path would have to be copied
-  // over via that data attribute
-  const isPkgInsideWorkspace = fs.realpathSync(nodeModulesPkgDir).includes(fs.realpathSync(`.`));
   // Mark build file as one to symlink instead of generate as the package dir is a symlink, we
   // have a BUILD file and the pkg is written inside the workspace
-  const symlinkBuildFile = isPkgDirASymlink && buildFilePath && isPkgInsideWorkspace;
+  const symlinkBuildFile = isPkgDirASymlink && buildFilePath && !GENERATE_LOCAL_MODULES_BUILD_FILES;
 
   // Log if a BUILD file was expected but was not found
-  if (!symlinkBuildFile && isPkgDirASymlink) {
+  if (isPkgDirASymlink && !buildFilePath && !GENERATE_LOCAL_MODULES_BUILD_FILES) {
     console.log(`[yarn_install/npm_install]: package ${
         nodeModulesPkgDir} is local symlink and as such a BUILD file for it is expected but none was found. Please add one at ${
         fs.realpathSync(nodeModulesPkgDir)}`)
diff --git a/internal/npm_install/index.js b/internal/npm_install/index.js
@@ -17,8 +17,9 @@ const WORKSPACE_ROOT_PREFIX = args[4];
 const WORKSPACE_ROOT_BASE = (_a = WORKSPACE_ROOT_PREFIX) === null || _a === void 0 ? void 0 : _a.split('/')[0];
 const STRICT_VISIBILITY = ((_b = args[5]) === null || _b === void 0 ? void 0 : _b.toLowerCase()) === 'true';
 const INCLUDED_FILES = args[6] ? args[6].split(',') : [];
-const BAZEL_VERSION = args[7];
-const PACKAGE_PATH = args[8];
+const GENERATE_LOCAL_MODULES_BUILD_FILES = (`${args[7]}`.toLowerCase()) === 'true';
+const BAZEL_VERSION = args[8];
+const PACKAGE_PATH = args[9];
 const PUBLIC_VISIBILITY = '//visibility:public';
 const LIMITED_VISIBILITY = `@${WORKSPACE}//:__subpackages__`;
 function generateBuildFileHeader(visibility = PUBLIC_VISIBILITY) {
@@ -123,9 +124,8 @@ function generatePackageBuildFiles(pkg) {
         buildFilePath = 'BUILD.bazel';
     const nodeModulesPkgDir = `node_modules/${pkg._dir}`;
     const isPkgDirASymlink = fs.existsSync(nodeModulesPkgDir) && fs.lstatSync(nodeModulesPkgDir).isSymbolicLink();
-    const isPkgInsideWorkspace = fs.realpathSync(nodeModulesPkgDir).includes(fs.realpathSync(`.`));
-    const symlinkBuildFile = isPkgDirASymlink && buildFilePath && isPkgInsideWorkspace;
-    if (!symlinkBuildFile && isPkgDirASymlink) {
+    const symlinkBuildFile = isPkgDirASymlink && buildFilePath && !GENERATE_LOCAL_MODULES_BUILD_FILES;
+    if (isPkgDirASymlink && !buildFilePath && !GENERATE_LOCAL_MODULES_BUILD_FILES) {
         console.log(`[yarn_install/npm_install]: package ${nodeModulesPkgDir} is local symlink and as such a BUILD file for it is expected but none was found. Please add one at ${fs.realpathSync(nodeModulesPkgDir)}`);
     }
     let buildFile = printPackage(pkg);
diff --git a/internal/npm_install/npm_install.bzl b/internal/npm_install/npm_install.bzl
@@ -44,6 +44,29 @@ repository so all files that the package manager depends on must be listed.
         doc = """Environment variables to set before calling the package manager.""",
         default = {},
     ),
+    "generate_local_modules_build_files": attr.bool(
+        default = True,
+        doc = """Enables the BUILD files auto generation for local modules installed with `file:` (npm) or `link:` (yarn)
+
+When using a monorepo it's common to have modules that we want to use locally and
+publish to an external package repository. This can be achieved using a `js_library` rule
+with a `package_name` attribute defined inside the local package `BUILD` file. However,
+if the project relies on the local package dependency with `file:` (npm) or `link:` (yarn) to be used outside Bazel, this
+could introduce a race condition with both `npm_install` or `yarn_install` rules.
+
+In order to overcome it, a link could be created to the package `BUILD` file from the
+npm external Bazel repository (so we can use a local BUILD file instead of an auto generated one),
+which require us to set `generate_local_modules_build_files = False` and complete a last step which is writing the
+expected targets on that same `BUILD` file to be later used both by `npm_install` or `yarn_install`
+rules, which are: `<package_name__files>`, `<package_name__nested_node_modules>`,
+`<package_name__contents>`, `<package_name__typings>` and the last one just `<package_name>`. If you doubt what those targets
+should look like, check the generated `BUILD` file for a given node module.
+
+When true, the rule will follow the default behaviour of auto generating BUILD files for each `node_module` at install time.
+
+When False, the rule will not auto generate BUILD files for `node_modules` that are installed as symlinks for local modules.
+""",
+    ),
     "included_files": attr.string_list(
         doc = """List of file extensions to be included in the npm package targets.
 
@@ -123,7 +146,7 @@ data attribute.
     ),
 })
 
-def _create_build_files(repository_ctx, rule_type, node, lock_file):
+def _create_build_files(repository_ctx, rule_type, node, lock_file, generate_local_modules_build_files):
     repository_ctx.report_progress("Processing node_modules: installing Bazel packages and generating BUILD files")
     if repository_ctx.attr.manual_build_file_contents:
         repository_ctx.file("manual_build_file_contents", repository_ctx.attr.manual_build_file_contents)
@@ -137,6 +160,7 @@ def _create_build_files(repository_ctx, rule_type, node, lock_file):
         _workspace_root_prefix(repository_ctx),
         str(repository_ctx.attr.strict_visibility),
         ",".join(repository_ctx.attr.included_files),
+        str(generate_local_modules_build_files),
         native.bazel_version,
         repository_ctx.attr.package_path,
         # double the default timeout in case of many packages, see #2231
@@ -344,7 +368,7 @@ cd /D "{root}" && "{npm}" {npm_args}
 
     _symlink_node_modules(repository_ctx)
 
-    _create_build_files(repository_ctx, "npm_install", node, repository_ctx.attr.package_lock_json)
+    _create_build_files(repository_ctx, "npm_install", node, repository_ctx.attr.package_lock_json, repository_ctx.attr.generate_local_modules_build_files)
 
 npm_install = repository_rule(
     attrs = dict(COMMON_ATTRIBUTES, **{
@@ -374,26 +398,7 @@ See npm CLI docs https://docs.npmjs.com/cli/install.html for complete list of su
 
 This rule will set the environment variable `BAZEL_NPM_INSTALL` to '1' (unless it
 set to another value in the environment attribute). Scripts may use to this to
-check if yarn is being run by the `npm_install` repository rule.
-
-
-**LOCAL MODULES WITH THE NEED TO BE USED BOTH INSIDE AND OUTSIDE BAZEL**
-
-When using a monorepo it's common to have modules that we want to use locally and
-publish to an external package repository. This can be achieved using a `js_library` rule
-with a `package_name` attribute defined inside the local package `BUILD` file. However,
-if the project relies on the local package dependency with `file:`, this could introduce a
-race condition with the `npm_install` rule.
-
-In order to overcome it, a link will be created to the package `BUILD` file from the
-npm external Bazel repository, which require us to complete a last step which is writing
-the expected targets on that same `BUILD` file to be later used by the `npm_install`
-rule, which are: `<package_name__files>`, `<package_name__nested_node_modules>`,
-`<package_name__contents>`, `<package_name__typings>` and the last
-one just `<package_name>`.
-
-If you doubt what those targets should look like, check the
-generated `BUILD` file for a given node module.""",
+check if yarn is being run by the `npm_install` repository rule.""",
     implementation = _npm_install_impl,
 )
 
@@ -499,7 +504,7 @@ cd /D "{root}" && "{yarn}" {yarn_args}
 
     _symlink_node_modules(repository_ctx)
 
-    _create_build_files(repository_ctx, "yarn_install", node, repository_ctx.attr.yarn_lock)
+    _create_build_files(repository_ctx, "yarn_install", node, repository_ctx.attr.yarn_lock, repository_ctx.attr.generate_local_modules_build_files)
 
 yarn_install = repository_rule(
     attrs = dict(COMMON_ATTRIBUTES, **{
@@ -550,25 +555,6 @@ to yarn so that the local cache is contained within the external repository.
 
 This rule will set the environment variable `BAZEL_YARN_INSTALL` to '1' (unless it
 set to another value in the environment attribute). Scripts may use to this to
-check if yarn is being run by the `yarn_install` repository rule.
-
-
-**LOCAL MODULES WITH THE NEED TO BE USED BOTH INSIDE AND OUTSIDE BAZEL**
-
-When using a monorepo it's common to have modules that we want to use locally and
-publish to an external package repository. This can be achieved using a `js_library` rule
-with a `package_name` attribute defined inside the local package `BUILD` file. However,
-if the project relies on the local package dependency with `link:`, this could introduce a
-race condition with the `yarn_install` rule.
-
-In order to overcome it, a link will be created to the package `BUILD` file from the
-npm external Bazel repository, which require us to complete a last step which is writing
-the expected targets on that same `BUILD` file to be later used by the `yarn_install`
-rule, which are: `<package_name__files>`, `<package_name__nested_node_modules>`,
-`<package_name__contents>`, `<package_name__typings>` and the last
-one just `<package_name>`.
-
-If you doubt what those targets should look like, check the
-generated `BUILD` file for a given node module.""",
+check if yarn is being run by the `yarn_install` repository rule.""",
     implementation = _yarn_install_impl,
 )
