refactor: remove install_source_map_support from nodejs_binary since …

…it is vendored in BREAKING CHANGE: `install_source_map_support` attribute removed from `nodejs_binary`. `source-map-support` is vendored in at `/third_party/github.com/source-map-support` so it can always be installed.
alexeagle · Jun 6, 2020 · 72f19e7 · 72f19e7
1 parent 88740e7
commit 72f19e7
Show file tree

Hide file tree

Showing 16 changed files with 10 additions and 43 deletions.
diff --git a/examples/angular/tools/angular_prerender.bzl b/examples/angular/tools/angular_prerender.bzl
@@ -45,7 +45,6 @@ def ng_prerender(name, index, prerender_roots = [], **kwargs):
             "@npm//domino",
             "@npm//reflect-metadata",
         ],
-        install_source_map_support = False,
         entry_point = "//src:prerender.ts",
     )
 

diff --git a/internal/bazel_integration_test/BUILD.bazel b/internal/bazel_integration_test/BUILD.bazel
@@ -22,8 +22,6 @@ nodejs_binary(
     configuration_env_vars = ["BAZEL_INTEGRATION_TEST_DEBUG"],
     data = ["@npm//tmp"],
     entry_point = ":test_runner.js",
-    # reduce memory usage by disabling source_map_support
-    install_source_map_support = False,
     templated_args = ["--node_options=--max-old-space-size=1024"],
 )
 

diff --git a/internal/node/node.bzl b/internal/node/node.bzl
@@ -97,7 +97,6 @@ def _write_require_patch_script(ctx, node_modules_root):
         substitutions = {
             "TEMPLATED_bin_dir": ctx.bin_dir.path,
             "TEMPLATED_gen_dir": ctx.genfiles_dir.path,
-            "TEMPLATED_install_source_map_support": str(ctx.attr.install_source_map_support).lower(),
             "TEMPLATED_module_roots": "\n  " + ",\n  ".join(module_mappings),
             "TEMPLATED_node_modules_root": node_modules_root,
             "TEMPLATED_target": str(ctx.label),
@@ -411,12 +410,6 @@ nodejs_binary(
         mandatory = True,
         allow_single_file = True,
     ),
-    "install_source_map_support": attr.bool(
-        doc = """Install the source-map-support package.
-        Enable this to get stack traces that point to original sources, e.g. if the program was written
-        in TypeScript.""",
-        default = True,
-    ),
     "node_modules": attr.label(
         doc = """The npm packages which should be available to `require()` during
         execution.

diff --git a/internal/node/require_patch.js b/internal/node/require_patch.js
@@ -52,7 +52,6 @@ const USER_WORKSPACE_NAME = 'TEMPLATED_user_workspace_name';
 const NODE_MODULES_ROOT = 'TEMPLATED_node_modules_root';
 const BIN_DIR = 'TEMPLATED_bin_dir';
 const GEN_DIR = 'TEMPLATED_gen_dir';
-const INSTALL_SOURCE_MAP_SUPPORT = TEMPLATED_install_source_map_support;
 const TARGET = 'TEMPLATED_target';
 
 log_verbose(`patching require for ${TARGET}
@@ -61,7 +60,6 @@ log_verbose(`patching require for ${TARGET}
   TARGET: ${TARGET}
   BIN_DIR: ${BIN_DIR}
   GEN_DIR: ${GEN_DIR}
-  INSTALL_SOURCE_MAP_SUPPORT: ${INSTALL_SOURCE_MAP_SUPPORT}
   MODULE_ROOTS: ${JSON.stringify(MODULE_ROOTS, undefined, 2)}
   NODE_MODULES_ROOT: ${NODE_MODULES_ROOT}
   USER_WORKSPACE_NAME: ${USER_WORKSPACE_NAME}
@@ -490,14 +488,11 @@ module.constructor._resolveFilename =
 
 // Before loading anything that might print a stack, install the
 // source-map-support.
-if (INSTALL_SOURCE_MAP_SUPPORT) {
-  try {
-    const sourcemap_support_package = path.resolve(
-        process.cwd(), '../build_bazel_rules_nodejs/third_party/github.com/source-map-support');
-    require(sourcemap_support_package).install();
-  } catch (_) {
-    log_verbose(`WARNING: source-map-support module not installed.
-      Stack traces from languages like TypeScript will point to generated .js files.
-      Set install_source_map_support = False in ${TARGET} to turn off this warning.`);
-  }
+try {
+  const sourcemap_support_package = path.resolve(
+      process.cwd(), '../build_bazel_rules_nodejs/third_party/github.com/source-map-support');
+  require(sourcemap_support_package).install();
+} catch (_) {
+  log_verbose(`WARNING: source-map-support module not installed.
+    Stack traces from languages like TypeScript will point to generated .js files.`);
 }
diff --git a/internal/npm_install/BUILD.bazel b/internal/npm_install/BUILD.bazel
@@ -5,7 +5,9 @@ load("@build_bazel_rules_nodejs//:index.bzl", "nodejs_binary")
 # avoid leaking a ts dependency
 load("//packages/typescript:checked_in_ts_project.bzl", "checked_in_ts_project")
 
-# using checked in ts library like the linker
+# Using checked in ts library like the linker
+# To update index.js run:
+#   bazel run //internal/npm_install:compile_generate_build_file_check_compiled.accept
 checked_in_ts_project(
     name = "compile_generate_build_file",
     src = "generate_build_file.ts",
@@ -54,6 +56,5 @@ nodejs_binary(
         "//third_party/npm/node_modules/named-amd",
     ],
     entry_point = ":browserify-wrapped.js",
-    install_source_map_support = False,
     visibility = ["//visibility:public"],
 )
diff --git a/internal/npm_install/generate_build_file.ts b/internal/npm_install/generate_build_file.ts
@@ -1071,7 +1071,6 @@ export function printPackageBin(pkg: Dep) {
 nodejs_binary(
     name = "${name}",
     entry_point = "//:node_modules/${pkg._dir}/${path}",
-    install_source_map_support = False,
     data = [${data.map(p => `"${p}"`).join(', ')}],
     templated_args = ["--nobazel_patch_module_resolver"],${additionalAttributes(pkg, name)}
 )
@@ -1107,7 +1106,6 @@ def ${name.replace(/-/g, '_')}(**kwargs):
     else:
         nodejs_binary(
             entry_point = "@${WORKSPACE}//:node_modules/${pkg._dir}/${path}",
-            install_source_map_support = False,
             data = [${data.map(p => `"${p}"`).join(', ')}] + kwargs.pop("data", []),
             templated_args = ["--nobazel_patch_module_resolver"] + kwargs.pop("templated_args", []),${
           additionalAttributes(pkg, name)}
@@ -1118,7 +1116,6 @@ def ${name.replace(/-/g, '_')}(**kwargs):
 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", []),
       templated_args = ["--nobazel_patch_module_resolver"] + kwargs.pop("templated_args", []),${
           additionalAttributes(pkg, name)}

diff --git a/internal/npm_install/index.js b/internal/npm_install/index.js
@@ -656,7 +656,6 @@ function printPackageBin(pkg) {
 nodejs_binary(
     name = "${name}",
     entry_point = "//:node_modules/${pkg._dir}/${path}",
-    install_source_map_support = False,
     data = [${data.map(p => `"${p}"`).join(', ')}],
     templated_args = ["--nobazel_patch_module_resolver"],${additionalAttributes(pkg, name)}
 )
@@ -689,7 +688,6 @@ def ${name.replace(/-/g, '_')}(**kwargs):
     else:
         nodejs_binary(
             entry_point = "@${WORKSPACE}//:node_modules/${pkg._dir}/${path}",
-            install_source_map_support = False,
             data = [${data.map(p => `"${p}"`).join(', ')}] + kwargs.pop("data", []),
             templated_args = ["--nobazel_patch_module_resolver"] + kwargs.pop("templated_args", []),${additionalAttributes(pkg, name)}
             **kwargs
@@ -699,7 +697,6 @@ def ${name.replace(/-/g, '_')}(**kwargs):
 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", []),
       templated_args = ["--nobazel_patch_module_resolver"] + kwargs.pop("templated_args", []),${additionalAttributes(pkg, name)}
       **kwargs

diff --git a/internal/npm_install/test/BUILD.bazel b/internal/npm_install/test/BUILD.bazel
@@ -30,7 +30,6 @@ nodejs_binary(
         "@npm//unidiff",
     ],
     entry_point = ":update_golden.js",
-    install_source_map_support = False,
 )
 
 npm_umd_bundle(

diff --git a/internal/npm_install/test/golden/@gregmagolan/test-a/bin/BUILD.bazel.golden b/internal/npm_install/test/golden/@gregmagolan/test-a/bin/BUILD.bazel.golden
@@ -4,7 +4,6 @@ load("@build_bazel_rules_nodejs//:index.bzl", "nodejs_binary")
 nodejs_binary(
     name = "test",
     entry_point = "//:node_modules/@gregmagolan/test-a/@bin/test.js",
-    install_source_map_support = False,
     data = ["//@gregmagolan/test-a:test-a"],
     templated_args = ["--nobazel_patch_module_resolver"],
 )
diff --git a/internal/npm_install/test/golden/@gregmagolan/test-a/index.bzl.golden b/internal/npm_install/test/golden/@gregmagolan/test-a/index.bzl.golden
@@ -6,15 +6,13 @@ def test(**kwargs):
     else:
         nodejs_binary(
             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

diff --git a/internal/npm_install/test/golden/jasmine/bin/BUILD.bazel.golden b/internal/npm_install/test/golden/jasmine/bin/BUILD.bazel.golden
@@ -4,7 +4,6 @@ load("@build_bazel_rules_nodejs//:index.bzl", "nodejs_binary")
 nodejs_binary(
     name = "jasmine",
     entry_point = "//:node_modules/jasmine/bin/jasmine.js",
-    install_source_map_support = False,
     data = ["//jasmine:jasmine"],
     templated_args = ["--nobazel_patch_module_resolver"],
 )
diff --git a/internal/npm_install/test/golden/jasmine/index.bzl.golden b/internal/npm_install/test/golden/jasmine/index.bzl.golden
@@ -6,15 +6,13 @@ def jasmine(**kwargs):
     else:
         nodejs_binary(
             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

diff --git a/internal/pkg_npm/BUILD.bazel b/internal/pkg_npm/BUILD.bazel
@@ -16,13 +16,11 @@ nodejs_binary(
     name = "packager",
     data = ["//third_party/github.com/gjtorikian/isBinaryFile"],
     entry_point = ":packager.js",
-    install_source_map_support = False,
 )
 
 nodejs_binary(
     name = "npm_script_generator",
     entry_point = ":npm_script_generator.js",
-    install_source_map_support = False,
 )
 
 filegroup(

diff --git a/internal/pkg_web/BUILD.bazel b/internal/pkg_web/BUILD.bazel
@@ -23,7 +23,6 @@ nodejs_binary(
     name = "assembler",
     data = ["assembler.js"],
     entry_point = ":assembler.js",
-    install_source_map_support = False,
     node_modules = ":node_modules_none",
 )
 

diff --git a/packages/karma/BUILD.bazel b/packages/karma/BUILD.bazel
@@ -47,7 +47,6 @@ nodejs_binary(
         "@npm//tmp",
     ],
     entry_point = "@npm//:node_modules/karma/bin/karma",
-    install_source_map_support = False,
 )
 
 bzl_library(

diff --git a/packages/labs/protobufjs/BUILD.bazel b/packages/labs/protobufjs/BUILD.bazel
@@ -44,7 +44,6 @@ nodejs_binary(
         "@build_bazel_rules_typescript_protobufs_compiletime_deps//estraverse",
     ],
     entry_point = "@build_bazel_rules_typescript_protobufs_compiletime_deps//:node_modules/protobufjs/bin/pbjs",
-    install_source_map_support = False,
 )
 
 nodejs_binary(
@@ -67,7 +66,6 @@ nodejs_binary(
         "@build_bazel_rules_typescript_protobufs_compiletime_deps//estraverse",
     ],
     entry_point = "@build_bazel_rules_typescript_protobufs_compiletime_deps//:node_modules/protobufjs/bin/pbts",
-    install_source_map_support = False,
 )
 
 # Runtime libraries needed by the protobufjs library.