refactor: stop using loader script, try running program directly

internal/linker/link_node_modules.bzl

@@ -45,7 +45,6 @@ def register_node_modules_linker(ctx, args, inputs):
         # as well as the root directory for the third-party node_modules;
         # we'll symlink the local "node_modules" to it
         if NpmPackageInfo in dep:
-            print("found npm package info in ", dep)
             possible_root = "/".join([dep[NpmPackageInfo].workspace, "node_modules"])
             if not node_modules_root:
                 node_modules_root = possible_root
@@ -70,7 +69,11 @@ def register_node_modules_linker(ctx, args, inputs):
         "workspace": ctx.workspace_name,
     }
     ctx.actions.write(modules_manifest, str(content))
-    args.add("--bazel_node_modules_manifest=%s" % modules_manifest.path)
+    arg = "--bazel_node_modules_manifest=%s" % modules_manifest.path
+    if (type(args) == type([])):
+        args.append(arg)
+    else:
+        args.add(arg)
     inputs.append(modules_manifest)
 
 def get_module_mappings(label, attrs, vars, rule_kind, srcs = [], workspace_name = None):

internal/node/node.bzl

@@ -93,8 +93,7 @@ def _write_loader_script(ctx):
     if len(ctx.attr.entry_point.files.to_list()) != 1:
         fail("labels in entry_point must contain exactly one file")
 
-    #entry_point_path = _to_manifest_path(ctx, ctx.file.entry_point)
-    entry_point_path = "./node_modules/rollup/bin/rollup"
+    entry_point_path = _to_execroot_path(ctx, ctx.file.entry_point)
 
     # If the entry point specified is a typescript file then set the entry
     # point to the corresponding .js file
@@ -127,7 +126,29 @@ def _to_manifest_path(ctx, file):
     else:
         return ctx.workspace_name + "/" + file.short_path
 
+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:])
+
+        # external/other_repo/some_pkg -> external/other_repo/some_pkg
+        return file.path
+
+    # some_pkg -> some_pkg
+    if file.is_source:
+        return file.short_path
+
+    return "/".join([ctx.workspace_name, file.path])
+
 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()
     node_modules = depset(ctx.files.node_modules)
 
     # Also include files from npm fine grained deps as inputs.
@@ -136,21 +157,23 @@ def _nodejs_binary_impl(ctx):
         if NpmPackageInfo in d:
             node_modules = depset(transitive = [node_modules, d[NpmPackageInfo].sources])
 
-    # 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()
-
     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])
+            node_modules = depset(transitive = [node_modules, d[JSNamedModuleInfo].sources])
         if hasattr(d, "files"):
-            sources = depset(transitive = [sources, d.files])
+            node_modules = depset(transitive = [node_modules, d.files])
 
-    _write_loader_script(ctx)
+    # _write_loader_script(ctx)
+    if len(ctx.attr.entry_point.files.to_list()) != 1:
+        fail("labels in entry_point must contain exactly one file")
 
-    script_path = _to_manifest_path(ctx, ctx.outputs.loader)
+    entry_point_path = _to_execroot_path(ctx, ctx.file.entry_point)
+
+    # If the entry point specified is a typescript file then set the entry
+    # point to the corresponding .js file
+    if entry_point_path.endswith(".ts"):
+        entry_point_path = entry_point_path[:-3] + ".js"
 
     env_vars = "export BAZEL_TARGET=%s\n" % ctx.label
     for k in ctx.attr.configuration_env_vars + ctx.attr.default_env_vars:
@@ -208,7 +231,7 @@ def _nodejs_binary_impl(ctx):
         "TEMPLATED_link_modules_script": _to_manifest_path(ctx, ctx.file._link_modules_script),
         "TEMPLATED_node": node_tool_info.target_tool_path,
         "TEMPLATED_repository_args": _to_manifest_path(ctx, ctx.file._repository_args),
-        "TEMPLATED_script_path": script_path,
+        "TEMPLATED_script_path": entry_point_path,
     }
     ctx.actions.expand_template(
         template = ctx.file._launcher_template,
@@ -217,7 +240,7 @@ def _nodejs_binary_impl(ctx):
         is_executable = True,
     )
 
-    runfiles = depset(node_tool_files + ctx.files._bash_runfile_helpers + [ctx.outputs.loader, ctx.file._repository_args], transitive = [sources])
+    runfiles = depset(node_tool_files + ctx.files._bash_runfile_helpers + [ctx.file._repository_args])
 
     if is_windows(ctx):
         runfiles = depset([ctx.outputs.script], transitive = [runfiles])
@@ -234,15 +257,7 @@ def _nodejs_binary_impl(ctx):
             executable = executable,
             runfiles = ctx.runfiles(
                 transitive_files = runfiles,
-                files = node_tool_files + [
-                            ctx.outputs.loader,
-                        ] + ctx.files._source_map_support_files +
-
-                        # We need this call to the list of Files.
-                        # Calling the .to_list() method may have some perfs hits,
-                        # so we should be running this method only once per rule.
-                        # see: https://docs.bazel.build/versions/master/skylark/depsets.html#performance
-                        sources.to_list(),
+                files = node_tool_files + ctx.files._source_map_support_files,
                 collect_data = True,
             ),
         ),
@@ -439,10 +454,6 @@ jasmine_node_test(
         default = Label("//internal/linker:index.js"),
         allow_single_file = True,
     ),
-    "_loader_template": attr.label(
-        default = Label("//internal/node:node_loader.js"),
-        allow_single_file = True,
-    ),
     "_repository_args": attr.label(
         default = Label("@nodejs//:bin/node_repo_args.sh"),
         allow_single_file = True,
@@ -458,7 +469,6 @@ jasmine_node_test(
 }
 
 _NODEJS_EXECUTABLE_OUTPUTS = {
-    "loader": "%{name}_loader.js",
     "script": "%{name}.sh",
 }
 

internal/node/node_launcher.sh

@@ -116,7 +116,7 @@ TEMPLATED_env_vars
 
 readonly node=$(rlocation "TEMPLATED_node")
 readonly repository_args=$(rlocation "TEMPLATED_repository_args")
-readonly script=$(rlocation "TEMPLATED_script_path")
+readonly script="./TEMPLATED_script_path"
 readonly link_modules_script=$(rlocation "TEMPLATED_link_modules_script")
 
 source $repository_args
@@ -135,6 +135,24 @@ done
 # Link the first-party modules into node_modules directory before running the actual program
 if [[ -n "$MODULES_MANIFEST" ]]; then
   "${node}" "${link_modules_script}" "${MODULES_MANIFEST}"
+else
+  # Maybe we should run linker anyway?
+  # cheap version:
+  echo "Did not run the linker" >&2
+  # if [[ ! -d node_modules ]]; then
+  #   mkdir node_modules
+  #   ln -s "." "node_modules/$(basename $PWD)"
+  # fi
+fi
+
+find . -name rollup >&2
+ls -R node_modules
+export NODE_DEBUG=module
+
+if [[ ! -e ${script} ]]; then
+  echo "the entry point script ${script} is not on the disk"
+
+  exit 1
 fi
 
 # The EXPECTED_EXIT_CODE lets us write bazel tests which assert that

internal/npm_package/npm_package.bzl

@@ -7,6 +7,7 @@ to the `deps` of one of their targets.
 """
 
 load("@build_bazel_rules_nodejs//:providers.bzl", "DeclarationInfo", "JSNamedModuleInfo")
+load("@build_bazel_rules_nodejs//internal/providers:runtime_deps_info.bzl", "NodeRuntimeDepsInfo")
 
 # Takes a depset of files and returns a corresponding list of file paths without any files
 # that aren't part of the specified package path. Also include files from external repositories
@@ -77,6 +78,9 @@ def create_package(ctx, deps_sources, nested_packages):
     if ctx.version_file:
         inputs.append(ctx.version_file)
 
+    if (NodeRuntimeDepsInfo in ctx.attr._packager):
+        inputs.extend(ctx.attr._packager[NodeRuntimeDepsInfo].deps.to_list())
+
     ctx.actions.run(
         progress_message = "Assembling npm package %s" % package_dir.short_path,
         executable = ctx.executable._packager,

internal/web_package/web_package.bzl

@@ -15,6 +15,8 @@
 """Contains the web_package rule.
 """
 
+load("@build_bazel_rules_nodejs//internal/providers:runtime_deps_info.bzl", "NodeRuntimeDepsInfo")
+
 def html_asset_inject(index_html, action_factory, injector, root_dirs, assets, output):
     """Injects JS and CSS resources into the index.html.
 
@@ -36,8 +38,12 @@ def html_asset_inject(index_html, action_factory, injector, root_dirs, assets, o
     args.add("--assets")
     args.add_all(assets)
     args.use_param_file("%s", use_always = True)
+    inputs = [index_html]
+    if (NodeRuntimeDepsInfo in injector):
+        inputs.extend(injector[NodeRuntimeDepsInfo].deps.to_list())
+
     action_factory.run(
-        inputs = [index_html],
+        inputs = inputs,
         outputs = [output],
         executable = injector,
         arguments = [args],
@@ -64,6 +70,9 @@ def move_files(output_name, files, action_factory, assembler, root_paths):
     args.add("--assets")
     args.add_all([f.path for f in files])
     args.use_param_file("%s", use_always = True)
+    inputs = files
+    if (NodeRuntimeDepsInfo in assembler):
+        inputs.extend(assembler[NodeRuntimeDepsInfo].deps.to_list())
     action_factory.run(
         inputs = files,
         outputs = [www_dir],

packages/rollup/src/rollup_bundle.bzl

@@ -256,10 +256,6 @@ def _rollup_bundle(ctx):
     deps_inputs = depset(transitive = deps_depsets).to_list()
 
     inputs = _filter_js(ctx.files.entry_point) + _filter_js(ctx.files.entry_points) + ctx.files.srcs + deps_inputs
-
-    if (NodeRuntimeDepsInfo in ctx.attr.rollup_bin):
-        inputs.extend(ctx.attr.rollup_bin[NodeRuntimeDepsInfo].deps.to_list())
-
     outputs = [getattr(ctx.outputs, o) for o in dir(ctx.outputs)]
 
     # See CLI documentation at https://rollupjs.org/guide/en/#command-line-reference
@@ -307,6 +303,8 @@ def _rollup_bundle(ctx):
     if (ctx.attr.sourcemap and ctx.attr.sourcemap != "false"):
         args.add_all(["--sourcemap", ctx.attr.sourcemap])
 
+    if (NodeRuntimeDepsInfo in ctx.attr.rollup_bin):
+        inputs.extend(ctx.attr.rollup_bin[NodeRuntimeDepsInfo].deps.to_list())
     ctx.actions.run(
         progress_message = "Bundling JavaScript %s [rollup]" % outputs[0].short_path,
         executable = ctx.executable.rollup_bin,

packages/typescript/src/internal/build_defs.bzl

@@ -15,6 +15,8 @@
 "TypeScript compilation"
 
 load("@build_bazel_rules_nodejs//:providers.bzl", "NpmPackageInfo", "js_ecma_script_module_info", "js_named_module_info", "node_modules_aspect")
+load("@build_bazel_rules_nodejs//internal/linker:link_node_modules.bzl", "register_node_modules_linker")
+load("@build_bazel_rules_nodejs//internal/providers:runtime_deps_info.bzl", "NodeRuntimeDepsInfo")
 
 # pylint: disable=unused-argument
 # pylint: disable=missing-docstring
@@ -142,6 +144,10 @@ def _compile_action(ctx, inputs, outputs, tsconfig_file, node_opts, description
         arguments.append(tsconfig_file.path)
         mnemonic = "tsc"
 
+    if (NodeRuntimeDepsInfo in ctx.attr.compiler):
+        action_inputs.extend(ctx.attr.compiler[NodeRuntimeDepsInfo].deps.to_list())
+    register_node_modules_linker(ctx, arguments, action_inputs)
+
     ctx.actions.run(
         progress_message = "Compiling TypeScript (%s) %s" % (description, ctx.label),
         mnemonic = mnemonic,