refactor: default to nobazel_patch_module_resolver

BUILD.bazel

@@ -79,6 +79,7 @@ pkg_npm(
     srcs = glob(["*.bzl"]) + [
         "BUILD.bazel",
         "LICENSE",
+        "rules_sass.pr126.patch",
     ],
     substitutions = COMMON_REPLACEMENTS,
     deps = [

e2e/node_loader_preserve_symlinks/BUILD.bazel

@@ -9,4 +9,6 @@ nodejs_test(
     ],
     entry_point = ":node_loader_test.spec.js",
     node_modules = "@npm//:node_modules",
+    # legacy "node_modules" attribute means linker didn't see deps
+    templated_args = ["--bazel_patch_module_resolver"],
 )

e2e/packages/BUILD.bazel

@@ -40,6 +40,8 @@ nodejs_test(
     node_modules = "@e2e_packages_npm_install_duplicate_for_determinism_testing//:node_modules",
     # TODO(gregmagolan): fix this test on windows
     tags = ["fix-windows"],
+    # TODO: use runfiles
+    templated_args = ["--bazel_patch_module_resolver"],
 )
 
 nodejs_test(
@@ -52,4 +54,6 @@ nodejs_test(
     node_modules = "@e2e_packages_yarn_install_duplicate_for_determinism_testing//:node_modules",
     # TODO(gregmagolan): fix this test on windows
     tags = ["fix-windows"],
+    # TODO: use runfiles
+    templated_args = ["--bazel_patch_module_resolver"],
 )

e2e/typescript/BUILD.bazel

@@ -51,6 +51,10 @@ jasmine_node_test(
         # Verify that worker_protocol.proto can be referenced as a target in the generated npm bazel workspace
         "@npm//@bazel/typescript/third_party/github.com/bazelbuild/bazel/src/main/protobuf:worker_protocol.proto",
     ],
+    templated_args = [
+        # ts_library produces named AMD output with repo name in the require statement
+        "--bazel_patch_module_resolver",
+    ],
     deps = [
         ":test_lib",
     ],

e2e/webapp/test.js

@@ -1,5 +1,6 @@
 const fs = require('fs');
-const content = fs.readFileSync(require.resolve('e2e_webapp/out.min/app.js'), 'utf-8');
+const runfiles = require(process.env['BAZEL_NODE_RUNFILES_HELPER']);
+const content = fs.readFileSync(runfiles.resolve('e2e_webapp/out.min/app.js'), 'utf-8');
 if (content.indexOf('import("./strings') < 0) {
   console.error(content);
   process.exitCode = 1;

examples/angular/WORKSPACE

@@ -36,6 +36,8 @@ http_archive(
 # Fetch sass rules for compiling sass files
 http_archive(
     name = "io_bazel_rules_sass",
+    patch_args = ["-p1"],
+    patches = ["@build_bazel_rules_nodejs//:rules_sass.pr126.patch"],
     sha256 = "c78be58f5e0a29a04686b628cf54faaee0094322ae0ac99da5a8a8afca59a647",
     strip_prefix = "rules_sass-1.25.0",
     urls = [

examples/angular/tools/BUILD.bazel

@@ -11,5 +11,7 @@ nodejs_binary(
         "@npm//@bazel/typescript",
     ],
     entry_point = "@npm//:node_modules/@bazel/typescript/internal/tsc_wrapped/tsc_wrapped.js",
+    # TODO: turn on --worker_sandboxing and remove this flag to see failure to load the plugin
+    templated_args = ["--bazel_patch_module_resolver"],
     visibility = ["//:__subpackages__"],
 )

examples/angular_view_engine/WORKSPACE

@@ -23,6 +23,8 @@ http_archive(
 # Fetch sass rules for compiling sass files
 http_archive(
     name = "io_bazel_rules_sass",
+    patch_args = ["-p1"],
+    patches = ["@build_bazel_rules_nodejs//:rules_sass.pr126.patch"],
     sha256 = "c78be58f5e0a29a04686b628cf54faaee0094322ae0ac99da5a8a8afca59a647",
     strip_prefix = "rules_sass-1.25.0",
     urls = [

examples/kotlin/BUILD.bazel

@@ -74,5 +74,9 @@ jasmine_node_test(
         ":bundle",
         "@npm//domino",
     ],
-    templated_args = ["--node_options=--experimental-modules"],
+    templated_args = [
+        # TODO: don't rely on patching require()
+        "--bazel_patch_module_resolver",
+        "--node_options=--experimental-modules",
+    ],
 )

examples/user_managed_deps/BUILD.bazel

@@ -43,6 +43,8 @@ jasmine_node_test(
     srcs = glob(["*.spec.js"]),
     node_modules = "//:node_modules",
     tags = ["no-local-jasmine-deps"],
+    # user-managed deps don't expose the provider the linker needs to make require work
+    templated_args = ["--bazel_patch_module_resolver"],
     deps = [
         ":decrement",
         ":program",

internal/bazel_integration_test/test_runner.js

@@ -23,7 +23,7 @@ const spawnSync = require('child_process').spawnSync;
 const fs = require('fs');
 const path = require('path');
 const tmp = require('tmp');
-
+const runfiles = require(process.env['BAZEL_NODE_RUNFILES_HELPER']);
 const DEBUG = !!process.env['BAZEL_INTEGRATION_TEST_DEBUG'];
 const VERBOSE_LOGS = !!process.env['VERBOSE_LOGS'];
 
@@ -189,7 +189,7 @@ if (bazelrcImportsKeys.length && isFile(bazelrcFile)) {
   let bazelrcContents = fs.readFileSync(bazelrcFile, {encoding: 'utf-8'});
   for (const importKey of bazelrcImportsKeys) {
     const importContents =
-        fs.readFileSync(require.resolve(config.bazelrcImports[importKey]), {encoding: 'utf-8'});
+        fs.readFileSync(runfiles.resolve(config.bazelrcImports[importKey]), {encoding: 'utf-8'});
     bazelrcContents = bazelrcContents.replace(importKey, importContents);
   }
   fs.writeFileSync(bazelrcFile, bazelrcContents);
@@ -212,7 +212,7 @@ if (config.bazelrcAppend) {
   let workspaceContents = fs.readFileSync(workspaceFile, {encoding: 'utf-8'});
   // replace repositories
   for (const repositoryKey of Object.keys(config.repositories)) {
-    const archiveFile = require.resolve(config.repositories[repositoryKey]).replace(/\\/g, '/');
+    const archiveFile = runfiles.resolve(config.repositories[repositoryKey]).replace(/\\/g, '/');
     const regex =
         new RegExp(`(local_repository|http_archive|git_repository)\\(\\s*name\\s*\\=\\s*"${
             repositoryKey}"[^)]+`);
@@ -281,7 +281,7 @@ if (isFile(packageJsonFile)) {
 
 const isWindows = process.platform === 'win32';
 const bazelBinary =
-    require.resolve(`${config.bazelBinaryWorkspace}/bazel${isWindows ? '.exe' : ''}`);
+    runfiles.resolve(`${config.bazelBinaryWorkspace}/bazel${isWindows ? '.exe' : ''}`);
 
 if (DEBUG) {
   log(`

internal/check_bazel_version.js

@@ -16,15 +16,16 @@
  */
 'use strict';
 
+const runfiles = require(process.env['BAZEL_NODE_RUNFILES_HELPER']);
 const fs = require('fs');
 const args = process.argv.slice(2);
 
 const BAZEL_VERSION = args[0];
 
 const version =
-    fs.readFileSync(require.resolve('build_bazel_rules_nodejs/.bazelversion'), 'utf-8').trim();
+    fs.readFileSync(runfiles.resolve('build_bazel_rules_nodejs/.bazelversion'), 'utf-8').trim();
 const bazelci_version =
-    fs.readFileSync(require.resolve('build_bazel_rules_nodejs/.bazelci/presubmit.yml'), 'utf-8')
+    fs.readFileSync(runfiles.resolve('build_bazel_rules_nodejs/.bazelci/presubmit.yml'), 'utf-8')
         .split('\n')
         .find(v => v.startsWith('bazel:'));
 

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,15 +192,16 @@ 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 ;;
     # Disable the linker pre-process (undocumented and unused; only here as an escape value)
-    --nobazel_run_linker) RUN_LINKER=false ;;
+    # It also enables the --bazel_patch_module_resolver flag, as either the linker or require() patch
+    # is needed for resolving third-party node modules.
+    --nobazel_run_linker) RUN_LINKER=false PATCH_REQUIRE=true ;;
     # Let users pass through arguments to node itself
     --node_options=*) USER_NODE_OPTIONS+=( "${ARG#--node_options=}" ) ;;
     # Remaining argv is collected to pass to the program

internal/node/test/BUILD.bazel

@@ -16,12 +16,13 @@ nodejs_binary(
 )
 
 # You can have a nodejs_binary with a node_modules attribute
-# and no fine grained deps
+# and no fine grained deps, but it requires patching the module resolver
 nodejs_binary(
     name = "has_deps_legacy",
     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,8 @@ nodejs_test(
         "@npm//node_resolve_nested_main",
     ],
     entry_point = ":module_resolution.spec.js",
+    # this is a test for the patched module resolver
+    templated_args = ["--bazel_patch_module_resolver"],
 )
 
 nodejs_test(
@@ -218,6 +221,8 @@ nodejs_test(
         "@npm//node_resolve_nested_main",
     ],
     entry_point = ":module_resolution_built.spec.js",
+    # TODO: passes locally without this flag but fails on CircleCI
+    templated_args = ["--bazel_patch_module_resolver"],
 )
 
 nodejs_test(
@@ -227,6 +232,8 @@ nodejs_test(
         ":genfile-runfile",
     ],
     entry_point = ":data_resolution_built.spec.js",
+    # TODO: fails on Windows without this flag
+    templated_args = ["--bazel_patch_module_resolver"],
 )
 
 npm_package_bin(

internal/node/test/binary_as_data/BUILD.bazel

@@ -9,4 +9,6 @@ nodejs_test(
     name = "main_bin_data_test",
     data = [":main_bin"],
     entry_point = "test.js",
+    # TODO: fails on Windows without this flag
+    templated_args = ["--bazel_patch_module_resolver"],
 )

internal/npm_install/BUILD.bazel

@@ -56,5 +56,7 @@ nodejs_binary(
         "//third_party/npm/node_modules/named-amd",
     ],
     entry_point = ":browserify-wrapped.js",
+    # TODO: figure out why browserify isn't resolved properly
+    templated_args = ["--bazel_patch_module_resolver"],
     visibility = ["//visibility:public"],
 )

internal/npm_install/test/BUILD.bazel

@@ -18,6 +18,9 @@ jasmine_node_test(
         "@fine_grained_goldens//:golden_files",
         "@npm//unidiff",
     ],
+    # Depends on having the .js file in source tree but resolve relative paths
+    # to .js files in the output tree
+    templated_args = ["--bazel_patch_module_resolver"],
 )
 
 nodejs_binary(
@@ -103,6 +106,8 @@ sh_test(
     ],
     node_modules = "@fine_grained_deps_%s//:node_modules" % pkgmgr,
     tags = ["no-local-jasmine-deps"],
+    # TODO: get this test running with just linker: failing under --config=no-runfiles
+    templated_args = ["--bazel_patch_module_resolver"],
     deps = [
         "@fine_grained_deps_%s//jasmine" % pkgmgr,
         "@fine_grained_deps_%s//jasmine-core" % pkgmgr,
@@ -122,6 +127,8 @@ sh_test(
         "fine.spec.js",
     ],
     tags = ["no-local-jasmine-deps"],
+    # TODO: get this test running with just linker: failing under --config=no-runfiles
+    templated_args = ["--bazel_patch_module_resolver"],
     deps = [
         "@fine_grained_deps_%s//jasmine" % pkgmgr,
         "@fine_grained_deps_%s//jasmine-core" % pkgmgr,

internal/npm_install/test/browserify.spec.js

@@ -1,7 +1,9 @@
 const fs = require('fs');
 const path = require('path');
-const mainFile = 'build_bazel_rules_nodejs/third_party/npm/node_modules/browserify/index.js';
-const directory = 'build_bazel_rules_nodejs/internal/npm_install/test';
+const runfiles = require(process.env['BAZEL_NODE_RUNFILES_HELPER']);
+const mainFile =
+    runfiles.resolve('build_bazel_rules_nodejs/third_party/npm/node_modules/browserify/index.js');
+const directory = runfiles.resolve('build_bazel_rules_nodejs/internal/npm_install/test');
 
 describe('our bundled, vendored browserify binary', () => {
   it('should preserve licenses', () => {

internal/npm_install/test/check.js

@@ -1,11 +1,12 @@
 const fs = require('fs');
 const path = require('path');
-const unidiff = require('unidiff')
+const unidiff = require('unidiff');
+const runfiles = require(process.env['BAZEL_NODE_RUNFILES_HELPER']);
 
 function check(file, updateGolden = false) {
   // Strip comments from generated file for comparison to golden
   // to make comparison less brittle
-  const actual = require.resolve(path.posix.join('fine_grained_goldens', file));
+  const actual = runfiles.resolve(path.posix.join('fine_grained_goldens', file));
   const actualContents =
       fs.readFileSync(actual, {encoding: 'utf-8'})
           .replace(/\r\n/g, '\n')
@@ -18,7 +19,7 @@ function check(file, updateGolden = false) {
           .replace(/[\n]+/g, '\n');
 
   // Load the golden file for comparison
-  const golden = require.resolve('./golden/' + file + '.golden');
+  const golden = runfiles.resolvePackageRelative('./golden/' + file + '.golden');
 
   if (updateGolden) {
     // Write to golden file

internal/pkg_npm/BUILD.bazel

@@ -16,6 +16,8 @@ nodejs_binary(
     name = "packager",
     data = ["//third_party/github.com/gjtorikian/isBinaryFile"],
     entry_point = ":packager.js",
+    # TODO: figure out why isbinaryfile is not linked in a way this can resolve
+    templated_args = ["--bazel_patch_module_resolver"],
 )
 
 nodejs_binary(

internal/pkg_web/BUILD.bazel

@@ -27,12 +27,16 @@ nodejs_binary(
     ],
     entry_point = ":assembler.js",
     node_modules = ":node_modules_none",
+    # TODO: figure out why isbinaryfile isn't resolved properly
+    templated_args = ["--bazel_patch_module_resolver"],
 )
 
 # BEGIN-INTERNAL
 jasmine_node_test(
     name = "assembler_test",
     srcs = ["assembler_spec.js"],
+    # TODO: figure out why isbinaryfile isn't resolved properly
+    templated_args = ["--bazel_patch_module_resolver"],
     deps = [
         "assembler.js",
         "//third_party/github.com/gjtorikian/isBinaryFile",

internal/pkg_web/test2/spec.js

@@ -1,14 +1,13 @@
 const fs = require('fs');
 const path = require('path');
+const runfiles = require(process.env['BAZEL_NODE_RUNFILES_HELPER']);
 
-process.chdir(path.join(process.env['TEST_SRCDIR'], 'build_bazel_rules_nodejs'));
-console.error(fs.readdirSync('.'));
 describe('pkg_web paths', () => {
   it('should match the golden file', () => {
     const output = 'build_bazel_rules_nodejs/internal/pkg_web/test2/pkg/index.html';
     const golden = 'build_bazel_rules_nodejs/internal/pkg_web/test2/index_golden.html_';
-    const actual = fs.readFileSync(require.resolve(output), {encoding: 'utf-8'});
-    const expected = fs.readFileSync(require.resolve(golden), {encoding: 'utf-8'});
+    const actual = fs.readFileSync(runfiles.resolve(output), {encoding: 'utf-8'});
+    const expected = fs.readFileSync(runfiles.resolve(golden), {encoding: 'utf-8'});
     // make the input hermetic by replacing the cache-buster timestamp
     expect(actual.replace(/\?v=\d+/g, '?v=123').trim()).toBe(expected.trim());
   });

package.bzl

@@ -31,11 +31,13 @@ def rules_nodejs_dev_dependencies():
     _maybe(
         http_archive,
         name = "io_bazel_rules_sass",
-        sha256 = "c78be58f5e0a29a04686b628cf54faaee0094322ae0ac99da5a8a8afca59a647",
-        strip_prefix = "rules_sass-1.25.0",
+        patch_args = ["-p1"],
+        patches = ["//:rules_sass.pr126.patch"],
+        sha256 = "cf28ff1bcfafb3c97f138bbc8ca9fe386e968ed3faaa9f8e6214abb5e88a2ecd",
+        strip_prefix = "rules_sass-1.29.0",
         urls = [
-            "https://github.com/bazelbuild/rules_sass/archive/1.25.0.zip",
-            "https://mirror.bazel.build/github.com/bazelbuild/rules_sass/archive/1.25.0.zip",
+            "https://github.com/bazelbuild/rules_sass/archive/1.29.0.zip",
+            "https://mirror.bazel.build/github.com/bazelbuild/rules_sass/archive/1.29.0.zip",
         ],
     )
 

packages/concatjs/web_test/test/karma_typescript/user_files.spec.js

@@ -1,11 +1,12 @@
 const fs = require('fs');
+const runfiles = require(process.env['BAZEL_NODE_RUNFILES_HELPER']);
 
 describe('karma_web_test_suite', () => {
   let config;
 
   beforeAll(() => {
     config = fs.readFileSync(
-        require.resolve(
+        runfiles.resolve(
             'build_bazel_rules_nodejs/packages/concatjs/web_test/test/karma_typescript/testing_wrapped_test.conf.js'),
         'utf-8');
   });

packages/create/test.js

@@ -2,7 +2,8 @@
  * Simple node program to test that workspace creation works.
  * We don't use a test framework here since dependencies are awkward.
  */
-const pkg = 'build_bazel_rules_nodejs/packages/create/npm_package';
+const runfiles = require(process.env['BAZEL_NODE_RUNFILES_HELPER']);
+const pkg = runfiles.resolve('build_bazel_rules_nodejs/packages/create/npm_package');
 const fs = require('fs');
 const {main} = require(pkg);