feat: create symlink for build files present on node modules installe…

…d with relative paths (#2330)

WORKSPACE

@@ -182,6 +182,9 @@ local_repository(
 yarn_install(
     name = "fine_grained_deps_yarn",
     data = [
+        "//:tools/npm_packages/local_module/yarn/BUILD.bazel",
+        "//:tools/npm_packages/local_module/yarn/index.js",
+        "//:tools/npm_packages/local_module/yarn/package.json",
         "//internal/npm_install/test:postinstall.js",
     ],
     environment = {
@@ -202,6 +205,9 @@ yarn_install(
 npm_install(
     name = "fine_grained_deps_npm",
     data = [
+        "//:tools/npm_packages/local_module/npm/BUILD.bazel",
+        "//:tools/npm_packages/local_module/npm/index.js",
+        "//:tools/npm_packages/local_module/npm/package.json",
         "//internal/npm_install/test:postinstall.js",
     ],
     environment = {

internal/npm_install/generate_build_file.ts

@@ -93,6 +93,15 @@ function writeFileSync(p: string, content: string) {
   fs.writeFileSync(p, content);
 }
 
+/**
+ * Creates a file symlink, first ensuring that the directory to
+ * create it into exists.
+ */
+function createFileSymlinkSync(target: string, p: string) {
+  mkdirp(path.dirname(p));
+  fs.symlinkSync(target, p, 'file');
+}
+
 /**
  * Main entrypoint.
  */
@@ -194,11 +203,37 @@ js_library(
  * Generates all BUILD & bzl files for a package.
  */
 function generatePackageBuildFiles(pkg: Dep) {
-  // If a BUILD file was shipped with the package, append its contents to the end of
-  // what we generate for the package.
+  // If a BUILD file was shipped with the package we should symlink the generated BUILD file
+  // instead of append its contents to the end of the one we were going to generate.
+  // https://github.com/bazelbuild/rules_nodejs/issues/2131
   let buildFilePath: string|undefined;
   if (pkg._files.includes('BUILD')) buildFilePath = 'BUILD';
   if (pkg._files.includes('BUILD.bazel')) buildFilePath = 'BUILD.bazel';
+
+  // Recreate the pkg dir inside the node_modules folder
+  const nodeModulesPkgDir = `node_modules/${pkg._dir}`;
+  // Check if the current package dep dir is a symlink (which happens when we
+  // 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;
+
+  // Log if a BUILD file was expected but was not found
+  if (!symlinkBuildFile && isPkgDirASymlink) {
+    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)}`)
+  }
+
+  // The following won't be used in a symlink build file case
   let buildFile = printPackage(pkg);
   if (buildFilePath) {
     buildFile = buildFile + '\n' +
@@ -256,7 +291,14 @@ exports_files(["index.bzl"])
     }
   }
 
-  writeFileSync(path.posix.join(pkg._dir, buildFilePath), generateBuildFileHeader(visibility) + buildFile);
+  if (!symlinkBuildFile) {
+    writeFileSync(
+        path.posix.join(pkg._dir, buildFilePath), generateBuildFileHeader(visibility) + buildFile);
+  } else {
+    const realPathBuildFileForPkg =
+        fs.realpathSync(path.posix.join(nodeModulesPkgDir, buildFilePath));
+    createFileSymlinkSync(realPathBuildFileForPkg, path.posix.join(pkg._dir, buildFilePath));
+  }
 }
 
 /**

internal/npm_install/index.js

@@ -39,6 +39,10 @@ function writeFileSync(p, content) {
     mkdirp(path.dirname(p));
     fs.writeFileSync(p, content);
 }
+function createFileSymlinkSync(target, p) {
+    mkdirp(path.dirname(p));
+    fs.symlinkSync(target, p, 'file');
+}
 function main() {
     const deps = getDirectDependencySet(PKG_JSON_FILE_PATH);
     const pkgs = findPackages('node_modules', deps);
@@ -113,6 +117,13 @@ function generatePackageBuildFiles(pkg) {
         buildFilePath = 'BUILD';
     if (pkg._files.includes('BUILD.bazel'))
         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) {
+        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);
     if (buildFilePath) {
         buildFile = buildFile + '\n' +
@@ -154,7 +165,13 @@ exports_files(["index.bzl"])
 `;
         }
     }
-    writeFileSync(path.posix.join(pkg._dir, buildFilePath), generateBuildFileHeader(visibility) + buildFile);
+    if (!symlinkBuildFile) {
+        writeFileSync(path.posix.join(pkg._dir, buildFilePath), generateBuildFileHeader(visibility) + buildFile);
+    }
+    else {
+        const realPathBuildFileForPkg = fs.realpathSync(path.posix.join(nodeModulesPkgDir, buildFilePath));
+        createFileSymlinkSync(realPathBuildFileForPkg, path.posix.join(pkg._dir, buildFilePath));
+    }
 }
 function generateBazelWorkspaces(pkgs) {
     const workspaces = {};

internal/npm_install/npm_install.bzl

@@ -312,7 +312,26 @@ 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.""",
+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.""",
     implementation = _npm_install_impl,
 )
 
@@ -475,6 +494,25 @@ 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.""",
+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.""",
     implementation = _yarn_install_impl,
 )

internal/npm_install/test/BUILD.bazel

@@ -133,6 +133,7 @@ sh_test(
         "@fine_grained_deps_%s//jasmine" % pkgmgr,
         "@fine_grained_deps_%s//jasmine-core" % pkgmgr,
         "@fine_grained_deps_%s//ajv" % pkgmgr,
+        "@fine_grained_deps_%s//local-module" % pkgmgr,
         "@fine_grained_deps_%s//typescript" % pkgmgr,
         "@fine_grained_deps_%s//rxjs" % pkgmgr,
         # Note, test-b depends on test-a@0.0.1 which should be

internal/npm_install/test/common.spec.js

@@ -15,6 +15,10 @@ describe('dependencies', () => {
     require('ajv/lib/$data');
   });
 
+  it(`should resolve local-module`, () => {
+    require('local-module');
+  });
+
   it(`should resolve rxjs/src/tsconfig.json`, () => {
     // the BUILD.bazel file in rxjs/src should have been
     // deleted by fine grained deps and rxjs/src/tsconfig.json

tools/fine_grained_deps_npm/package.json

@@ -12,6 +12,7 @@
     "chokidar": "2.0.4",
     "http-server": "github:alexeagle/http-server#97205e945b69091606ed83aa0c8489e9ce65d282",
     "klaw": "1.3.1",
+    "local-module": "file:tools/npm_packages/local_module/npm",
     "rxjs": "6.5.0"
   },
   "scripts": {

tools/fine_grained_deps_yarn/package.json

@@ -12,6 +12,7 @@
     "chokidar": "2.0.4",
     "http-server": "github:alexeagle/http-server#97205e945b69091606ed83aa0c8489e9ce65d282",
     "klaw": "1.3.1",
+    "local-module": "link:tools/npm_packages/local_module/yarn",
     "rxjs": "6.5.0"
   },
   "scripts": {

tools/fine_grained_deps_yarn/yarn.lock

@@ -751,6 +751,10 @@ klaw@1.3.1:
   optionalDependencies:
     graceful-fs "^4.1.9"
 
+"local-module@link:tools/npm_packages/local_module/yarn":
+  version "0.0.0"
+  uid ""
+
 lodash.debounce@^4.0.8:
   version "4.0.8"
   resolved "https://registry.yarnpkg.com/lodash.debounce/-/lodash.debounce-4.0.8.tgz#82d79bff30a67c4005ffd5e2515300ad9ca4d7af"

tools/npm_packages/local_module/npm/BUILD.bazel

@@ -0,0 +1,35 @@
+load("@build_bazel_rules_nodejs//:index.bzl", "js_library")
+
+package(default_visibility = ["//visibility:public"])
+
+SRCS = [
+    "index.js",
+    "package.json",
+]
+
+filegroup(
+    name = "local-module__files",
+    srcs = ["@fine_grained_deps_npm//:node_modules/local-module/%s" % file for file in SRCS],
+)
+
+js_library(
+    name = "local-module",
+    srcs = [":local-module__files"],
+    deps = [
+        ":local-module__contents",
+    ],
+)
+
+js_library(
+    name = "local-module__contents",
+    srcs = [
+        ":local-module__files",
+        ":local-module__nested_node_modules",
+    ],
+    visibility = ["//:__subpackages__"],
+)
+
+filegroup(
+    name = "local-module__nested_node_modules",
+    visibility = ["//:__subpackages__"],
+)

tools/npm_packages/local_module/npm/index.js

@@ -0,0 +1 @@
+module.exports = 'local_module';

tools/npm_packages/local_module/npm/package.json

@@ -0,0 +1,4 @@
+{
+  "version": "0.0.1",
+  "main": "index.js"
+}

tools/npm_packages/local_module/yarn/BUILD.bazel

@@ -0,0 +1,35 @@
+load("@build_bazel_rules_nodejs//:index.bzl", "js_library")
+
+package(default_visibility = ["//visibility:public"])
+
+SRCS = [
+    "index.js",
+    "package.json",
+]
+
+filegroup(
+    name = "local-module__files",
+    srcs = ["@fine_grained_deps_yarn//:node_modules/local-module/%s" % file for file in SRCS],
+)
+
+js_library(
+    name = "local-module",
+    srcs = [":local-module__files"],
+    deps = [
+        ":local-module__contents",
+    ],
+)
+
+js_library(
+    name = "local-module__contents",
+    srcs = [
+        ":local-module__files",
+        ":local-module__nested_node_modules",
+    ],
+    visibility = ["//:__subpackages__"],
+)
+
+filegroup(
+    name = "local-module__nested_node_modules",
+    visibility = ["//:__subpackages__"],
+)

tools/npm_packages/local_module/yarn/index.js

@@ -0,0 +1 @@
+module.exports = 'local_module';

tools/npm_packages/local_module/yarn/package.json

@@ -0,0 +1,4 @@
+{
+  "version": "0.0.1",
+  "main": "index.js"
+}