Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(create): add workers flags #865

Merged
merged 2 commits into from
Jun 17, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 0 additions & 5 deletions common.bazelrc
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
9 changes: 9 additions & 0 deletions e2e/ts_library/WORKSPACE
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add a comment here that this is to exclude hammerjs because the test asserts that it's missing?

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",
)
Expand Down
5 changes: 4 additions & 1 deletion e2e/ts_library/reference_types_directive/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this makes me think it's a breaking change. I hoped we could keep the old code building?

Copy link
Collaborator

@gregmagolan gregmagolan Jun 17, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because @npm//:node_modules is now a node_modules_library and no longer a filegroup and the rules_typescript change makes ts_library now all of its .d.ts files and stamp out the types in tsconfig this ts_library now successfully compiles. This test was checking for the missing dep "TS2304: Cannot find name 'Hammer'" so for the test to work as designed the node_modules must be a plain filegroup.

tsconfig = ":tsconfig.json",
)

Expand Down
3 changes: 3 additions & 0 deletions e2e/ts_library/reference_types_directive/tsconfig_types.ts
Original file line number Diff line number Diff line change
@@ -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);
15 changes: 15 additions & 0 deletions internal/npm_install/test/package/BUILD.bazel
Original file line number Diff line number Diff line change
@@ -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",
)
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
2 changes: 1 addition & 1 deletion packages/typescript/WORKSPACE
Original file line number Diff line number Diff line change
Expand Up @@ -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",
)

Expand Down
27 changes: 26 additions & 1 deletion packages/typescript/internal/build_defs.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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):
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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.
Expand Down
44 changes: 22 additions & 22 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -2,29 +2,29 @@
# yarn lockfile v1


"@bazel/[email protected]-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/[email protected]-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/[email protected]-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/[email protected].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/[email protected]":
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/[email protected]":
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/[email protected]":
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/[email protected].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/[email protected]":
version "0.25.1"
Expand Down