refactor: default to nobazel_patch_module_resolver

This turns off our monkey-patches for require() for nodejs_binary, nodejs_test, and macros like jasmine_node_test.
Note it was previously disabled for npm_package_bin and generated index.bzl binaries in bazel-contrib#1440

BREAKING CHANGE:
we no longer patch the require() function, instead you should rely on the linker to make node modules resolvable at the standard location
if this breaks you, try opting-in to the require patches with templated_args = ["--bazel_patch_module_resolver"]

Fixes bazel-contrib#2125

e2e/nodejs_image/BUILD.bazel

@@ -9,8 +9,6 @@ nodejs_binary(
         "@npm//date-fns",
     ],
     entry_point = "main.js",
-    # Turn off require() monkey patches so linker is required to run inside docker container
-    templated_args = ["--nobazel_patch_module_resolver"],
 )
 
 # bazel run --platforms=@build_bazel_rules_nodejs//toolchains/node:linux_amd64 //:nodejs_image

examples/nestjs/src/BUILD.bazel

@@ -61,7 +61,6 @@ nodejs_binary(
         "@npm//minimist",
     ],
     entry_point = ":main.ts",
-    templated_args = ["--nobazel_patch_module_resolver"],
 )
 
 jasmine_node_test(

examples/user_managed_deps/BUILD.bazel

@@ -43,6 +43,7 @@ jasmine_node_test(
     srcs = glob(["*.spec.js"]),
     node_modules = "//:node_modules",
     tags = ["no-local-jasmine-deps"],
+    templated_args = ["--bazel_patch_module_resolver"],
     deps = [
         ":decrement",
         ":program",

internal/BUILD.bazel

@@ -32,7 +32,10 @@ nodejs_test(
         "//:.bazelversion",
     ],
     entry_point = ":check_bazel_version.js",
-    templated_args = [BAZEL_VERSION],
+    templated_args = [
+        "--bazel_patch_module_resolver",
+        BAZEL_VERSION,
+    ],
 )
 
 # Detect if the build is running under --stamp

internal/bazel_integration_test/BUILD.bazel

@@ -22,7 +22,10 @@ nodejs_binary(
     configuration_env_vars = ["BAZEL_INTEGRATION_TEST_DEBUG"],
     data = ["@npm//tmp"],
     entry_point = ":test_runner.js",
-    templated_args = ["--node_options=--max-old-space-size=1024"],
+    templated_args = [
+        "--bazel_patch_module_resolver",
+        "--node_options=--max-old-space-size=1024",
+    ],
 )
 
 filegroup(

internal/linker/test/local/BUILD.bazel

@@ -4,6 +4,5 @@ jasmine_node_test(
     name = "test",
     srcs = ["test.js"],
     tags = ["local"],
-    templated_args = ["--nobazel_patch_module_resolver"],
     deps = ["//internal/linker/test/local/fit"],
 )

internal/linker/test/workspace_link/BUILD.bazel

@@ -4,7 +4,6 @@ jasmine_node_test(
     name = "test",
     srcs = ["test.js"],
     link_workspace_root = True,
-    templated_args = ["--nobazel_patch_module_resolver"],
     deps = [
         "//internal/linker/test/workspace_link/bar",
         "//internal/linker/test/workspace_link/foo",

internal/node/launcher.sh

@@ -179,8 +179,7 @@ EXIT_CODE_CAPTURE=""
 
 RUN_LINKER=true
 NODE_PATCHES=true
-# TODO(alex): change the default to false
-PATCH_REQUIRE=true
+PATCH_REQUIRE=false
 for ARG in ${ALL_ARGS[@]+"${ALL_ARGS[@]}"}; do
   case "$ARG" in
     # Supply custom linker arguments for first-party dependencies
@@ -193,10 +192,9 @@ for ARG in ${ALL_ARGS[@]+"${ALL_ARGS[@]}"}; do
     --bazel_capture_exit_code=*) EXIT_CODE_CAPTURE="${ARG#--bazel_capture_exit_code=}" ;;
     # Disable the node_loader.js 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.js monkey patches for require()
-    # Currently a no-op, but specifying this makes the behavior unchanged when we update
-    # the default for PATCH_REQUIRE above
     --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 ;;

internal/node/test/BUILD.bazel

@@ -22,6 +22,7 @@ nodejs_binary(
     data = ["has-deps.js"],
     entry_point = ":has-deps.js",
     node_modules = "@fine_grained_deps_yarn//:node_modules",
+    templated_args = ["--bazel_patch_module_resolver"],
 )
 
 # You can have a nodejs_binary with no node_modules attribute
@@ -101,6 +102,7 @@ nodejs_test(
         "@npm//node_resolve_nested_main",
     ],
     entry_point = ":module_resolution.spec.js",
+    templated_args = ["--bazel_patch_module_resolver"],
 )
 
 nodejs_test(
@@ -218,6 +220,7 @@ nodejs_test(
         "@npm//node_resolve_nested_main",
     ],
     entry_point = ":module_resolution_built.spec.js",
+    templated_args = ["--bazel_patch_module_resolver"],
 )
 
 nodejs_test(
@@ -331,14 +334,6 @@ jasmine_node_test(
         "dir_output",
         "minified.js",
     ],
-    # Turn on the linker & turn off require patches so that the external workspace jasmine_node_test
-    # entry point npm/@bazel/jasmine/jasmine_runner.js's require('@bazel/jasmine') is exercised without
-    # require patches.
-    templated_args = select({
-        # TODO: fix this linker assertion on Windows
-        "@bazel_tools//src/conditions:host_windows": [],
-        "//conditions:default": ["--nobazel_patch_module_resolver"],
-    }),
 )
 
 [nodejs_toolchain_test(
@@ -491,5 +486,4 @@ nodejs_test(
     name = "main_test",
     data = [":main_lib"],
     entry_point = ":main.js",
-    templated_args = ["--nobazel_patch_module_resolver"],
 )

internal/npm_install/BUILD.bazel

@@ -56,5 +56,6 @@ nodejs_binary(
         "//third_party/npm/node_modules/named-amd",
     ],
     entry_point = ":browserify-wrapped.js",
+    templated_args = ["--bazel_patch_module_resolver"],
     visibility = ["//visibility:public"],
 )

internal/npm_install/generate_build_file.ts

@@ -1108,8 +1108,7 @@ export function printPackageBin(pkg: Dep) {
 nodejs_binary(
     name = "${name}",
     entry_point = "//:node_modules/${pkg._dir}/${path}",
-    data = [${data.map(p => `"${p}"`).join(', ')}],
-    templated_args = ["--nobazel_patch_module_resolver"],${additionalAttributes(pkg, name)}
+    data = [${data.map(p => `"${p}"`).join(', ')}],${additionalAttributes(pkg, name)}
 )
 `;
     }
@@ -1144,8 +1143,7 @@ def ${name.replace(/-/g, '_')}(**kwargs):
         nodejs_binary(
             entry_point = "@${WORKSPACE}//:node_modules/${pkg._dir}/${path}",
             data = [${data.map(p => `"${p}"`).join(', ')}] + kwargs.pop("data", []),
-            templated_args = ["--nobazel_patch_module_resolver"] + kwargs.pop("templated_args", []),${
-          additionalAttributes(pkg, name)}
+            templated_args = kwargs.pop("templated_args", []),${additionalAttributes(pkg, name)}
             **kwargs
         )
 
@@ -1154,8 +1152,7 @@ def ${name.replace(/-/g, '_')}_test(**kwargs):
     nodejs_test(
       entry_point = "@${WORKSPACE}//:node_modules/${pkg._dir}/${path}",
       data = [${data.map(p => `"${p}"`).join(', ')}] + kwargs.pop("data", []),
-      templated_args = ["--nobazel_patch_module_resolver"] + kwargs.pop("templated_args", []),${
-          additionalAttributes(pkg, name)}
+      templated_args = kwargs.pop("templated_args", []),${additionalAttributes(pkg, name)}
       **kwargs
     )
 `;

internal/npm_install/index.js

@@ -673,8 +673,7 @@ function printPackageBin(pkg) {
 nodejs_binary(
     name = "${name}",
     entry_point = "//:node_modules/${pkg._dir}/${path}",
-    data = [${data.map(p => `"${p}"`).join(', ')}],
-    templated_args = ["--nobazel_patch_module_resolver"],${additionalAttributes(pkg, name)}
+    data = [${data.map(p => `"${p}"`).join(', ')}],${additionalAttributes(pkg, name)}
 )
 `;
         }
@@ -706,7 +705,7 @@ def ${name.replace(/-/g, '_')}(**kwargs):
         nodejs_binary(
             entry_point = "@${WORKSPACE}//:node_modules/${pkg._dir}/${path}",
             data = [${data.map(p => `"${p}"`).join(', ')}] + kwargs.pop("data", []),
-            templated_args = ["--nobazel_patch_module_resolver"] + kwargs.pop("templated_args", []),${additionalAttributes(pkg, name)}
+            templated_args = kwargs.pop("templated_args", []),${additionalAttributes(pkg, name)}
             **kwargs
         )
 
@@ -715,7 +714,7 @@ def ${name.replace(/-/g, '_')}_test(**kwargs):
     nodejs_test(
       entry_point = "@${WORKSPACE}//:node_modules/${pkg._dir}/${path}",
       data = [${data.map(p => `"${p}"`).join(', ')}] + kwargs.pop("data", []),
-      templated_args = ["--nobazel_patch_module_resolver"] + kwargs.pop("templated_args", []),${additionalAttributes(pkg, name)}
+      templated_args = kwargs.pop("templated_args", []),${additionalAttributes(pkg, name)}
       **kwargs
     )
 `;

internal/npm_install/test/BUILD.bazel

@@ -18,6 +18,7 @@ jasmine_node_test(
         "@fine_grained_goldens//:golden_files",
         "@npm//unidiff",
     ],
+    templated_args = ["--bazel_patch_module_resolver"],
 )
 
 nodejs_binary(
@@ -81,6 +82,7 @@ jasmine_node_test(
         "//internal/npm_install:browserify-wrapped",
         "@npm//date-fns:date-fns.umd.js",
     ],
+    templated_args = ["--bazel_patch_module_resolver"],
 )
 
 sh_test(

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

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

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

@@ -7,13 +7,13 @@ def test(**kwargs):
         nodejs_binary(
             entry_point = "@fine_grained_goldens//:node_modules/@gregmagolan/test-a/@bin/test.js",
             data = ["@fine_grained_goldens//@gregmagolan/test-a:test-a"] + kwargs.pop("data", []),
-            templated_args = ["--nobazel_patch_module_resolver"] + kwargs.pop("templated_args", []),
+            templated_args = kwargs.pop("templated_args", []),
             **kwargs
         )
 def test_test(**kwargs):
     nodejs_test(
       entry_point = "@fine_grained_goldens//:node_modules/@gregmagolan/test-a/@bin/test.js",
       data = ["@fine_grained_goldens//@gregmagolan/test-a:test-a"] + kwargs.pop("data", []),
-      templated_args = ["--nobazel_patch_module_resolver"] + kwargs.pop("templated_args", []),
+      templated_args = kwargs.pop("templated_args", []),
       **kwargs
     )

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

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

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

@@ -7,13 +7,13 @@ def jasmine(**kwargs):
         nodejs_binary(
             entry_point = "@fine_grained_goldens//:node_modules/jasmine/bin/jasmine.js",
             data = ["@fine_grained_goldens//jasmine:jasmine"] + kwargs.pop("data", []),
-            templated_args = ["--nobazel_patch_module_resolver"] + kwargs.pop("templated_args", []),
+            templated_args = kwargs.pop("templated_args", []),
             **kwargs
         )
 def jasmine_test(**kwargs):
     nodejs_test(
       entry_point = "@fine_grained_goldens//:node_modules/jasmine/bin/jasmine.js",
       data = ["@fine_grained_goldens//jasmine:jasmine"] + kwargs.pop("data", []),
-      templated_args = ["--nobazel_patch_module_resolver"] + kwargs.pop("templated_args", []),
+      templated_args = kwargs.pop("templated_args", []),
       **kwargs
     )

internal/pkg_npm/BUILD.bazel

@@ -16,6 +16,7 @@ nodejs_binary(
     name = "packager",
     data = ["//third_party/github.com/gjtorikian/isBinaryFile"],
     entry_point = ":packager.js",
+    templated_args = ["--bazel_patch_module_resolver"],
 )
 
 nodejs_binary(

internal/pkg_npm/test/directory/BUILD.bazel

@@ -22,5 +22,4 @@ nodejs_test(
         ":folder_pkg",
     ],
     entry_point = "main.js",
-    templated_args = ["--nobazel_patch_module_resolver"],
 )

internal/pkg_npm/test/linking/BUILD.bazel

@@ -86,5 +86,4 @@ nodejs_test(
         "//internal/pkg_npm/test/linking/fuz:scoped_fuz",
     ],
     entry_point = ":main.js",
-    templated_args = ["--nobazel_patch_module_resolver"],
 )

internal/pkg_web/BUILD.bazel

@@ -27,6 +27,7 @@ nodejs_binary(
     ],
     entry_point = ":assembler.js",
     node_modules = ":node_modules_none",
+    templated_args = ["--bazel_patch_module_resolver"],
 )
 
 # BEGIN-INTERNAL

internal/pkg_web/test2/BUILD.bazel

@@ -98,4 +98,5 @@ jasmine_node_test(
     tags = [
         "fix-windows",
     ],
+    templated_args = ["--bazel_patch_module_resolver"],
 )

internal/providers/node_runtime_deps_info.bzl

@@ -112,11 +112,6 @@ def run_node(ctx, inputs, arguments, executable, **kwargs):
         add_arg(arguments, "--bazel_capture_exit_code=%s" % exit_code_file.path)
         outputs = outputs + [exit_code_file]
 
-    # By using the run_node helper, you suggest that your program
-    # doesn't implicitly use runfiles to require() things
-    # To access runfiles, you must use a runfiles helper in the program instead
-    add_arg(arguments, "--nobazel_patch_module_resolver")
-
     env = kwargs.pop("env", {})
 
     # Always forward the COMPILATION_MODE to node process as an environment variable

packages/concatjs/web_test/test/karma_typescript/BUILD.bazel

@@ -86,4 +86,5 @@ jasmine_node_test(
     data = [
         ":testing_chromium-local",
     ],
+    templated_args = ["--bazel_patch_module_resolver"],
 )

packages/cypress/internal/template.cypress_web_test.bzl

@@ -112,7 +112,6 @@ def cypress_web_test(
         ] + srcs,
         entry_point = "@build_bazel_rules_nodejs//packages/cypress:internal/run-cypress.js",
         templated_args = [
-            "--nobazel_patch_module_resolver",
             "$(rootpath {config_file})".format(config_file = config_file),
             "$(rootpath {cypress_plugin})".format(cypress_plugin = cypress_plugin),
             "$(rootpath {cypress_archive})".format(cypress_archive = cypress_archive),
@@ -154,7 +153,6 @@ def cypress_web_test_global_cache(
         ] + srcs,
         entry_point = "@build_bazel_rules_nodejs//packages/cypress:internal/run-cypress.js",
         templated_args = [
-            "--nobazel_patch_module_resolver",
             "$(rootpath {config_file})".format(config_file = config_file),
             "$(rootpath {cypress_plugin})".format(cypress_plugin = cypress_plugin),
         ] + templated_args,

packages/labs/grpc_web/BUILD.bazel

@@ -15,6 +15,7 @@ nodejs_binary(
     name = "change_import_style",
     entry_point = ":change_import_style.js",
     node_modules = "@build_bazel_rules_typescript_grpc_web_compiletime_deps//:node_modules",
+    templated_args = ["--bazel_patch_module_resolver"],
     visibility = ["//visibility:public"],
 )
 

packages/labs/test/grpc_web/BUILD.bazel

@@ -10,6 +10,7 @@ jasmine_node_test(
         "@npm//google-protobuf",
         "@npm//grpc-web",
     ],
+    templated_args = ["--bazel_patch_module_resolver"],
     deps = [
         ":commonjs_test_lib",
     ],

packages/labs/test/protobufjs/BUILD.bazel

@@ -25,6 +25,7 @@ protobufjs_ts_library(
 jasmine_node_test(
     name = "protobuf_test",
     srcs = [":test"],
+    templated_args = ["--bazel_patch_module_resolver"],
     deps = [
         "@npm//protobufjs",
     ],

packages/node-patches/BUILD.bazel

@@ -60,7 +60,13 @@ nodejs_test(
     ],
     entry_point = "@npm_node_patches//:node_modules/mocha/bin/mocha",
     tags = ["fix-windows"],
-    templated_args = ["$$(rlocation $(rootpath %s))" % s for s in test_js] + ["--nobazel_node_patches"],
+    templated_args = [
+        "$$(rlocation $(rootpath %s))" % s
+        for s in test_js
+    ] + [
+        "--nobazel_node_patches",
+        "--bazel_patch_module_resolver",
+    ],
 )
 
 rollup_bundle(

packages/protractor/protractor_web_test.bzl

@@ -261,6 +261,7 @@ def protractor_web_test(
         entry_point = Label(protractor_entry_point),
         data = srcs + deps + data + [Label(d) for d in peer_deps],
         testonly = 1,
+        templated_args = ["--bazel_patch_module_resolver"],
         visibility = ["//visibility:private"],
     )
 

packages/protractor/test/protractor-utils/BUILD.bazel

@@ -29,4 +29,5 @@ jasmine_node_test(
     data = [
         ":fake-devserver",
     ],
+    templated_args = ["--bazel_patch_module_resolver"],
 )

packages/rollup/install.md

@@ -144,7 +144,6 @@ nodejs_binary(
     data = ["@npm//rollup:rollup"],
     entry_point = "@npm//:node_modules/rollup/dist/bin/rollup",
     templated_args = [
-        "--nobazel_patch_module_resolver",
         "--node_options=--max-old-space-size=<SOME_SIZE>",
     ],
 )

packages/rollup/test/integration/golden.amd.js.map.sha256_

@@ -1 +1 @@
-c924104f49b195bc75b35b75e0b2ef151c06d920a7fbef8788dff3af1ee673df
+a3f8b014324476ec5fa98c03bce988bfecac12f63fbc7d985b9a4b15d9da9fbd

packages/rollup/test/integration/golden.cjs.js.map.sha256_

@@ -1 +1 @@
-86daf398a91b909de1ba663bad9aeff0373fa7e859c304308d6e98d341c7bbc1
+4b2ff88ab16971f40b088e95aaa39e94d2e2b81daca6ec6f1bf3012143ba56b6

packages/rollup/test/integration/golden.esm.js.map.sha256_

@@ -1 +1 @@
-9fe0f43821c5735b20edad3f95d2e2c26d9d495ad94831cbb1cf7a73642cde8c
+91ac056e05a90641cfa1c98150da7a14b4808054bec52b07eedc4446a29ba405