diff --git a/docs/rules.md b/docs/rules.md index 54fd4d97..606e5376 100644 --- a/docs/rules.md +++ b/docs/rules.md @@ -39,10 +39,9 @@ extended configuration file as well, to pass them both to the TypeScript compile 
 ts_project_rule(name, allow_js, args, assets, buildinfo_out, composite, data, declaration,
                 declaration_dir, declaration_map, deps, emit_declaration_only, extends, incremental,
-                internal_do_not_depend_supports_workers_is_none, is_typescript_5_or_greater, js_outs,
-                map_outs, out_dir, preserve_jsx, resolve_json_module, root_dir, source_map, srcs,
-                supports_workers, transpile, tsc, tsc_worker, tsconfig, typing_maps_outs,
-                typings_outs)
+                is_typescript_5_or_greater, js_outs, map_outs, out_dir, preserve_jsx,
+                resolve_json_module, root_dir, source_map, srcs, supports_workers, transpile, tsc,
+                tsc_worker, tsconfig, typing_maps_outs, typings_outs)
Implementation rule behind the ts_project macro. @@ -71,7 +70,6 @@ Implementation rule behind the ts_project macro. | emit_declaration_only | https://www.typescriptlang.org/tsconfig#emitDeclarationOnly | Boolean | optional | False | | extends | https://www.typescriptlang.org/tsconfig#extends | Label | optional | None | | incremental | https://www.typescriptlang.org/tsconfig#incremental | Boolean | optional | False | -| internal_do_not_depend_supports_workers_is_none | Internal. DO NOT DEPEND! | Boolean | optional | False | | is_typescript_5_or_greater | Whether TypeScript version is >= 5.0.0 | Boolean | optional | False | | js_outs | Locations in bazel-out where tsc will write .js files | List of labels | optional | | | map_outs | Locations in bazel-out where tsc will write .js.map files | List of labels | optional | | @@ -81,7 +79,7 @@ Implementation rule behind the ts_project macro. | root_dir | https://www.typescriptlang.org/tsconfig#rootDir | String | optional | "" | | source_map | https://www.typescriptlang.org/tsconfig#sourceMap | Boolean | optional | False | | srcs | TypeScript source files | List of labels | required | | -| supports_workers | Whether the tsc compiler understands Bazel's persistent worker protocol | Boolean | optional | False | +| supports_workers | Whether to use a custom tsc compiler which understands Bazel's persistent worker protocol.

See the docs for supports_workers on the [ts_project](#ts_project-supports_workers) macro. | Integer | optional | 0 | | transpile | whether tsc should be used to produce .js outputs | Boolean | optional | True | | tsc | TypeScript compiler binary | Label | required | | | tsc_worker | TypeScript compiler worker binary | Label | required | | @@ -210,7 +208,7 @@ If you have problems getting your `ts_project` to work correctly, read the dedic | declaration_dir | String specifying a subdirectory under the bazel-out folder where generated declaration outputs are written. Equivalent to the TypeScript --declarationDir option. By default declarations are written to the out_dir. | None | | out_dir | String specifying a subdirectory under the bazel-out folder where outputs are written. Equivalent to the TypeScript --outDir option.

Note that Bazel always requires outputs be written under a subdirectory matching the input package, so if your rule appears in path/to/my/package/BUILD.bazel and out_dir = "foo" then the .js files will appear in bazel-out/[arch]/bin/path/to/my/package/foo/*.js.

By default the out_dir is the package's folder under bazel-out. | None | | root_dir | 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. By default it is '.', meaning the source directory where the BUILD file lives. | None | -| supports_workers | Whether the worker protocol is enabled. To disable worker mode for a particular target set supports_workers to False.

Note that value of this attribute always preferred over --@aspect_rules_ts//ts:supports_workers flag unless the supports_workers attribute is not set explicitly.

Worker mode can be disabled workspace wide by using the --@aspect_rules_ts//ts:supports_workers flag. To disable worker mode globally, insert build --@aspect_rules_ts//ts:supports_workers=false into the .bazelrc.

Alternatively, worker mode can be controlled via --strategy. To disable worker mode globally via --strategy insert build --strategy=TsProject=sandboxed into the .bazelrc.

See https://docs.bazel.build/versions/main/user-manual.html#flag--strategy for more details. | None | +| supports_workers | Whether the "Persistent Worker" protocol is enabled. This uses a custom tsc compiler to make rebuilds faster. Note that this causes some known correctness bugs, see https://docs.aspect.build/rules/aspect_rules_ts/docs/troubleshooting. We do not intend to fix these bugs.

Worker mode can be enabled for all ts_projects in a build with the global --@aspect_rules_ts//ts:supports_workers flag. To enable worker mode for all builds in the workspace, add build --@aspect_rules_ts//ts:supports_workers to the .bazelrc.

This is a "tri-state" attribute, accepting values [-1, 0, 1]. The behavior is:

- -1: use the value of the global --@aspect_rules_ts//ts:supports_workers flag. - 0: Override the global flag, disabling workers for this target. - 1: Override the global flag, enabling workers for this target. | -1 | | kwargs | passed through to underlying [ts_project_rule](#ts_project_rule), eg. visibility, tags | none | diff --git a/docs/troubleshooting.md b/docs/troubleshooting.md index 59f40900..0e42e65d 100644 --- a/docs/troubleshooting.md +++ b/docs/troubleshooting.md @@ -67,16 +67,17 @@ redirect the stdout to a file that you can analyze with power tools. ## Non-deterministic behavior -By default, we run `tsc` in a "watch mode" using the [Bazel Persistent Worker](https://bazel.build/remote/persistent) feature. +When Worker Support is enabled, we run `tsc` in a "watch mode" using the [Bazel Persistent Worker](https://bazel.build/remote/persistent) feature. +See the [`supports_workers`](https://docs.aspect.build/rules/aspect_rules_ts/docs/rules#supports_workers-1) attribute for docs on enabling this feature. + However, this risks leaking state from one compilation to another, and you may still encounter such bugs, for example: - removing a required types package from `deps` but the compilation still succeeds - outputs created by a previous compilation are still produced even though the source file is deleted You can confirm that it's a worker bug by running `bazel shutdown` and trying again. If that resolves the issue, it means that some state was leaking. -Please check for issues or file one if you find a bug with persistent workers. - -To disable persistent workers for a single target, use the `supports_workers` attribute of `ts_project`. To disable globally, add the line `build --strategy=TsProject=sandboxed` to your `.bazelrc`. +Please check for issues with persistent workers: +[persistent workers label](https://github.com/aspect-build/rules_ts/issues?q=label%3A%22persistent+workers%22) ## Which files should be emitted diff --git a/e2e/test/strategy.bats b/e2e/test/strategy.bats index 048426e3..04ff285b 100644 --- a/e2e/test/strategy.bats +++ b/e2e/test/strategy.bats @@ -27,7 +27,7 @@ teardown() { ts_project --src "source.ts" echo 'const t: string = "local";' > source.ts - run bazel build :foo --strategy=TsProject=local + run bazel build :foo --strategy=TsProject=local --@aspect_rules_ts//ts:supports_workers assert_success assert_output -p "WARNING: Running" "TsProject" "as a standalone process" "From Compiling TypeScript project" } diff --git a/e2e/test/third_party_deps.bats b/e2e/test/third_party_deps.bats index ab09986b..12468372 100644 --- a/e2e/test/third_party_deps.bats +++ b/e2e/test/third_party_deps.bats @@ -88,7 +88,7 @@ teardown() { echo "import {cool} from '@feature/cool'; export const t: number = cool()" > source.ts ts_project -l -s "source.ts" -d ":node_modules/@feature/cool" - run bazel build :foo + run bazel build :foo --@aspect_rules_ts//ts:supports_workers assert_failure assert_output -p "node_modules/.aspect_rules_js/@feature+cool@0.0.0/node_modules/@feature/cool/index.d.ts(1,40): error TS2307: Cannot find module '@feature/notcool' or its corresponding type declarations." @@ -96,6 +96,6 @@ teardown() { echo '{"dependencies":{"@feature/cool": "workspace:*", "@types/node": "*"}, "pnpm": {"packageExtensions": {"@feature/cool": {"dependencies": {"@feature/notcool": "workspace:*"}}}}}' > package.json run pnpm install --lockfile-only assert_success - run bazel build :foo + run bazel build :foo --@aspect_rules_ts//ts:supports_workers assert_success } \ No newline at end of file diff --git a/e2e/test/tracing_and_performance.bats b/e2e/test/tracing_and_performance.bats index d0155dfc..16480239 100644 --- a/e2e/test/tracing_and_performance.bats +++ b/e2e/test/tracing_and_performance.bats @@ -15,7 +15,7 @@ teardown() { tsconfig ts_project --src "source.ts" echo "export const f = 1;" > source.ts - run bazel build :foo + run bazel build :foo --@aspect_rules_ts//ts:supports_workers assert_success run cat $(bazel info output_base)/bazel-workers/worker-1-TsProject.log assert_output -p "# Beginning new work" "# Finished the work" "creating a new worker with the key" diff --git a/e2e/worker/.bazelrc b/e2e/worker/.bazelrc index 3636fce2..0e8e62a0 100644 --- a/e2e/worker/.bazelrc +++ b/e2e/worker/.bazelrc @@ -2,4 +2,5 @@ import %workspace%/../../.bazelrc.common build:local --worker_verbose -build:no-disk-cache --disk_cache=/tmp/no-disk-cache \ No newline at end of file +build:no-disk-cache --disk_cache=/tmp/no-disk-cache +build --@aspect_rules_ts//ts:supports_workers diff --git a/ts/BUILD.bazel b/ts/BUILD.bazel index 7e350424..8fc53900 100644 --- a/ts/BUILD.bazel +++ b/ts/BUILD.bazel @@ -65,7 +65,7 @@ bool_flag( bool_flag( name = "supports_workers", - build_setting_default = True, + build_setting_default = False, visibility = ["//visibility:public"], ) @@ -116,7 +116,6 @@ options( ), supports_workers = select({ ":supports_workers_flag": True, - # TODO(2.0): default should be False if we set swc as default "//conditions:default": False, }), verbose = select({ diff --git a/ts/defs.bzl b/ts/defs.bzl index 90d2e6ac..9ca5fc04 100644 --- a/ts/defs.bzl +++ b/ts/defs.bzl @@ -66,7 +66,7 @@ def ts_project( declaration_dir = None, out_dir = None, root_dir = None, - supports_workers = None, + supports_workers = -1, **kwargs): """Compiles one TypeScript project using `tsc --project`. @@ -225,19 +225,22 @@ def ts_project( ts_build_info_file: The user-specified value of `tsBuildInfoFile` from the tsconfig. Helps Bazel to predict the path where the .tsbuildinfo output is written. - supports_workers: Whether the worker protocol is enabled. - To disable worker mode for a particular target set `supports_workers` to `False`. + supports_workers: Whether the "Persistent Worker" protocol is enabled. + This uses a custom `tsc` compiler to make rebuilds faster. + Note that this causes some known correctness bugs, see + https://docs.aspect.build/rules/aspect_rules_ts/docs/troubleshooting. + We do not intend to fix these bugs. - Note that value of this attribute always preferred over `--@aspect_rules_ts//ts:supports_workers` flag - unless the `supports_workers` attribute is not set explicitly. + Worker mode can be enabled for all `ts_project`s in a build with the global + `--@aspect_rules_ts//ts:supports_workers` flag. + To enable worker mode for all builds in the workspace, add + `build --@aspect_rules_ts//ts:supports_workers` to the .bazelrc. - Worker mode can be disabled workspace wide by using the `--@aspect_rules_ts//ts:supports_workers` flag. - To disable worker mode globally, insert `build --@aspect_rules_ts//ts:supports_workers=false` into the .bazelrc. + This is a "tri-state" attribute, accepting values `[-1, 0, 1]`. The behavior is: - Alternatively, worker mode can be controlled via `--strategy`. - To disable worker mode globally via `--strategy` insert `build --strategy=TsProject=sandboxed` into the .bazelrc. - - See https://docs.bazel.build/versions/main/user-manual.html#flag--strategy for more details. + - `-1`: use the value of the global `--@aspect_rules_ts//ts:supports_workers` flag. + - `0`: Override the global flag, disabling workers for this target. + - `1`: Override the global flag, enabling workers for this target. **kwargs: passed through to underlying [`ts_project_rule`](#ts_project_rule), eg. `visibility`, `tags` """ @@ -391,7 +394,7 @@ def ts_project( # Disable workers if a custom tsc was provided but not a custom tsc_worker. if tsc != _tsc and tsc_worker == _tsc_worker: - supports_workers = False + supports_workers = 0 ts_project_rule( name = tsc_target_name, @@ -427,6 +430,5 @@ def ts_project( "@npm_typescript//:is_typescript_5_or_greater": True, "//conditions:default": False, }), - internal_do_not_depend_supports_workers_is_none = supports_workers == None, **kwargs ) diff --git a/ts/private/ts_lib.bzl b/ts/private/ts_lib.bzl index fa06bf43..cdd268f4 100644 --- a/ts/private/ts_lib.bzl +++ b/ts/private/ts_lib.bzl @@ -41,19 +41,19 @@ See more details on the `assets` parameter of the `ts_project` macro. allow_files = True, mandatory = True, ), - "supports_workers": attr.bool( - doc = "Whether the tsc compiler understands Bazel's persistent worker protocol", - default = False, + "supports_workers": attr.int( + doc = """\ + Whether to use a custom `tsc` compiler which understands Bazel's persistent worker protocol. + + See the docs for `supports_workers` on the [`ts_project`](#ts_project-supports_workers) macro. + """, + default = 0, + values = [-1, 0, 1], ), "is_typescript_5_or_greater": attr.bool( doc = "Whether TypeScript version is >= 5.0.0", default = False, ), - # TODO(2.0): remove this and make supports_workers a tri-state int. - "internal_do_not_depend_supports_workers_is_none": attr.bool( - doc = "Internal. DO NOT DEPEND!", - default = False, - ), "transpile": attr.bool( doc = "whether tsc should be used to produce .js outputs", default = True, diff --git a/ts/private/ts_project.bzl b/ts/private/ts_project.bzl index 67cc5e1b..739c9121 100644 --- a/ts/private/ts_project.bzl +++ b/ts/private/ts_project.bzl @@ -67,13 +67,11 @@ def _ts_project_impl(ctx): execution_requirements = {} executable = ctx.executable.tsc - supports_workers = ctx.attr.supports_workers - - # workers can be enabled/disabled globally. if no supports_workers attribute is set explicitly for this target, - # which is indicated by internal_do_not_depend_supports_workers_is_none attribute, then set it to global supports_workers config - # TODO(2.0): remove this - if ctx.attr.internal_do_not_depend_supports_workers_is_none: - supports_workers = options.supports_workers + supports_workers = options.supports_workers + if ctx.attr.supports_workers == 1: + supports_workers = True + elif ctx.attr.supports_workers == 0: + supports_workers = False host_is_windows = platform_utils.host_platform_is_windows() if host_is_windows and supports_workers: diff --git a/ts/test/flags_test.bzl b/ts/test/flags_test.bzl index bc5d1502..be504503 100644 --- a/ts/test/flags_test.bzl +++ b/ts/test/flags_test.bzl @@ -55,46 +55,6 @@ def _find_tsc_action(env, target_under_test): asserts.true(env, found_action != None, "could not find the TsProject action") return found_action -# explicit supports_workers = true -def _supports_workers_explicitly_true_test_impl(ctx): - env = analysistest.begin(ctx) - target_under_test = analysistest.target_under_test(env) - action = _find_tsc_action(env, target_under_test) - asserts.true(env, action.argv[0].find("npm_typescript/tsc_worker.sh") != -1, "expected workers to be enabled explicitly.") - return analysistest.end(env) - -_supports_workers_explicitly_true_test = analysistest.make(_supports_workers_explicitly_true_test_impl) - -# explicit supports_workers = false -def _supports_workers_explicitly_false_test_impl(ctx): - env = analysistest.begin(ctx) - target_under_test = analysistest.target_under_test(env) - action = _find_tsc_action(env, target_under_test) - asserts.true(env, action.argv[0].find("npm_typescript/tsc.sh") != -1, "expected workers to be disabled explicitly.") - return analysistest.end(env) - -_supports_workers_explicitly_false_test = analysistest.make(_supports_workers_explicitly_false_test_impl) - -# implicit supports_workers = true -def _supports_workers_implicitly_true_test_impl(ctx): - env = analysistest.begin(ctx) - target_under_test = analysistest.target_under_test(env) - action = _find_tsc_action(env, target_under_test) - asserts.true(env, action.argv[0].find("npm_typescript/tsc_worker.sh") != -1, "expected workers to be enabled globally.") - return analysistest.end(env) - -_supports_workers_implicitly_true_test = analysistest.make(_supports_workers_implicitly_true_test_impl) - -# implicit supports_workers = false -def _supports_workers_implicitly_false_test_impl(ctx): - env = analysistest.begin(ctx) - target_under_test = analysistest.target_under_test(env) - action = _find_tsc_action(env, target_under_test) - asserts.true(env, action.argv[0].find("npm_typescript/tsc.sh") != -1, "expected workers to be disabled globally.") - return analysistest.end(env) - -_supports_workers_implicitly_false_test = analysistest.make(_supports_workers_implicitly_false_test_impl) - # verbose flag = true test def _verbose_true_test_impl(ctx): env = analysistest.begin(ctx) @@ -179,52 +139,6 @@ def ts_project_flags_test_suite(name): }, } - _ts_project_with_flags( - name = "supports_workers_explicitly_true", - tsconfig = _TSCONFIG, - supports_workers = True, - # supports_workers should override the flag - supports_workers_flag = False, - ) - _supports_workers_explicitly_true_test( - name = "supports_workers_explicitly_true_test", - target_under_test = ":supports_workers_explicitly_true", - ) - - _ts_project_with_flags( - name = "supports_workers_explicitly_false", - tsconfig = _TSCONFIG, - supports_workers = False, - # supports_workers should override the flag - supports_workers_flag = True, - ) - _supports_workers_explicitly_false_test( - name = "supports_workers_explicitly_false_test", - target_under_test = ":supports_workers_explicitly_false", - ) - - _ts_project_with_flags( - name = "supports_workers_implicitly_true", - tsconfig = _TSCONFIG, - # supports_workers attribute is not set explicitly so the flag should be respected - supports_workers_flag = True, - ) - _supports_workers_implicitly_true_test( - name = "supports_workers_implicitly_true_test", - target_under_test = ":supports_workers_implicitly_true", - ) - - _ts_project_with_flags( - name = "supports_workers_implicitly_false", - tsconfig = _TSCONFIG, - # supports_workers attribute is not set explicitly so the flag should be respected - supports_workers_flag = False, - ) - _supports_workers_implicitly_false_test( - name = "supports_workers_implicitly_false_test", - target_under_test = ":supports_workers_implicitly_false", - ) - _ts_project_with_flags( name = "verbose_true", tsconfig = _TSCONFIG, @@ -268,10 +182,6 @@ def ts_project_flags_test_suite(name): native.test_suite( name = name, tests = [ - ":supports_workers_explicitly_true_test", - ":supports_workers_explicitly_false_test", - ":supports_workers_implicitly_true_test", - ":supports_workers_implicitly_false_test", ":verbose_true_test", ":verbose_false_test", ":skip_lib_check_always_test",