From 72f19e7600ff8d984b8586419d9d26e159ab661a Mon Sep 17 00:00:00 2001 From: Greg Magolan Date: Sat, 11 Jan 2020 22:36:35 -0800 Subject: [PATCH] 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. --- examples/angular/tools/angular_prerender.bzl | 1 - internal/bazel_integration_test/BUILD.bazel | 2 -- internal/node/node.bzl | 7 ------- internal/node/require_patch.js | 19 +++++++------------ internal/npm_install/BUILD.bazel | 5 +++-- internal/npm_install/generate_build_file.ts | 3 --- internal/npm_install/index.js | 3 --- internal/npm_install/test/BUILD.bazel | 1 - .../test-a/bin/BUILD.bazel.golden | 1 - .../@gregmagolan/test-a/index.bzl.golden | 2 -- .../golden/jasmine/bin/BUILD.bazel.golden | 1 - .../test/golden/jasmine/index.bzl.golden | 2 -- internal/pkg_npm/BUILD.bazel | 2 -- internal/pkg_web/BUILD.bazel | 1 - packages/karma/BUILD.bazel | 1 - packages/labs/protobufjs/BUILD.bazel | 2 -- 16 files changed, 10 insertions(+), 43 deletions(-) diff --git a/examples/angular/tools/angular_prerender.bzl b/examples/angular/tools/angular_prerender.bzl index 71d0790d52..d7d6b5cc60 100644 --- 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 index 91d9672378..bbc085ec1d 100644 --- 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 index e83de69d97..fa568f591a 100644 --- 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 index e0b251d28c..2bcb7ab57e 100644 --- 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 index 287d475427..75e04f01b5 100644 --- 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 index 758b56da07..8daf1270ed 100644 --- 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 index 54810b34b8..08a3d148c9 100644 --- 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 index 352fbeab6c..0c02469053 100644 --- 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 index c49118f926..92d22a1dbf 100644 --- 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 index 8e3a864ddc..bd6420ee64 100644 --- 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,7 +6,6 @@ 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 @@ -14,7 +13,6 @@ def test(**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 index 93c29fa488..b1b65eb5de 100644 --- 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 index d71afcd81f..95c02cce05 100644 --- a/internal/npm_install/test/golden/jasmine/index.bzl.golden +++ b/internal/npm_install/test/golden/jasmine/index.bzl.golden @@ -6,7 +6,6 @@ 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 @@ -14,7 +13,6 @@ def jasmine(**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 index 98207815fa..ea0addbeb3 100644 --- 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 index bd9da9edb7..6c13513f09 100644 --- 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 index 18edf14f42..33de0e67fa 100644 --- 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 index e91eefd3c1..7912fa49c4 100644 --- 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.