feat(builtin): use linker for all generated :bin targets

rather than loader.js module-resolver hacks
for generated targets, we now expect that tools will use the linker to
set up their node_modules tree, or will explicitly use a runfiles helper
library to resolve files in the runfiles, like other languages in Bazel.

BREAKING CHANGE:
If you use the generated nodejs_binary or nodejs_test rules in the npm
workspace, for example @npm//typescript/bin:tsc, your custom rule must now link the
node_modules directory into that process. A typical way to do this is
with the run_node helper. See updates to examples in this commit.

Author: alexeagle

examples/BUILD.bazel

@@ -91,6 +91,10 @@ example_integration_test(
     npm_packages = {
         "//packages/jasmine:npm_package": "@bazel/jasmine",
     },
+    # Parcel spawns a subprocess which requires our node-patches
+    # but we don't yet have a mechanism on Windows for spawned processes
+    # to inherit the --require script needed to install the patches
+    tags = ["no-bazelci-windows"],
 )
 
 example_integration_test(

examples/parcel/parcel.bzl

@@ -17,6 +17,8 @@
 This is not a full-featured Parcel bazel rule, just enough to demonstrate how to write one.
 """
 
+load("@build_bazel_rules_nodejs//:providers.bzl", "run_node")
+
 def _parcel_impl(ctx):
     """The "implementation function" for our rule.
 
@@ -31,9 +33,12 @@ def _parcel_impl(ctx):
     args += ["--out-dir", ctx.outputs.bundle.dirname]
     args += ["--out-file", ctx.outputs.bundle.basename]
 
-    ctx.actions.run(
+    # We use the run_node helper rather than ctx.actions.run so that the npm package
+    # gets automatically included in the action inputs
+    run_node(
+        ctx = ctx,
         inputs = ctx.files.srcs + [ctx.file.entry_point],
-        executable = ctx.executable.parcel,
+        executable = "parcel",
         outputs = [ctx.outputs.bundle, ctx.outputs.sourcemap],
         arguments = args,
         env = {"COMPILATION_MODE": ctx.var["COMPILATION_MODE"]},

internal/linker/index.js

@@ -143,12 +143,16 @@ class Runfiles {
     }
     resolve(modulePath) {
         if (this.manifest) {
-            return this.lookupDirectory(modulePath);
+            const result = this.lookupDirectory(modulePath);
+            if (result)
+                return result;
         }
-        if (exports.runfiles.dir) {
+        if (exports.runfiles.dir && fs.existsSync(path.join(exports.runfiles.dir, modulePath))) {
             return path.join(exports.runfiles.dir, modulePath);
         }
-        throw new Error(`could not resolve modulePath ${modulePath}`);
+        const e = new Error(`could not resolve modulePath ${modulePath}`);
+        e.code = 'MODULE_NOT_FOUND';
+        throw e;
     }
     resolveWorkspaceRelative(modulePath) {
         if (!this.workspace) {
@@ -345,7 +349,12 @@ function main(args, runfiles) {
                             else {
                                 runfilesPath = `${workspace}/${runfilesPath}`;
                             }
-                            target = runfiles.resolve(runfilesPath) || '<runfiles resolution failed>';
+                            try {
+                                target = runfiles.resolve(runfilesPath);
+                            }
+                            catch (_a) {
+                                target = '<runfiles resolution failed>';
+                            }
                             break;
                     }
                     yield symlink(target, m.name);

internal/linker/link_node_modules.ts

@@ -245,12 +245,15 @@ export class Runfiles {
   resolve(modulePath: string) {
     // Look in the runfiles first
     if (this.manifest) {
-      return this.lookupDirectory(modulePath);
+      const result = this.lookupDirectory(modulePath);
+      if (result) return result;
     }
-    if (runfiles.dir) {
+    if (runfiles.dir && fs.existsSync(path.join(runfiles.dir, modulePath))) {
       return path.join(runfiles.dir, modulePath);
     }
-    throw new Error(`could not resolve modulePath ${modulePath}`);
+    const e = new Error(`could not resolve modulePath ${modulePath}`);
+    (e as any).code = 'MODULE_NOT_FOUND';
+    throw e;
   }
 
   resolveWorkspaceRelative(modulePath: string) {
@@ -569,7 +572,11 @@ export async function main(args: string[], runfiles: Runfiles) {
           } else {
             runfilesPath = `${workspace}/${runfilesPath}`;
           }
-          target = runfiles.resolve(runfilesPath) || '<runfiles resolution failed>';
+          try {
+            target = runfiles.resolve(runfilesPath);
+          } catch {
+            target = '<runfiles resolution failed>';
+          }
           break;
       }
 

internal/node/node_patches.js

@@ -593,7 +593,10 @@ fi
         process.env.PATH = nodeDir + path.delimiter + process.env.PATH;
     }
     // fix execPath so folks use the proxy node
-    process.argv[0] = process.execPath = path.join(nodeDir, 'node');
+    if (process.platform == 'win32') ;
+    else {
+        process.argv[0] = process.execPath = path.join(nodeDir, 'node');
+    }
     // replace any instances of require script in execArgv with the absolute path to the script.
     // example: bazel-require-script.js
     process.execArgv.map(v => {

internal/npm_install/generate_build_file.ts

@@ -1074,7 +1074,8 @@ nodejs_binary(
     name = "${name}",
     entry_point = "//:node_modules/${pkg._dir}/${path}",
     install_source_map_support = False,
-    data = [${data.map(p => `"${p}"`).join(', ')}],${additionalAttributes(pkg, name)}
+    data = [${data.map(p => `"${p}"`).join(', ')}],
+    templated_args = ["--nobazel_patch_module_resolver"],${additionalAttributes(pkg, name)}
 )
 `;
     }

internal/npm_install/index.js

@@ -645,7 +645,8 @@ nodejs_binary(
     name = "${name}",
     entry_point = "//:node_modules/${pkg._dir}/${path}",
     install_source_map_support = False,
-    data = [${data.map(p => `"${p}"`).join(', ')}],${additionalAttributes(pkg, name)}
+    data = [${data.map(p => `"${p}"`).join(', ')}],
+    templated_args = ["--nobazel_patch_module_resolver"],${additionalAttributes(pkg, name)}
 )
 `;
         }

internal/npm_install/test/golden/@gregmagolan/test-a/bin/BUILD.bazel.golden

@@ -6,4 +6,5 @@ nodejs_binary(
     entry_point = "//:node_modules/@gregmagolan/test-a/@bin/test.js",
     install_source_map_support = False,
     data = ["//@gregmagolan/test-a:test-a"],
+    templated_args = ["--nobazel_patch_module_resolver"],
 )

internal/npm_install/test/golden/jasmine/bin/BUILD.bazel.golden

@@ -6,4 +6,5 @@ nodejs_binary(
     entry_point = "//:node_modules/jasmine/bin/jasmine.js",
     install_source_map_support = False,
     data = ["//jasmine:jasmine"],
+    templated_args = ["--nobazel_patch_module_resolver"],
 )

internal/providers/node_runtime_deps_info.bzl

@@ -74,7 +74,6 @@ def run_node(ctx, inputs, arguments, executable, **kwargs):
 
     modules_manifest = write_node_modules_manifest(ctx, link_data)
     add_arg(arguments, "--bazel_node_modules_manifest=%s" % modules_manifest.path)
-    inputs.append(modules_manifest)
 
     # By using the run_node helper, you suggest that your program
     # doesn't implicitly use runfiles to require() things
@@ -98,7 +97,7 @@ def run_node(ctx, inputs, arguments, executable, **kwargs):
     env["BAZEL_NODE_MODULES_ROOT"] = _compute_node_modules_root(ctx)
 
     ctx.actions.run(
-        inputs = inputs + extra_inputs,
+        inputs = inputs + extra_inputs + [modules_manifest],
         arguments = arguments,
         executable = exec_exec,
         env = env,