From 88d6ae3c77091067f89d1c1a6e8393ae519b0f31 Mon Sep 17 00:00:00 2001 From: Daniel Muller Date: Mon, 24 Aug 2020 10:24:21 -0600 Subject: [PATCH 1/6] feat(typescript): worker mode for ts_project --- examples/react_webpack/BUILD.bazel | 2 + examples/react_webpack/tsconfig.json | 12 +++- internal/node/node.bzl | 12 +++- packages/typescript/BUILD.bazel | 1 + packages/typescript/internal/BUILD.bazel | 1 + packages/typescript/internal/ts_project.bzl | 66 ++++++++++++++++++- .../typescript/internal/worker/BUILD.bazel | 55 ++++++++++++++++ .../internal/worker/worker_adapter.js | 52 +++++++++++++++ packages/typescript/replacements.bzl | 3 +- .../test/ts_project/worker/BUILD.bazel | 11 ++++ .../typescript/test/ts_project/worker/big.ts | 3 + 11 files changed, 211 insertions(+), 7 deletions(-) create mode 100644 packages/typescript/internal/worker/BUILD.bazel create mode 100644 packages/typescript/internal/worker/worker_adapter.js create mode 100644 packages/typescript/test/ts_project/worker/BUILD.bazel create mode 100644 packages/typescript/test/ts_project/worker/big.ts diff --git a/examples/react_webpack/BUILD.bazel b/examples/react_webpack/BUILD.bazel index b65b59ceda..e23aca00c2 100644 --- a/examples/react_webpack/BUILD.bazel +++ b/examples/react_webpack/BUILD.bazel @@ -14,6 +14,8 @@ sass( ) ts_project( + # Start a tsc daemon to watch for changes to make recompiles faster. + supports_workers = True, deps = [ "@npm//@types", "@npm//csstype", diff --git a/examples/react_webpack/tsconfig.json b/examples/react_webpack/tsconfig.json index 0ece51d45f..90da6e4b0b 100644 --- a/examples/react_webpack/tsconfig.json +++ b/examples/react_webpack/tsconfig.json @@ -1,6 +1,14 @@ { "compilerOptions": { "jsx": "react", - "lib": ["ES2015", "DOM"] - } + "lib": [ + "ES2015", + "DOM" + ] + }, + // When using ts_project in worker mode, make sure to whitelist the files that should be part of this particular compilation. In worker mode, the CWD is different than by default so typescript will pickup extraneous files without the whitelist. + "include": [ + "*.tsx", + "*.ts" + ] } \ No newline at end of file diff --git a/internal/node/node.bzl b/internal/node/node.bzl index e4f0e917a2..642f706a47 100644 --- a/internal/node/node.bzl +++ b/internal/node/node.bzl @@ -37,7 +37,7 @@ def _trim_package_node_modules(package_name): for n in package_name.split("/"): if n == "node_modules": break - segments += [n] + segments.append(n) return "/".join(segments) def _compute_node_modules_root(ctx): @@ -250,7 +250,15 @@ fi expanded_args = [expand_location_into_runfiles(ctx, a, ctx.attr.data) for a in expanded_args] # Next expand predefined variables & custom variables - expanded_args = [ctx.expand_make_variables("templated_args", e, {}) for e in expanded_args] + rule_dir = [f for f in [ + ctx.bin_dir.path, + ctx.label.workspace_root, + ctx.label.package, + ] if f] + additional_substitutions = {} + additional_substitutions["@D"] = "/".join([o for o in rule_dir if o]) + additional_substitutions["RULEDIR"] = "/".join([o for o in rule_dir if o]) + expanded_args = [ctx.expand_make_variables("templated_args", e, additional_substitutions) for e in expanded_args] substitutions = { # TODO: Split up results of multifile expansions into separate args and qoute them with diff --git a/packages/typescript/BUILD.bazel b/packages/typescript/BUILD.bazel index cf0467b6b2..83649dbcfe 100644 --- a/packages/typescript/BUILD.bazel +++ b/packages/typescript/BUILD.bazel @@ -100,6 +100,7 @@ pkg_npm( ":npm_version_check", "//packages/typescript/internal:BUILD", "//packages/typescript/internal:ts_project_options_validator.js", + "//packages/typescript/internal/worker", ] + select({ # FIXME: fix stardoc on Windows; //packages/typescript:index.md generation fails with: # ERROR: D:/b/62unjjin/external/npm_bazel_typescript/BUILD.bazel:36:1: Couldn't build file diff --git a/packages/typescript/internal/BUILD.bazel b/packages/typescript/internal/BUILD.bazel index 0a95c526ff..4b08e5f55a 100644 --- a/packages/typescript/internal/BUILD.bazel +++ b/packages/typescript/internal/BUILD.bazel @@ -64,6 +64,7 @@ filegroup( "ts_config.bzl", "ts_project.bzl", "//packages/typescript/internal/devserver:package_contents", + "//packages/typescript/internal/worker:package_contents", ], visibility = ["//packages/typescript:__subpackages__"], ) diff --git a/packages/typescript/internal/ts_project.bzl b/packages/typescript/internal/ts_project.bzl index bc192d43da..56740b3078 100644 --- a/packages/typescript/internal/ts_project.bzl +++ b/packages/typescript/internal/ts_project.bzl @@ -2,6 +2,7 @@ load("@build_bazel_rules_nodejs//:providers.bzl", "DeclarationInfo", "NpmPackageInfo", "declaration_info", "js_module_info", "run_node") load("@build_bazel_rules_nodejs//internal/linker:link_node_modules.bzl", "module_mappings_aspect") +load("@build_bazel_rules_nodejs//internal/node:node.bzl", "nodejs_binary") load(":ts_config.bzl", "TsConfigInfo", "write_tsconfig") _ValidOptionsInfo = provider() @@ -13,6 +14,13 @@ _DEFAULT_TSC = ( "//typescript/bin:tsc" ) +_DEFAULT_TSC_BIN = ( + # BEGIN-INTERNAL + "@npm" + + # END-INTERNAL + "//:node_modules/typescript/bin/tsc" +) + _ATTRS = { "args": attr.string_list(), "declaration_dir": attr.string(), @@ -33,7 +41,14 @@ _ATTRS = { # if you swap out the `compiler` attribute (like with ngtsc) # that compiler might allow more sources than tsc does. "srcs": attr.label_list(allow_files = True, mandatory = True), - "tsc": attr.label(default = Label(_DEFAULT_TSC), executable = True, cfg = "host"), + "supports_workers": attr.bool( + doc = """Experimental! Use only with caution. + +Allows you to enable the Bazel Worker strategy for this project. +This requires that the tsc binary support it.""", + default = False, + ), + "tsc": attr.label(default = Label(_DEFAULT_TSC), executable = True, cfg = "target"), "tsconfig": attr.label(mandatory = True, allow_single_file = [".json"]), } @@ -56,6 +71,16 @@ def _join(*elements): def _ts_project_impl(ctx): arguments = ctx.actions.args() + execution_requirements = {} + progress_prefix = "Compiling TypeScript project" + + if ctx.attr.supports_workers: + # Set to use a multiline param-file for worker mode + arguments.use_param_file("@%s", use_always = True) + arguments.set_param_file_format("multiline") + execution_requirements["supports-workers"] = "1" + execution_requirements["worker-key-mnemonic"] = "TsProjectMnemonic" + progress_prefix = "Compiling TypeScript project (worker mode)" generated_srcs = False for src in ctx.files.srcs: @@ -162,7 +187,9 @@ def _ts_project_impl(ctx): arguments = [arguments], outputs = outputs, executable = "tsc", - progress_message = "Compiling TypeScript project %s [tsc -p %s]" % ( + execution_requirements = execution_requirements, + progress_message = "%s %s [tsc -p %s]" % ( + progress_prefix, ctx.label, ctx.file.tsconfig.short_path, ), @@ -288,6 +315,7 @@ def ts_project_macro( ts_build_info_file = None, tsc = None, validate = True, + supports_workers = False, declaration_dir = None, out_dir = None, root_dir = None, @@ -455,6 +483,11 @@ def ts_project_macro( validate: boolean; whether to check that the tsconfig settings match the attributes. + supports_workers: Experimental! Use only with caution. + + Allows you to enable the Bazel Worker strategy for this project. + This requires that the tsc binary support it. + root_dir: a string specifying a subdirectory under the input package which should be consider the root directory of all the input files. Equivalent to the TypeScript --rootDir option. @@ -559,6 +592,31 @@ def ts_project_macro( ) extra_deps.append("_validate_%s_options" % name) + if supports_workers: + tsc_worker = "%s_worker" % name + nodejs_binary( + name = tsc_worker, + data = [ + Label("//packages/typescript/internal/worker:worker"), + Label(_DEFAULT_TSC_BIN), + tsconfig, + ], + entry_point = Label("//packages/typescript/internal/worker:worker_adapter"), + templated_args = [ + "--nobazel_patch_module_resolver", + "$(execpath {})".format(Label(_DEFAULT_TSC_BIN)), + "--project", + "$(execpath {})".format(tsconfig), + # FIXME: should take out_dir into account + "--outDir", + "$(RULEDIR)", + # FIXME: what about other settings like declaration_dir, root_dir, etc + "--watch", + ], + ) + + tsc = ":" + tsc_worker + typings_out_dir = declaration_dir if declaration_dir else out_dir tsbuildinfo_path = ts_build_info_file if ts_build_info_file else name + ".tsbuildinfo" @@ -583,5 +641,9 @@ def ts_project_macro( buildinfo_out = tsbuildinfo_path if composite or incremental else None, tsc = tsc, link_workspace_root = link_workspace_root, + supports_workers = select({ + "@bazel_tools//src/conditions:host_windows": False, + "//conditions:default": supports_workers, + }), **kwargs ) diff --git a/packages/typescript/internal/worker/BUILD.bazel b/packages/typescript/internal/worker/BUILD.bazel new file mode 100644 index 0000000000..f6593042a2 --- /dev/null +++ b/packages/typescript/internal/worker/BUILD.bazel @@ -0,0 +1,55 @@ +# BEGIN-INTERNAL + +load("//internal/common:copy_to_bin.bzl", "copy_to_bin") +load("//third_party/github.com/bazelbuild/bazel-skylib:rules/copy_file.bzl", "copy_file") + +# Copy the proto file to a matching third_party/... nested directory +# so the runtime require() statements still work +_worker_proto_dir = "third_party/github.com/bazelbuild/bazel/src/main/protobuf" + +genrule( + name = "copy_worker_js", + srcs = ["//packages/worker:npm_package"], + outs = ["worker.js"], + cmd = "cp $(execpath //packages/worker:npm_package)/index.js $@", + visibility = ["//visibility:public"], +) + +copy_file( + name = "copy_worker_proto", + src = "@build_bazel_rules_typescript//%s:worker_protocol.proto" % _worker_proto_dir, + out = "%s/worker_protocol.proto" % _worker_proto_dir, + visibility = ["//visibility:public"], +) + +copy_to_bin( + name = "worker_adapter", + srcs = [ + "worker_adapter.js", + ], + visibility = ["//visibility:public"], +) + +filegroup( + name = "package_contents", + srcs = [ + "BUILD.bazel", + ], + visibility = ["//packages/typescript:__subpackages__"], +) + +# END-INTERNAL + +exports_files([ + "worker_adapter.js", +]) + +filegroup( + name = "worker", + srcs = [ + "third_party/github.com/bazelbuild/bazel/src/main/protobuf/worker_protocol.proto", + "worker.js", + "worker_adapter.js", + ], + visibility = ["//visibility:public"], +) diff --git a/packages/typescript/internal/worker/worker_adapter.js b/packages/typescript/internal/worker/worker_adapter.js new file mode 100644 index 0000000000..aa1e542cc1 --- /dev/null +++ b/packages/typescript/internal/worker/worker_adapter.js @@ -0,0 +1,52 @@ +const child_process = require('child_process'); + +const worker = require('./worker'); + +const workerArg = process.argv.indexOf('--persistent_worker') +if (workerArg > 0) { + process.argv.splice(workerArg, 1) + + if (process.platform !== 'linux' && process.platform !== 'darwin') { + throw new Error('Worker mode is only supported on unix type systems.'); + } + + worker.runWorkerLoop(awaitOneBuild); +} + +const [tscBin, ...tscArgs] = process.argv.slice(2); + +const child = child_process.spawn( + tscBin, + tscArgs, + {stdio: 'pipe'}, +); + +function awaitOneBuild() { + child.kill('SIGCONT') + + let buffer = []; + return new Promise((res) => { + function awaitBuild(s) { + buffer.push(s); + + if (s.includes('Watching for file changes.')) { + child.kill('SIGSTOP') + + const success = s.includes('Found 0 errors.'); + res(success); + + child.stdout.removeListener('data', awaitBuild); + + if (!success) { + console.error( + `\nError output from tsc worker:\n\n ${ + buffer.slice(1).map(s => s.toString()).join('').replace(/\n/g, '\n ')}`, + ) + } + + buffer = []; + } + }; + child.stdout.on('data', awaitBuild); + }); +} \ No newline at end of file diff --git a/packages/typescript/replacements.bzl b/packages/typescript/replacements.bzl index 32d925447a..5665ffe7a7 100644 --- a/packages/typescript/replacements.bzl +++ b/packages/typescript/replacements.bzl @@ -12,7 +12,7 @@ # See the License for the specific language governing permissions and # limitations under the License. -"""Replacements for @npm/typescript package +"""Replacements for @bazel/typescript package """ load("@build_bazel_rules_nodejs//:index.bzl", "COMMON_REPLACEMENTS") @@ -24,6 +24,7 @@ TYPESCRIPT_REPLACEMENTS = dict( # @build_bazel_rules_typescript//:npm_bazel_typescript_package # use this alternate fencing "(#|\\/\\/)\\s+BEGIN-DEV-ONLY[\\w\\W]+?(#|\\/\\/)\\s+END-DEV-ONLY": "", + "//packages/typescript/internal/worker:worker_adapter": "//@bazel/typescript/internal/worker:worker_adapter.js", # This file gets vendored into our repo "@build_bazel_rules_typescript//internal:common": "//@bazel/typescript/internal:common", # Replace the local compiler label with one that comes from npm diff --git a/packages/typescript/test/ts_project/worker/BUILD.bazel b/packages/typescript/test/ts_project/worker/BUILD.bazel new file mode 100644 index 0000000000..1abe48d0de --- /dev/null +++ b/packages/typescript/test/ts_project/worker/BUILD.bazel @@ -0,0 +1,11 @@ +load("//packages/typescript:index.bzl", "ts_project") + +ts_project( + supports_workers = True, + tsconfig = { + "compilerOptions": { + "declaration": True, + "types": [], + }, + }, +) diff --git a/packages/typescript/test/ts_project/worker/big.ts b/packages/typescript/test/ts_project/worker/big.ts new file mode 100644 index 0000000000..124564061e --- /dev/null +++ b/packages/typescript/test/ts_project/worker/big.ts @@ -0,0 +1,3 @@ +// TODO: make it big enough to slow down tsc + +export const a: number = 2; From be2ad208a19001b384c7b73ad738cb88528fc017 Mon Sep 17 00:00:00 2001 From: Alex Eagle Date: Mon, 26 Oct 2020 22:49:52 -0700 Subject: [PATCH 2/6] chore: cleanup and declare protobufjs dependency --- examples/react_webpack/BUILD.bazel | 2 +- examples/react_webpack/tsconfig.json | 5 +++-- internal/node/node.bzl | 16 ++++++++-------- packages/typescript/internal/ts_project.bzl | 7 +++++++ 4 files changed, 19 insertions(+), 11 deletions(-) diff --git a/examples/react_webpack/BUILD.bazel b/examples/react_webpack/BUILD.bazel index e23aca00c2..25fcecc6d7 100644 --- a/examples/react_webpack/BUILD.bazel +++ b/examples/react_webpack/BUILD.bazel @@ -14,7 +14,7 @@ sass( ) ts_project( - # Start a tsc daemon to watch for changes to make recompiles faster. + # Experimental: Start a tsc daemon to watch for changes to make recompiles faster. supports_workers = True, deps = [ "@npm//@types", diff --git a/examples/react_webpack/tsconfig.json b/examples/react_webpack/tsconfig.json index 90da6e4b0b..7ea5cf1299 100644 --- a/examples/react_webpack/tsconfig.json +++ b/examples/react_webpack/tsconfig.json @@ -6,9 +6,10 @@ "DOM" ] }, - // When using ts_project in worker mode, make sure to whitelist the files that should be part of this particular compilation. In worker mode, the CWD is different than by default so typescript will pickup extraneous files without the whitelist. + // When using ts_project in worker mode, we run outside the Bazel sandbox (unless using --worker_sandboxing). + // We list the files that should be part of this particular compilation to avoid TypeScript discovering others. "include": [ "*.tsx", "*.ts" ] -} \ No newline at end of file +} diff --git a/internal/node/node.bzl b/internal/node/node.bzl index 642f706a47..b614f9b27c 100644 --- a/internal/node/node.bzl +++ b/internal/node/node.bzl @@ -150,6 +150,9 @@ def _to_execroot_path(ctx, file): return file.path +def _join(*elements): + return "/".join([f for f in elements if f]) + def _nodejs_binary_impl(ctx): node_modules_manifest = write_node_modules_manifest(ctx, link_workspace_root = ctx.attr.link_workspace_root) node_modules_depsets = [] @@ -250,14 +253,11 @@ fi expanded_args = [expand_location_into_runfiles(ctx, a, ctx.attr.data) for a in expanded_args] # Next expand predefined variables & custom variables - rule_dir = [f for f in [ - ctx.bin_dir.path, - ctx.label.workspace_root, - ctx.label.package, - ] if f] - additional_substitutions = {} - additional_substitutions["@D"] = "/".join([o for o in rule_dir if o]) - additional_substitutions["RULEDIR"] = "/".join([o for o in rule_dir if o]) + rule_dir = _join(ctx.bin_dir.path, ctx.label.workspace_root, ctx.label.package) + additional_substitutions = { + "@D": rule_dir, + "RULEDIR": rule_dir, + } expanded_args = [ctx.expand_make_variables("templated_args", e, additional_substitutions) for e in expanded_args] substitutions = { diff --git a/packages/typescript/internal/ts_project.bzl b/packages/typescript/internal/ts_project.bzl index 56740b3078..89bb3824a7 100644 --- a/packages/typescript/internal/ts_project.bzl +++ b/packages/typescript/internal/ts_project.bzl @@ -594,11 +594,18 @@ def ts_project_macro( if supports_workers: tsc_worker = "%s_worker" % name + protobufjs = ( + # BEGIN-INTERNAL + "@npm" + + # END-INTERNAL + "//protobufjs" + ) nodejs_binary( name = tsc_worker, data = [ Label("//packages/typescript/internal/worker:worker"), Label(_DEFAULT_TSC_BIN), + Label(protobufjs), tsconfig, ], entry_point = Label("//packages/typescript/internal/worker:worker_adapter"), From 342f052498e748dba714ffc41291369f6320426f Mon Sep 17 00:00:00 2001 From: Daniel Muller Date: Sun, 1 Nov 2020 16:26:23 -0700 Subject: [PATCH 3/6] fix: do not pass --watch for non-worker mode --- packages/typescript/internal/ts_project.bzl | 3 +- .../internal/worker/worker_adapter.js | 28 +++++++++++++++---- 2 files changed, 24 insertions(+), 7 deletions(-) diff --git a/packages/typescript/internal/ts_project.bzl b/packages/typescript/internal/ts_project.bzl index 89bb3824a7..5bd7bb0e69 100644 --- a/packages/typescript/internal/ts_project.bzl +++ b/packages/typescript/internal/ts_project.bzl @@ -79,7 +79,7 @@ def _ts_project_impl(ctx): arguments.use_param_file("@%s", use_always = True) arguments.set_param_file_format("multiline") execution_requirements["supports-workers"] = "1" - execution_requirements["worker-key-mnemonic"] = "TsProjectMnemonic" + execution_requirements["worker-key-mnemonic"] = "TsProject" progress_prefix = "Compiling TypeScript project (worker mode)" generated_srcs = False @@ -618,7 +618,6 @@ def ts_project_macro( "--outDir", "$(RULEDIR)", # FIXME: what about other settings like declaration_dir, root_dir, etc - "--watch", ], ) diff --git a/packages/typescript/internal/worker/worker_adapter.js b/packages/typescript/internal/worker/worker_adapter.js index aa1e542cc1..88a95fe90d 100644 --- a/packages/typescript/internal/worker/worker_adapter.js +++ b/packages/typescript/internal/worker/worker_adapter.js @@ -1,16 +1,14 @@ const child_process = require('child_process'); - +const MNEMONIC = 'TsProject'; const worker = require('./worker'); const workerArg = process.argv.indexOf('--persistent_worker') if (workerArg > 0) { - process.argv.splice(workerArg, 1) + process.argv.splice(workerArg, 1, '--watch') if (process.platform !== 'linux' && process.platform !== 'darwin') { throw new Error('Worker mode is only supported on unix type systems.'); } - - worker.runWorkerLoop(awaitOneBuild); } const [tscBin, ...tscArgs] = process.argv.slice(2); @@ -20,7 +18,6 @@ const child = child_process.spawn( tscArgs, {stdio: 'pipe'}, ); - function awaitOneBuild() { child.kill('SIGCONT') @@ -49,4 +46,25 @@ function awaitOneBuild() { }; child.stdout.on('data', awaitBuild); }); +} + +async function main() { + // Bazel will pass a special argument to the program when it's running us as a worker + if (workerArg > 0) { + worker.log(`Running ${MNEMONIC} as a Bazel worker`); + + worker.runWorkerLoop(awaitOneBuild); + } else { + // Running standalone so stdout is available as usual + console.log(`Running ${MNEMONIC} as a standalone process`); + console.error( + `Started a new process to perform this action. Your build might be misconfigured, try + --strategy=${MNEMONIC}=worker`); + + child.on('exit', code => process.exit(code)); + } +} + +if (require.main === module) { + main(); } \ No newline at end of file From a69c6cdf49640f040c6e85bec55fa643170c0ade Mon Sep 17 00:00:00 2001 From: Daniel Muller Date: Sun, 1 Nov 2020 17:31:29 -0700 Subject: [PATCH 4/6] fix: do not test worker on windows --- packages/typescript/test/ts_project/worker/BUILD.bazel | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/typescript/test/ts_project/worker/BUILD.bazel b/packages/typescript/test/ts_project/worker/BUILD.bazel index 1abe48d0de..73924e9036 100644 --- a/packages/typescript/test/ts_project/worker/BUILD.bazel +++ b/packages/typescript/test/ts_project/worker/BUILD.bazel @@ -2,6 +2,7 @@ load("//packages/typescript:index.bzl", "ts_project") ts_project( supports_workers = True, + tags = ["fix-windows"], tsconfig = { "compilerOptions": { "declaration": True, From 83f8d37be0f3493f7e30e5c8e22720abbed4ebb7 Mon Sep 17 00:00:00 2001 From: Dan Muller Date: Mon, 2 Nov 2020 16:04:02 +0000 Subject: [PATCH 5/6] fix: log on standalone failures --- packages/typescript/internal/ts_project.bzl | 23 +++++++++++++++++-- .../internal/worker/worker_adapter.js | 20 +++++++++++++++- 2 files changed, 40 insertions(+), 3 deletions(-) diff --git a/packages/typescript/internal/ts_project.bzl b/packages/typescript/internal/ts_project.bzl index 5bd7bb0e69..c9146a82af 100644 --- a/packages/typescript/internal/ts_project.bzl +++ b/packages/typescript/internal/ts_project.bzl @@ -21,6 +21,13 @@ _DEFAULT_TSC_BIN = ( "//:node_modules/typescript/bin/tsc" ) +_DEFAULT_TYPESCRIP_MODULE = ( + # BEGIN-INTERNAL + "@npm" + + # END-INTERNAL + "//typescript" +) + _ATTRS = { "args": attr.string_list(), "declaration_dir": attr.string(), @@ -314,6 +321,8 @@ def ts_project_macro( emit_declaration_only = False, ts_build_info_file = None, tsc = None, + worker_tsc_bin = _DEFAULT_TSC_BIN, + worker_typescript_module = _DEFAULT_TYPESCRIP_MODULE, validate = True, supports_workers = False, declaration_dir = None, @@ -481,6 +490,15 @@ def ts_project_macro( For example, `tsc = "@my_deps//typescript/bin:tsc"` Or you can pass a custom compiler binary instead. + worker_tsc_bin: Label of the TypeScript compiler binary to run when running in worker mode. + + For example, `tsc = "@my_deps//node_modules/typescript/bin/tsc"` + Or you can pass a custom compiler binary instead. + + worker_typescript_module: Label of the package containing all data deps of worker_tsc_bin. + + For example, `tsc = "@my_deps//typescript"` + validate: boolean; whether to check that the tsconfig settings match the attributes. supports_workers: Experimental! Use only with caution. @@ -604,14 +622,15 @@ def ts_project_macro( name = tsc_worker, data = [ Label("//packages/typescript/internal/worker:worker"), - Label(_DEFAULT_TSC_BIN), + Label(worker_tsc_bin), + Label(worker_typescript_module), Label(protobufjs), tsconfig, ], entry_point = Label("//packages/typescript/internal/worker:worker_adapter"), templated_args = [ "--nobazel_patch_module_resolver", - "$(execpath {})".format(Label(_DEFAULT_TSC_BIN)), + "$(execpath {})".format(Label(worker_tsc_bin)), "--project", "$(execpath {})".format(tsconfig), # FIXME: should take out_dir into account diff --git a/packages/typescript/internal/worker/worker_adapter.js b/packages/typescript/internal/worker/worker_adapter.js index 88a95fe90d..f96d3f08f2 100644 --- a/packages/typescript/internal/worker/worker_adapter.js +++ b/packages/typescript/internal/worker/worker_adapter.js @@ -61,7 +61,25 @@ async function main() { `Started a new process to perform this action. Your build might be misconfigured, try --strategy=${MNEMONIC}=worker`); - child.on('exit', code => process.exit(code)); + const stdoutbuffer = []; + child.stdout.on('data', data => stdoutbuffer.push(data)); + + const stderrbuffer = []; + child.stderr.on('data', data => stderrbuffer.push(data)); + + child.on('exit', code => { + if (code !== 0) { + console.error( + `\nstdout from tsc:\n\n ${ + stdoutbuffer.map(s => s.toString()).join('').replace(/\n/g, '\n ')}`, + ) + console.error( + `\nstderr from tsc:\n\n ${ + stderrbuffer.map(s => s.toString()).join('').replace(/\n/g, '\n ')}`, + ) + } + process.exit(code) + }); } } From 11db7c3b5d6fe6093607ef4fcadb107d0498afc5 Mon Sep 17 00:00:00 2001 From: Alex Eagle Date: Mon, 9 Nov 2020 16:24:43 -0800 Subject: [PATCH 6/6] chore: docs improvements --- packages/typescript/internal/ts_project.bzl | 14 ++++++++++---- .../typescript/internal/worker/worker_adapter.js | 15 +++++++++++++-- 2 files changed, 23 insertions(+), 6 deletions(-) diff --git a/packages/typescript/internal/ts_project.bzl b/packages/typescript/internal/ts_project.bzl index c9146a82af..4dff0c71a5 100644 --- a/packages/typescript/internal/ts_project.bzl +++ b/packages/typescript/internal/ts_project.bzl @@ -21,7 +21,7 @@ _DEFAULT_TSC_BIN = ( "//:node_modules/typescript/bin/tsc" ) -_DEFAULT_TYPESCRIP_MODULE = ( +_DEFAULT_TYPESCRIPT_MODULE = ( # BEGIN-INTERNAL "@npm" + # END-INTERNAL @@ -322,7 +322,7 @@ def ts_project_macro( ts_build_info_file = None, tsc = None, worker_tsc_bin = _DEFAULT_TSC_BIN, - worker_typescript_module = _DEFAULT_TYPESCRIP_MODULE, + worker_typescript_module = _DEFAULT_TYPESCRIPT_MODULE, validate = True, supports_workers = False, declaration_dir = None, @@ -503,8 +503,14 @@ def ts_project_macro( supports_workers: Experimental! Use only with caution. - Allows you to enable the Bazel Worker strategy for this project. - This requires that the tsc binary support it. + Allows you to enable the Bazel Persistent Workers strategy for this project. + See https://docs.bazel.build/versions/master/persistent-workers.html + + This requires that the tsc binary support a `--watch` option. + + NOTE: this does not work on Windows yet. + We will silently fallback to non-worker mode on Windows regardless of the value of this attribute. + Follow https://github.com/bazelbuild/rules_nodejs/issues/2277 for progress on this feature. root_dir: a string specifying a subdirectory under the input package which should be consider the root directory of all the input files. diff --git a/packages/typescript/internal/worker/worker_adapter.js b/packages/typescript/internal/worker/worker_adapter.js index f96d3f08f2..38240228b7 100644 --- a/packages/typescript/internal/worker/worker_adapter.js +++ b/packages/typescript/internal/worker/worker_adapter.js @@ -1,3 +1,13 @@ +/** + * @fileoverview wrapper program around the TypeScript compiler, tsc + * + * It intercepts the Bazel Persistent Worker protocol, using it to remote-control tsc running as a + * child process. In between builds, the tsc process is stopped (akin to ctrl-z in a shell) and then + * resumed (akin to `fg`) when the inputs have changed. + * + * See https://medium.com/@mmorearty/how-to-create-a-persistent-worker-for-bazel-7738bba2cabb + * for more background (note, that is documenting a different implementation) + */ const child_process = require('child_process'); const MNEMONIC = 'TsProject'; const worker = require('./worker'); @@ -7,7 +17,8 @@ if (workerArg > 0) { process.argv.splice(workerArg, 1, '--watch') if (process.platform !== 'linux' && process.platform !== 'darwin') { - throw new Error('Worker mode is only supported on unix type systems.'); + throw new Error(`Worker mode is only supported on linux and darwin, not ${process.platform}. + See https://github.com/bazelbuild/rules_nodejs/issues/2277`); } } @@ -85,4 +96,4 @@ async function main() { if (require.main === module) { main(); -} \ No newline at end of file +}