fix(builtin): support for scoped modules in linker (#1199)

Author: gregmagolan

internal/linker/index.js

@@ -24,6 +24,17 @@ Include as much of the build output as you can without disclosing anything confi
   `);
 }
 
+/**
+ * Create a new directory and any necessary subdirectories
+ * if they do not exist.
+ */
+function mkdirp(p: string) {
+  if (!fs.existsSync(p)) {
+    mkdirp(path.dirname(p));
+    fs.mkdirSync(p);
+  }
+}
+
 async function symlink(target: string, path: string) {
   log_verbose(`symlink( ${path} -> ${target} )`);
   // Use junction on Windows since symlinks require elevated permissions.
@@ -46,7 +57,7 @@ async function symlink(target: string, path: string) {
     if (!fs.existsSync(path)) {
       log_verbose(
           'ERROR\n***\nLooks like we created a bad symlink:' +
-          `\n  pwd ${process.cwd()}\n  target ${target}\n***`);
+          `\n  pwd ${process.cwd()}\n  target ${target}\n  path ${path}\n***`);
     }
   }
 }
@@ -179,8 +190,6 @@ export class Runfiles {
     if (this.manifest) {
       return this.lookupDirectory(modulePath);
     }
-    // how can we avoid this FS lookup every time? we don't know when process.cwd changed...
-    // const runfilesRelative = runfiles.dir ? path.relative('.', runfiles.dir) : undefined;
     if (runfiles.dir) {
       return path.join(runfiles.dir, modulePath);
     }
@@ -262,7 +271,7 @@ export async function main(args: string[], runfiles: Runfiles) {
   process.chdir(rootDir);
 
   // Symlinks to packages need to reach back to the workspace/runfiles directory
-  const workspaceRelative = path.relative('.', workspaceDir);
+  const workspaceAbs = path.resolve(workspaceDir);
 
   // Now add symlinks to each of our first-party packages so they appear under the node_modules tree
   const links = [];
@@ -273,7 +282,7 @@ export async function main(args: string[], runfiles: Runfiles) {
     switch (root) {
       case 'bin':
         // FIXME(#1196)
-        target = path.join(workspaceRelative, bin, toWorkspaceDir(modulePath));
+        target = path.join(workspaceAbs, bin, toWorkspaceDir(modulePath));
         // Spend an extra FS lookup to give better error in this case
         if (!await exists(target)) {
           // TODO: there should be some docs explaining how users are
@@ -287,7 +296,7 @@ export async function main(args: string[], runfiles: Runfiles) {
         }
         break;
       case 'src':
-        target = path.join(workspaceRelative, toWorkspaceDir(modulePath));
+        target = path.join(workspaceAbs, toWorkspaceDir(modulePath));
         break;
       case 'runfiles':
         target = runfiles.resolve(modulePath) || '<runfiles resolution failed>';
@@ -298,6 +307,14 @@ export async function main(args: string[], runfiles: Runfiles) {
   }
 
   for (const m of Object.keys(modules)) {
+    const segments = m.split('/');
+    if (segments.length > 2) {
+      throw new Error(`module ${m} has more than 2 segments which is not a valid node module name`);
+    }
+    if (segments.length == 2) {
+      // ensure the scope exists
+      mkdirp(segments[0]);
+    }
     const [kind, modulePath] = modules[m];
     links.push(linkModule(m, kind, modulePath));
   }

internal/linker/link_node_modules.ts

@@ -21,6 +21,7 @@ sh_binary(
         ":program.js",
         "//internal/linker:index.js",
         "//internal/linker/test/integration/static_linked_pkg",
+        "//internal/linker/test/integration/static_linked_scoped_pkg",
         "@bazel_tools//tools/bash/runfiles",
         "@build_bazel_rules_nodejs//toolchains/node:node_bin",
     ],
@@ -35,6 +36,7 @@ linked(
         "//%s/absolute_import:index.js" % package_name(),
         ":run_program",
         "//internal/linker/test/integration/dynamic_linked_pkg",
+        "//internal/linker/test/integration/dynamic_linked_scoped_pkg",
         "@npm//semver",
     ],
 )

internal/linker/test/integration/BUILD.bazel

@@ -0,0 +1,10 @@
+load("//internal/js_library:js_library.bzl", "js_library")
+
+package(default_visibility = ["//internal/linker/test:__subpackages__"])
+
+js_library(
+    name = "dynamic_linked_scoped_pkg",
+    srcs = ["index.js"],
+    module_from_src = True,
+    module_name = "@linker_scoped/dynamic_linked",
+)

internal/linker/test/integration/dynamic_linked_scoped_pkg/BUILD.bazel

@@ -0,0 +1,5 @@
+function addD(str) {
+  return `${str}_d`;
+}
+
+exports.addD = addD;

internal/linker/test/integration/dynamic_linked_scoped_pkg/index.js

@@ -1 +1 @@
-1.2.3_c_b_a
+1.2.3_a_b_c_d_e

internal/linker/test/integration/golden.txt

@@ -1,14 +1,16 @@
-// First-party package from ./static_linked_pkg
-// it should get resolved through runfiles
+// First-party "static linked" packages
+// they should get resolved through runfiles
 const a = require('static_linked');
-// First-party package from ./dynamic_linked_pkg
-// it should get resolved from the execroot
+const e = require('@linker_scoped/static_linked');
+// First-party "dynamic linked" packages
+// they should get resolved from the execroot
 const b = require('dynamic_linked');
+const d = require('@linker_scoped/dynamic_linked');
 // We've always supported `require('my_workspace')` for absolute imports like Google does it
 const c = require('build_bazel_rules_nodejs/internal/linker/test/integration/absolute_import');
 
 // Third-party package installed in the root node_modules
 const semver = require('semver');
 
 // This output should match what's in the golden.txt file
-console.log(a.addA(b.addB(c.addC(semver.clean(' =v1.2.3 ')))));
+console.log(e.addE(d.addD(c.addC(b.addB(a.addA(semver.clean(' =v1.2.3 ')))))));

internal/linker/test/integration/program.js

@@ -0,0 +1,9 @@
+load("//internal/js_library:js_library.bzl", "js_library")
+
+package(default_visibility = ["//internal/linker/test:__subpackages__"])
+
+js_library(
+    name = "static_linked_scoped_pkg",
+    srcs = ["index.js"],
+    module_name = "@linker_scoped/static_linked",
+)

internal/linker/test/integration/static_linked_scoped_pkg/BUILD.bazel

@@ -0,0 +1,5 @@
+function addE(str) {
+  return `${str}_e`;
+}
+
+exports.addE = addE;