fix(builtin): remove unnecessary loader script (#3495)

Author: kormide

examples/angular/WORKSPACE

@@ -38,6 +38,8 @@ http_archive(
     patches = [
         # Updates @bazel/work dep to 4.0.0 inside rules_sass so it is compatible
         "//:io_bazel_rules_sass.patch",
+        # Add missing extension to entrypoint (https://github.com/bazelbuild/rules_sass/pull/142)
+        "//:io_bazel_rules_sass_entrypoint.patch",
     ],
     sha256 = "5313032124ff191eed68efcfbdc6ee9b5198093b2b80a8e640ea34feabbffc69",
     strip_prefix = "rules_sass-1.34.0",

examples/angular/io_bazel_rules_sass_entrypoint.patch

@@ -0,0 +1,12 @@
+index cfa8b8b..394f6c2 100644
+--- a/sass/BUILD
++++ b/sass/BUILD
+@@ -11,7 +11,7 @@ exports_files([
+ # Executable for the sass_binary rule
+ nodejs_binary(
+     name = "sass",
+-    entry_point = "sass_wrapper",
++    entry_point = "sass_wrapper.js",
+     data = [
+         ":sass_wrapper.js",
+         "@build_bazel_rules_sass_deps//sass",

internal/node/BUILD.bazel

@@ -20,7 +20,6 @@ package(default_visibility = ["//visibility:public"])
 exports_files([
     "node_patches.cjs",
     "require_patch.cjs",
-    "loader.cjs",
 ])
 
 bzl_library(
@@ -40,7 +39,6 @@ exports_files([
     "node.bzl",  # Exported to be consumed for generating stardoc.
     "node_repositories.bzl",  # Exported to be consumed for generating stardoc.
     "launcher.sh",
-    "loader.cjs",
 ])
 
 filegroup(

internal/node/launcher.sh

@@ -199,11 +199,11 @@ for ARG in ${ALL_ARGS[@]+"${ALL_ARGS[@]}"}; do
     --bazel_capture_exit_code=*) EXIT_CODE_CAPTURE="${PWD}/${ARG#--bazel_capture_exit_code=}" ;;
     # Outputs nothing on success
     --bazel_silent_on_success=*) SILENT_ON_SUCCESS=true ;;
-    # Disable the node_loader.cjs monkey patches for require()
+    # Disable the require_patch.cjs monkey patches for require()
     # Note that this means you need an explicit runfiles helper library
     # This flag is now a no-op since the default is also false
     --nobazel_patch_module_resolver) PATCH_REQUIRE=false ;;
-    # Enable the node_loader.cjs monkey patches for require()
+    # Enable the require_patch.cjs monkey patches for require()
     --bazel_patch_module_resolver) PATCH_REQUIRE=true ;;
     # Disable the --require node-patches (undocumented and unused; only here as an escape value)
     --nobazel_node_patches) NODE_PATCHES=false ;;
@@ -331,26 +331,24 @@ if [ "$PATCH_REQUIRE" = true ]; then
     *           ) require_patch_script="${PWD}/${require_patch_script}" ;;
   esac
   LAUNCHER_NODE_OPTIONS+=( "--require" "$require_patch_script" )
-  # Change the entry point to be the loader.cjs script so we run code before node
-  MAIN=$(rlocation "TEMPLATED_loader_script")
-else
-  # Entry point is the user-supplied script
-  MAIN="${PWD}/"TEMPLATED_entry_point_execroot_path
-  # TODO: after we link-all-bins we should not need this extra lookup
-  if [[ ! -e "$MAIN" ]]; then
-    if [ "$FROM_EXECROOT" = true ]; then
-      MAIN="$EXECROOT/"TEMPLATED_entry_point_execroot_path
-    else
-      MAIN=TEMPLATED_entry_point_manifest_path
-    fi
-  fi
-  # Always set up source-map-support using our vendored copy, just like the require_patch_script
-  register_source_map_support=$(rlocation build_bazel_rules_nodejs/third_party/github.com/source-map-support/register.js)
-  LAUNCHER_NODE_OPTIONS+=( "--require" "${register_source_map_support}" )
-  if [[ -n "TEMPLATED_entry_point_main" ]]; then
-    MAIN="${MAIN}/"TEMPLATED_entry_point_main
+fi
+
+# Entry point is the user-supplied script
+MAIN="${PWD}/"TEMPLATED_entry_point_execroot_path
+# TODO: after we link-all-bins we should not need this extra lookup
+if [[ ! -e "$MAIN" ]]; then
+  if [ "$FROM_EXECROOT" = true ]; then
+    MAIN="$EXECROOT/"TEMPLATED_entry_point_execroot_path
+  else
+    MAIN=TEMPLATED_entry_point_manifest_path
   fi
 fi
+# Always set up source-map-support using our vendored copy, just like the require_patch_script
+register_source_map_support=$(rlocation build_bazel_rules_nodejs/third_party/github.com/source-map-support/register.js)
+LAUNCHER_NODE_OPTIONS+=( "--require" "${register_source_map_support}" )
+if [[ -n "TEMPLATED_entry_point_main" ]]; then
+  MAIN="${MAIN}/"TEMPLATED_entry_point_main
+fi
 
 if [ "${SILENT_ON_SUCCESS:-}" = true ]; then
   if [[ -z "${STDOUT_CAPTURE}" ]]; then
@@ -484,3 +482,4 @@ if [[ -n "${EXIT_CODE_CAPTURE}" ]]; then
 else
   exit ${RESULT}
 fi
+

internal/node/loader.cjs

@@ -117,21 +117,6 @@ def _get_entry_point_file(ctx):
         return ctx.attr.entry_point[DirectoryFilePathInfo].directory
     fail("entry_point must either be a file, or provide DirectoryFilePathInfo")
 
-def _write_loader_script(ctx):
-    substitutions = {}
-    substitutions["TEMPLATED_entry_point_path"] = _ts_to_js(_to_manifest_path(ctx, _get_entry_point_file(ctx)))
-    if DirectoryFilePathInfo in ctx.attr.entry_point:
-        substitutions["TEMPLATED_entry_point_main"] = ctx.attr.entry_point[DirectoryFilePathInfo].path
-    else:
-        substitutions["TEMPLATED_entry_point_main"] = ""
-
-    ctx.actions.expand_template(
-        template = ctx.file._loader_template,
-        output = ctx.outputs.loader_script,
-        substitutions = substitutions,
-        is_executable = True,
-    )
-
 # Avoid using non-normalized paths (workspace/../other_workspace/path)
 def _to_manifest_path(ctx, file):
     if file.short_path.startswith("../"):
@@ -208,8 +193,6 @@ def _nodejs_binary_impl(ctx, data = [], runfiles = [], expanded_args = []):
         node_modules_root = "build_bazel_rules_nodejs/node_modules"
     _write_require_patch_script(ctx, data, node_modules_root)
 
-    _write_loader_script(ctx)
-
     # Provide the target name as an environment variable avaiable to all actions for the
     # runfiles helpers to use.
     env_vars = "export BAZEL_TARGET=%s\n" % ctx.label
@@ -276,7 +259,6 @@ fi
     runfiles = runfiles[:]
     runfiles.extend(node_tool_files)
     runfiles.extend(ctx.files._bash_runfile_helper)
-    runfiles.append(ctx.outputs.loader_script)
     runfiles.append(ctx.outputs.require_patch_script)
 
     # First replace any instances of "$(rlocation " with "$$(rlocation " to preserve
@@ -331,7 +313,6 @@ if (process.cwd() !== __dirname) {
         "TEMPLATED_expected_exit_code": str(expected_exit_code),
         "TEMPLATED_lcov_merger_script": _to_manifest_path(ctx, ctx.file._lcov_merger_script),
         "TEMPLATED_link_modules_script": _to_manifest_path(ctx, ctx.file._link_modules_script),
-        "TEMPLATED_loader_script": _to_manifest_path(ctx, ctx.outputs.loader_script),
         "TEMPLATED_modules_manifest": _to_manifest_path(ctx, node_modules_manifest),
         "TEMPLATED_node_patches_script": _to_manifest_path(ctx, ctx.file._node_patches_script),
         "TEMPLATED_require_patch_script": _to_manifest_path(ctx, ctx.outputs.require_patch_script),
@@ -378,7 +359,6 @@ if (process.cwd() !== __dirname) {
             runfiles = ctx.runfiles(
                 transitive_files = depset(runfiles),
                 files = node_tool_files + [
-                            ctx.outputs.loader_script,
                             ctx.outputs.require_patch_script,
                         ] + ctx.files._source_map_support_files +
 
@@ -608,10 +588,6 @@ Predefined genrule variables are not supported in this context.
         default = Label("//internal/linker:index.js"),
         allow_single_file = True,
     ),
-    "_loader_template": attr.label(
-        default = Label("//internal/node:loader.cjs"),
-        allow_single_file = True,
-    ),
     "toolchain": attr.label(),
     "_node_args": attr.label(default = "@rules_nodejs//nodejs:default_args"),
     "_node_patches_script": attr.label(
@@ -642,7 +618,6 @@ Predefined genrule variables are not supported in this context.
 
 _NODEJS_EXECUTABLE_OUTPUTS = {
     "launcher_sh": "%{name}.sh",
-    "loader_script": "%{name}_loader.cjs",
     "require_patch_script": "%{name}_require_patch.cjs",
 }
 

internal/node/node.bzl

@@ -1,4 +1,4 @@
-load("@build_bazel_rules_nodejs//:index.bzl", "nodejs_binary")
+load("@build_bazel_rules_nodejs//:index.bzl", "nodejs_binary", "npm_package_bin")
 
 nodejs_binary(
     name = "has_deps",
@@ -12,3 +12,16 @@ nodejs_binary(
     name = "no_deps",
     entry_point = ":no-deps.mjs",
 )
+
+nodejs_binary(
+    name = "no_deps_linker_disabled",
+    entry_point = ":no-deps-create-file.mjs",
+    templated_args = ["--nobazel_run_linker"],
+)
+
+npm_package_bin(
+    name = "run_with_linker_disabled",
+    outs = ["out.txt"],
+    args = ["$@"],
+    tool = ":no_deps_linker_disabled",
+)

internal/node/test/esm/BUILD.bazel

@@ -0,0 +1,4 @@
+import {writeFileSync} from "fs";
+
+const outputFile = process.argv[2];
+writeFileSync(outputFile, "foobar\n");