Hook up linker to nodejs_binary/nodejs_test

internal/linker/link_node_modules.bzl

@@ -34,14 +34,12 @@ def add_arg(args, arg):
     else:
         args.add(arg)
 
-def register_node_modules_linker(ctx, args, inputs, data = []):
-    """Helps an action to run node by setting up the node_modules linker as a pre-process
+def write_node_modules_manifest(ctx, extra_data = []):
+    """Writes a manifest file read by the linker, containing info about resolving runtime dependencies
 
     Args:
-      ctx: Bazel's starlark execution context, used to get attributes and actions
-      args: Arguments being passed to the program; a linker argument will be appended
-      inputs: inputs being passed to the program; a linker input will be appended
-      data: labels to search for npm packages that need to be linked (ctx.attr.deps and ctx.attr.data will always be searched)
+        ctx: starlark rule execution context
+        extra_data: labels to search for npm packages that need to be linked (ctx.attr.deps and ctx.attr.data will always be searched)
     """
 
     mappings = {
@@ -53,7 +51,7 @@ def register_node_modules_linker(ctx, args, inputs, data = []):
     node_modules_root = ""
 
     # Look through data/deps attributes to find...
-    for dep in data + getattr(ctx.attr, "data", []) + getattr(ctx.attr, "deps", []):
+    for dep in extra_data + getattr(ctx.attr, "data", []) + getattr(ctx.attr, "deps", []):
         # ...the root directory for the third-party node_modules; we'll symlink the local "node_modules" to it
         if NpmPackageInfo in dep:
             possible_root = "/".join([dep[NpmPackageInfo].workspace, "node_modules"])
@@ -80,8 +78,7 @@ def register_node_modules_linker(ctx, args, inputs, data = []):
         "workspace": ctx.workspace_name,
     }
     ctx.actions.write(modules_manifest, str(content))
-    add_arg(args, "--bazel_node_modules_manifest=%s" % modules_manifest.path)
-    inputs.append(modules_manifest)
+    return modules_manifest
 
 def get_module_mappings(label, attrs, vars, rule_kind, srcs = [], workspace_name = None):
     """Returns the module_mappings from the given attrs.

internal/linker/test/integration/rule.bzl

@@ -1,18 +1,17 @@
 "Minimal fixture for executing the linker's starlark code"
 
-load("@build_bazel_rules_nodejs//internal/linker:link_node_modules.bzl", "module_mappings_aspect", "register_node_modules_linker")
+load("@build_bazel_rules_nodejs//internal/linker:link_node_modules.bzl", "module_mappings_aspect", "write_node_modules_manifest")
 
 def _linked(ctx):
-    inputs = ctx.files.deps[:]
-    outputs = [ctx.outputs.out]
-    args = ctx.actions.args()
-    register_node_modules_linker(ctx, args, inputs)
-    args.add(ctx.outputs.out.path)
+    modules_manifest = write_node_modules_manifest(ctx)
     ctx.actions.run(
-        inputs = inputs,
-        outputs = outputs,
+        inputs = ctx.files.deps + [modules_manifest],
+        outputs = [ctx.outputs.out],
         executable = ctx.executable.program,
-        arguments = [args],
+        arguments = [
+            "--bazel_node_modules_manifest=%s" % modules_manifest.path,
+            ctx.outputs.out.path,
+        ],
     )
 
 linked = rule(_linked, attrs = {

internal/node/node.bzl

@@ -25,6 +25,7 @@ load("//internal/common:expand_into_runfiles.bzl", "expand_location_into_runfile
 load("//internal/common:module_mappings.bzl", "module_mappings_runtime_aspect")
 load("//internal/common:path_utils.bzl", "strip_external")
 load("//internal/common:windows_utils.bzl", "create_windows_native_launcher_script", "is_windows")
+load("//internal/linker:link_node_modules.bzl", "write_node_modules_manifest")
 load("//internal/node:node_repositories.bzl", "BUILT_IN_NODE_PLATFORMS")
 
 def _trim_package_node_modules(package_name):
@@ -132,18 +133,15 @@ def _to_manifest_path(ctx, file):
 def _to_execroot_path(ctx, file):
     parts = file.path.split("/")
 
-    #print("_to_execroot", file.path, file.is_source)
     if parts[0] == "external":
         if parts[2] == "node_modules":
             # external/npm/node_modules -> node_modules/foo
             # the linker will make sure we can resolve node_modules from npm
             return "/".join(parts[2:])
-    if file.is_source:
-        return file.path
-
-    return ("<ERROR> _to_execroot_path not yet implemented for " + file.path)
+    return file.path
 
 def _nodejs_binary_impl(ctx):
+    node_modules_manifest = write_node_modules_manifest(ctx)
     node_modules = depset(ctx.files.node_modules)
 
     # Also include files from npm fine grained deps as inputs.
@@ -155,14 +153,15 @@ def _nodejs_binary_impl(ctx):
     # Using a depset will allow us to avoid flattening files and sources
     # inside this loop. This should reduce the performances hits,
     # since we don't need to call .to_list()
-    sources = depset()
+    sources_sets = []
 
     for d in ctx.attr.data:
         # TODO: switch to JSModuleInfo when it is available
         if JSNamedModuleInfo in d:
-            sources = depset(transitive = [sources, d[JSNamedModuleInfo].sources])
+            sources_sets.append(d[JSNamedModuleInfo].sources)
         if hasattr(d, "files"):
-            sources = depset(transitive = [sources, d.files])
+            sources_sets.append(d.files)
+    sources = depset(transitive = sources_sets)
 
     _write_loader_script(ctx)
 
@@ -198,6 +197,7 @@ def _nodejs_binary_impl(ctx):
 
     node_tool_files.append(ctx.file._link_modules_script)
     node_tool_files.append(ctx.file._bazel_require_script)
+    node_tool_files.append(node_modules_manifest)
 
     if not ctx.outputs.templated_args_file:
         templated_args = ctx.attr.templated_args
@@ -236,6 +236,7 @@ def _nodejs_binary_impl(ctx):
         "TEMPLATED_expected_exit_code": str(expected_exit_code),
         "TEMPLATED_link_modules_script": _to_manifest_path(ctx, ctx.file._link_modules_script),
         "TEMPLATED_loader_path": script_path,
+        "TEMPLATED_modules_manifest": _to_manifest_path(ctx, node_modules_manifest),
         "TEMPLATED_repository_args": _to_manifest_path(ctx, ctx.file._repository_args),
         "TEMPLATED_script_path": _to_execroot_path(ctx, ctx.file.entry_point),
         "TEMPLATED_vendored_node": "" if is_builtin else strip_external(ctx.file._node.path),

internal/node/node_launcher.sh

@@ -186,6 +186,12 @@ for ARG in "${ALL_ARGS[@]}"; do
     --nobazel_patch_module_resolver)
       MAIN="TEMPLATED_script_path"
       NODE_OPTIONS+=( "--require" "$bazel_require_script" )
+
+      # In this case we should always run the linker
+      # For programs which are called with bazel run or bazel test, there will be no additional runtime
+      # dependencies to link, so we use the default modules_manifest which has only the static dependencies
+      # of the binary itself
+      MODULES_MANIFEST=${MODULES_MANIFEST:-$(rlocation "TEMPLATED_modules_manifest")}
       ;;
     --node_options=*) NODE_OPTIONS+=( "${ARG#--node_options=}" ) ;;
     *) ARGS+=( "$ARG" )

internal/npm_install/generate_build_file.js

@@ -982,7 +982,8 @@ def ${name.replace(/-/g, '_')}(**kwargs):
         nodejs_binary(
             entry_point = "@${WORKSPACE}//:node_modules/${pkg._dir}/${path}",
             install_source_map_support = False,
-            data = [${data.map(p => `"${p}"`).join(', ')}] + kwargs.pop("data", []),${additionalAttributes(pkg, name)}
+            data = [${data.map(p => `"${p}"`).join(', ')}] + kwargs.pop("data", []),
+            templated_args = ["--nobazel_patch_module_resolver"] + kwargs.pop("templated_args", []),${additionalAttributes(pkg, name)}
             **kwargs
         )
 
@@ -991,7 +992,8 @@ def ${name.replace(/-/g, '_')}_test(**kwargs):
     nodejs_test(
       entry_point = "@${WORKSPACE}//:node_modules/${pkg._dir}/${path}",
       install_source_map_support = False,
-      data = [${data.map(p => `"${p}"`).join(', ')}] + kwargs.pop("data", []),${additionalAttributes(pkg, name)}
+      data = [${data.map(p => `"${p}"`).join(', ')}] + kwargs.pop("data", []),
+      templated_args = ["--nobazel_patch_module_resolver"] + kwargs.pop("templated_args", []),${additionalAttributes(pkg, name)}
       **kwargs
     )
 `;

internal/npm_install/generate_build_file.ts

@@ -1084,7 +1084,8 @@ def ${name.replace(/-/g, '_')}(**kwargs):
         nodejs_binary(
             entry_point = "@${WORKSPACE}//:node_modules/${pkg._dir}/${path}",
             install_source_map_support = False,
-            data = [${data.map(p => `"${p}"`).join(', ')}] + kwargs.pop("data", []),${
+            data = [${data.map(p => `"${p}"`).join(', ')}] + kwargs.pop("data", []),
+            templated_args = ["--nobazel_patch_module_resolver"] + kwargs.pop("templated_args", []),${
           additionalAttributes(pkg, name)}
             **kwargs
         )
@@ -1094,7 +1095,8 @@ def ${name.replace(/-/g, '_')}_test(**kwargs):
     nodejs_test(
       entry_point = "@${WORKSPACE}//:node_modules/${pkg._dir}/${path}",
       install_source_map_support = False,
-      data = [${data.map(p => `"${p}"`).join(', ')}] + kwargs.pop("data", []),${
+      data = [${data.map(p => `"${p}"`).join(', ')}] + kwargs.pop("data", []),
+      templated_args = ["--nobazel_patch_module_resolver"] + kwargs.pop("templated_args", []),${
           additionalAttributes(pkg, name)}
       **kwargs
     )

internal/npm_install/test/golden/@gregmagolan/test-a/index.bzl.golden

@@ -8,12 +8,14 @@ def test(**kwargs):
             entry_point = "@fine_grained_goldens//:node_modules/@gregmagolan/test-a/@bin/test.js",
             install_source_map_support = False,
             data = ["@fine_grained_goldens//@gregmagolan/test-a:test-a"] + kwargs.pop("data", []),
+            templated_args = ["--nobazel_patch_module_resolver"] + kwargs.pop("templated_args", []),
             **kwargs
         )
 def test_test(**kwargs):
     nodejs_test(
       entry_point = "@fine_grained_goldens//:node_modules/@gregmagolan/test-a/@bin/test.js",
       install_source_map_support = False,
       data = ["@fine_grained_goldens//@gregmagolan/test-a:test-a"] + kwargs.pop("data", []),
+      templated_args = ["--nobazel_patch_module_resolver"] + kwargs.pop("templated_args", []),
       **kwargs
     )

internal/npm_install/test/golden/jasmine/index.bzl.golden

@@ -8,12 +8,14 @@ def jasmine(**kwargs):
             entry_point = "@fine_grained_goldens//:node_modules/jasmine/bin/jasmine.js",
             install_source_map_support = False,
             data = ["@fine_grained_goldens//jasmine:jasmine"] + kwargs.pop("data", []),
+            templated_args = ["--nobazel_patch_module_resolver"] + kwargs.pop("templated_args", []),
             **kwargs
         )
 def jasmine_test(**kwargs):
     nodejs_test(
       entry_point = "@fine_grained_goldens//:node_modules/jasmine/bin/jasmine.js",
       install_source_map_support = False,
       data = ["@fine_grained_goldens//jasmine:jasmine"] + kwargs.pop("data", []),
+      templated_args = ["--nobazel_patch_module_resolver"] + kwargs.pop("templated_args", []),
       **kwargs
     )

internal/providers/node_runtime_deps_info.bzl

@@ -14,7 +14,7 @@
 
 """ Custom provider that mimics the Runfiles, but doesn't incur the expense of creating the runfiles symlink tree"""
 
-load("//internal/linker:link_node_modules.bzl", "add_arg", "register_node_modules_linker")
+load("//internal/linker:link_node_modules.bzl", "add_arg", "write_node_modules_manifest")
 
 NodeRuntimeDepsInfo = provider(
     doc = """Stores runtime dependencies of a nodejs_binary or nodejs_test
@@ -54,7 +54,9 @@ def run_node(ctx, inputs, arguments, executable, **kwargs):
         extra_inputs = exec_attr[NodeRuntimeDepsInfo].deps.to_list()
         link_data = exec_attr[NodeRuntimeDepsInfo].pkgs
 
-    register_node_modules_linker(ctx, arguments, inputs, link_data)
+    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

packages/node-patches/BUILD.bazel

@@ -12,8 +12,8 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
+load("@build_bazel_rules_nodejs//:index.bzl", "nodejs_test")
 load("@npm_bazel_rollup//:index.bzl", "rollup_bundle")
-load("@npm_node_patches//mocha:index.bzl", "mocha_test")
 load("@npm_node_patches//typescript:index.bzl", "tsc")
 
 package(default_visibility = ["//:__subpackages__"])
@@ -52,12 +52,14 @@ tsc(
     ],
 )
 
-mocha_test(
+# Like the generated mocha_test but we don't want to run the patches before testing the patches
+nodejs_test(
     name = "unit_test",
     args = ["$(location %s)" % s for s in test_js],
     data = js + test_js + [
         "@npm_node_patches//:node_modules",
     ],
+    entry_point = "@npm_node_patches//:node_modules/mocha/bin/mocha",
     tags = [
         "fix-windows",
     ],