Skip to content

Commit

Permalink
chore: flip supports_workers to default False
Browse files Browse the repository at this point in the history
Fixes #400
  • Loading branch information
alexeagle committed Aug 9, 2023
1 parent 681e216 commit b1b0b00
Show file tree
Hide file tree
Showing 11 changed files with 45 additions and 136 deletions.
12 changes: 5 additions & 7 deletions docs/rules.md

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

9 changes: 5 additions & 4 deletions docs/troubleshooting.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
2 changes: 1 addition & 1 deletion e2e/test/strategy.bats
Original file line number Diff line number Diff line change
Expand Up @@ -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"
}
4 changes: 2 additions & 2 deletions e2e/test/third_party_deps.bats
Original file line number Diff line number Diff line change
Expand Up @@ -88,14 +88,14 @@ 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/@[email protected]/node_modules/@feature/cool/index.d.ts(1,40): error TS2307: Cannot find module '@feature/notcool' or its corresponding type declarations."


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
}
2 changes: 1 addition & 1 deletion e2e/test/tracing_and_performance.bats
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
3 changes: 2 additions & 1 deletion e2e/worker/.bazelrc
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,5 @@
import %workspace%/../../.bazelrc.common

build:local --worker_verbose
build:no-disk-cache --disk_cache=/tmp/no-disk-cache
build:no-disk-cache --disk_cache=/tmp/no-disk-cache
build --@aspect_rules_ts//ts:supports_workers
3 changes: 1 addition & 2 deletions ts/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ bool_flag(

bool_flag(
name = "supports_workers",
build_setting_default = True,
build_setting_default = False,
visibility = ["//visibility:public"],
)

Expand Down Expand Up @@ -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({
Expand Down
28 changes: 15 additions & 13 deletions ts/defs.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -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`.
Expand Down Expand Up @@ -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`
"""
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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
)
16 changes: 8 additions & 8 deletions ts/private/ts_lib.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
12 changes: 5 additions & 7 deletions ts/private/ts_project.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
Loading

0 comments on commit b1b0b00

Please sign in to comment.