From c9b5f36e56ee1413e7b31d88062bd03df6f26052 Mon Sep 17 00:00:00 2001 From: Alex Eagle Date: Mon, 17 Jun 2019 09:54:32 -0700 Subject: [PATCH 1/2] chore: update to bazel 0.27.0 --- package.json | 2 +- yarn.lock | 44 ++++++++++++++++++++++---------------------- 2 files changed, 23 insertions(+), 23 deletions(-) diff --git a/package.json b/package.json index 891afa082e..84de17c60b 100644 --- a/package.json +++ b/package.json @@ -9,7 +9,7 @@ "homepage": "https://github.com/bazelbuild/rules_nodejs", "license": "Apache-2.0", "devDependencies": { - "@bazel/bazel": "0.27.0rc5", + "@bazel/bazel": "0.27.0", "@bazel/buildifier": "^0.25.1", "@bazel/ibazel": "0.10.1", "@commitlint/cli": "^8.0.0", diff --git a/yarn.lock b/yarn.lock index 6077e52cbe..e3f33ec924 100644 --- a/yarn.lock +++ b/yarn.lock @@ -2,29 +2,29 @@ # yarn lockfile v1 -"@bazel/bazel-darwin_x64@0.27.0-rc5": - version "0.27.0-rc5" - resolved "https://registry.yarnpkg.com/@bazel/bazel-darwin_x64/-/bazel-darwin_x64-0.27.0-rc5.tgz#e7420f7fa79ede0155565f1257a84782b120f99f" - integrity sha512-Xw+cEbpU+6sSTMBCXnEQw0oK7UYdOiBtGOZ0mjRr7CehRUcd7Wdr5sCx0+vu1lFSnwgpqM0HEdygw0wzvaM8Qw== - -"@bazel/bazel-linux_x64@0.27.0-rc5": - version "0.27.0-rc5" - resolved "https://registry.yarnpkg.com/@bazel/bazel-linux_x64/-/bazel-linux_x64-0.27.0-rc5.tgz#196d2a640b892780e525d292bd9c5d5389a3d138" - integrity sha512-1MTaWEj5R4t9B8n3BrERWX/jt0hYYJrql+ek/4alVj2g6WAOzKY7zvXc8Odvy+izJbR7LuqEspYD+6vk8YR98w== - -"@bazel/bazel-win32_x64@0.27.0-rc5": - version "0.27.0-rc5" - resolved "https://registry.yarnpkg.com/@bazel/bazel-win32_x64/-/bazel-win32_x64-0.27.0-rc5.tgz#3350840cdee1af2802d8aeb2e714adf1e6e9309b" - integrity sha512-BUe+wBo2WdsvfRkbPEVLAANfAQxnvjiyde9ReM1pbR6Be9mr/qxUXdydkykae9TmDkztrKtUKGD14IMhz5ZCQw== - -"@bazel/bazel@0.27.0rc5": - version "0.27.0-rc5" - resolved "https://registry.yarnpkg.com/@bazel/bazel/-/bazel-0.27.0-rc5.tgz#7cd8d47960df422fb8f3b7cdd7007e7bc1aacd24" - integrity sha512-9czrKaSpTXeeo3vfIwRgv5+3PWAS0JNMjOU09GnSV0HTK/mGuCnYhkR7hdz6HjaNcXDLif3gvJHvS0Cs3905bA== +"@bazel/bazel-darwin_x64@0.27.0": + version "0.27.0" + resolved "https://registry.yarnpkg.com/@bazel/bazel-darwin_x64/-/bazel-darwin_x64-0.27.0.tgz#83a03c92d52ae60e48e86a1ee697e69e12ddbdf6" + integrity sha512-CyavIbRKRa/55aMyfEM9hjbt4TVISfv7HLeymHTGTLWzS8uQokQ8W9tR/pgc7YJn8F0x64dd7nQKxhbYHM1Qfg== + +"@bazel/bazel-linux_x64@0.27.0": + version "0.27.0" + resolved "https://registry.yarnpkg.com/@bazel/bazel-linux_x64/-/bazel-linux_x64-0.27.0.tgz#2358292aa4901f526ba9b90874256f63d6d6608c" + integrity sha512-hO04v7C6M3Jf+qNlLE+IticZZKkkS7Nv6+pXDnENgFWrqLV2H93un6kNQnk83B0hP2cEqRQ8rw5yrIcqXNNuSQ== + +"@bazel/bazel-win32_x64@0.27.0": + version "0.27.0" + resolved "https://registry.yarnpkg.com/@bazel/bazel-win32_x64/-/bazel-win32_x64-0.27.0.tgz#0e5e498de5ccccc9a6cf481cd46904ee938ab6c2" + integrity sha512-7hIGNhdgaxRt9ceSOCs3eSTtCNi4GXz3nu6OjfgSp+QiqLbW/iAVWa8M8vD5aemZ1BSHek4/LISdWUTncLJk+w== + +"@bazel/bazel@0.27.0": + version "0.27.0" + resolved "https://registry.yarnpkg.com/@bazel/bazel/-/bazel-0.27.0.tgz#4e72c8e1cbb4da41022b6de1afe28731f5f58e09" + integrity sha512-yIj64IkesNzjHsoeehQUMBOgrCK/JMSuon5CvBqo6+izPXAmt+OW05uqaLL/FPAYAFQ2mHxsYtDsPxsM8DUbqA== optionalDependencies: - "@bazel/bazel-darwin_x64" "0.27.0-rc5" - "@bazel/bazel-linux_x64" "0.27.0-rc5" - "@bazel/bazel-win32_x64" "0.27.0-rc5" + "@bazel/bazel-darwin_x64" "0.27.0" + "@bazel/bazel-linux_x64" "0.27.0" + "@bazel/bazel-win32_x64" "0.27.0" "@bazel/buildifier-darwin_x64@0.25.1": version "0.25.1" From e57804835c3fff505c61cf03e79c5c14742823b0 Mon Sep 17 00:00:00 2001 From: Alex Eagle Date: Thu, 13 Jun 2019 17:13:20 -0700 Subject: [PATCH 2/2] fix(typescript): fix issue with types[] in non-sandboxed tsc To make worker mode work with TypeScript ambient type discovery, always zero out the compilerOptions#types in the generated tsconfig - users should provide these as deps[] instead. Fixes https://github.com/bazelbuild/rules_typescript/issues/449 --- common.bazelrc | 5 ---- e2e/ts_library/WORKSPACE | 9 +++++++ .../reference_types_directive/BUILD.bazel | 5 +++- .../tsconfig_types.ts | 3 +++ internal/npm_install/test/package/BUILD.bazel | 15 +++++++++++ packages/typescript/WORKSPACE | 2 +- packages/typescript/internal/build_defs.bzl | 27 ++++++++++++++++++- 7 files changed, 58 insertions(+), 8 deletions(-) create mode 100644 internal/npm_install/test/package/BUILD.bazel diff --git a/common.bazelrc b/common.bazelrc index 1ba1ea9748..f561c064ba 100644 --- a/common.bazelrc +++ b/common.bazelrc @@ -15,11 +15,6 @@ build --symlink_prefix=dist/ # build --symlink_prefix=/ # however this makes it harder to find outputs. -# This option is changed to true in Bazel 0.27 but exposes a known worker mode bug: -# https://github.com/bazelbuild/rules_typescript/issues/449 -# While we are fixing that issue, we need to keep this breaking change disabled -build --incompatible_list_based_execution_strategy_selection=false - # Specifies desired output mode for running tests. # Valid values are # 'summary' to output only test status summary diff --git a/e2e/ts_library/WORKSPACE b/e2e/ts_library/WORKSPACE index 503c271f9a..654764897c 100644 --- a/e2e/ts_library/WORKSPACE +++ b/e2e/ts_library/WORKSPACE @@ -26,6 +26,15 @@ load("@build_bazel_rules_nodejs//:defs.bzl", "yarn_install") yarn_install( name = "npm", + manual_build_file_contents = """ +filegroup( + name = "node_modules_filegroup", + srcs = [ + "//node_modules/@types/hammerjs:hammerjs__files", + "//node_modules/@types/jasmine:jasmine__files", + "//node_modules/typescript:typescript__files", + ], +)""", package_json = "//:package.json", yarn_lock = "//:yarn.lock", ) diff --git a/e2e/ts_library/reference_types_directive/BUILD.bazel b/e2e/ts_library/reference_types_directive/BUILD.bazel index bf4ceb5fb1..a984b223fe 100644 --- a/e2e/ts_library/reference_types_directive/BUILD.bazel +++ b/e2e/ts_library/reference_types_directive/BUILD.bazel @@ -7,7 +7,10 @@ ts_library( expected_diagnostics = [ "TS2304: Cannot find name 'Hammer'", ], - node_modules = "@npm//:node_modules", + # Use a filegroup to simulate user-managed node modules + # This exercises logic in build_defs.bzl that must preserve + # the types[] in tsconfig.json so that jasmine is found. + node_modules = "@npm//:node_modules_filegroup", tsconfig = ":tsconfig.json", ) diff --git a/e2e/ts_library/reference_types_directive/tsconfig_types.ts b/e2e/ts_library/reference_types_directive/tsconfig_types.ts index 657fce4f9b..da10367e55 100644 --- a/e2e/ts_library/reference_types_directive/tsconfig_types.ts +++ b/e2e/ts_library/reference_types_directive/tsconfig_types.ts @@ -1,3 +1,6 @@ +// Cecking that `describe` is a valid symbol here to ensure +// that @types/jasmine is getting picked up +describe('', () => {}); // Since "hammerjs" is not included in the types=[] array in // tsconfig, this should result in a compile error: TS2304: Cannot find name 'Hammer' console.log(typeof Hammer); diff --git a/internal/npm_install/test/package/BUILD.bazel b/internal/npm_install/test/package/BUILD.bazel new file mode 100644 index 0000000000..92b2d286be --- /dev/null +++ b/internal/npm_install/test/package/BUILD.bazel @@ -0,0 +1,15 @@ +# Generated file from yarn_install/npm_install rule. +# See $(bazel info output_base)/external/build_bazel_rules_nodejs/internal/npm_install/generate_build_file.js + +# All rules in other repositories can use these targets +package(default_visibility = ["//visibility:public"]) + +load("@build_bazel_rules_nodejs//internal/npm_install:node_module_library.bzl", "node_module_library") + +# The node_modules directory in one catch-all node_module_library. +# NB: Using this target may have bad performance implications if +# there are many files in target. +# See https://github.com/bazelbuild/bazel/issues/5153. +node_module_library( + name = "node_modules", +) diff --git a/packages/typescript/WORKSPACE b/packages/typescript/WORKSPACE index 75a999a003..98327f26ed 100644 --- a/packages/typescript/WORKSPACE +++ b/packages/typescript/WORKSPACE @@ -40,7 +40,7 @@ rules_nodejs_dev_dependencies() # With http_archive it only sees releases/download/*.tar.gz urls git_repository( name = "build_bazel_rules_typescript", - commit = "b1fa8ad9d15a4b46da800b33333512e538c69bd4", + commit = "78af714217bd2b844d2f75961263be4a9e39773e", remote = "http://github.com/bazelbuild/rules_typescript.git", ) diff --git a/packages/typescript/internal/build_defs.bzl b/packages/typescript/internal/build_defs.bzl index 632c35ed26..5f748f833b 100644 --- a/packages/typescript/internal/build_defs.bzl +++ b/packages/typescript/internal/build_defs.bzl @@ -23,6 +23,7 @@ load("@build_bazel_rules_typescript//internal:common/tsconfig.bzl", "create_tsco load("//internal:ts_config.bzl", "TsConfigInfo") _DEFAULT_COMPILER = "@npm//@bazel/typescript/bin:tsc_wrapped" +_DEFAULT_NODE_MODULES = Label("@npm//typescript:typescript__typings") def _trim_package_node_modules(package_name): # trim a package name down to its path prior to a node_modules @@ -35,6 +36,17 @@ def _trim_package_node_modules(package_name): segments += [n] return "/".join(segments) +# Detect use of bazel-managed node modules. +# Return True if the node_modules is the default or a course-grained deps +# `node_modules = "@npm//:node_modules"` +# Return False if the user used a hand-rolled filegroup +# `node_modules = "//:my_node_modules"` +# Note: this works for ts_library since we have a default for node_modules +# but wouldn't work for other rules like nodejs_binary +def _uses_bazel_managed_node_modules(ctx): + # If the user put a filegroup as the node_modules it will have no provider + return NodeModuleSources in ctx.attr.node_modules + # This function is similar but slightly different than _compute_node_modules_root # in /internal/node/node.bzl. TODO(gregmagolan): consolidate these functions def _compute_node_modules_root(ctx): @@ -206,6 +218,19 @@ def tsc_wrapped_tsconfig( # Since g3 isn't ready to do this yet config["compilerOptions"]["target"] = "es2015" + # It's fine for users to have types[] in their tsconfig.json to help the editor + # know which of the node_modules/@types/* entries to include in the program. + # But we don't want TypeScript to do any automatic resolution under tsc_wrapped + # because when not run in a sandbox, it will scan the @types directory and find + # entries that aren't in the action inputs. + # See https://github.com/bazelbuild/rules_typescript/issues/449 + # This setting isn't shared with g3 because there is no node_modules directory there. + # NB: Under user-managed dependencies we don't have a Provider in the node_modules + # so our mechanism to collect typings and put them into the program doesn't work. + # In that case we leave the types[] alone. + if _uses_bazel_managed_node_modules(ctx): + config["compilerOptions"]["types"] = [] + # If the user gives a tsconfig attribute, the generated file should extend # from the user's tsconfig. # See https://github.com/Microsoft/TypeScript/issues/9876 @@ -349,7 +374,7 @@ ts_library = rule( yarn_lock = "//:yarn.lock", ) """, - default = Label("@npm//typescript:typescript__typings"), + default = _DEFAULT_NODE_MODULES, ), "supports_workers": attr.bool( doc = """Intended for internal use only.