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

ts_project cannot find deps with --worker_sandboxing #127

Closed
tsawada opened this issue Aug 14, 2022 · 8 comments
Closed

ts_project cannot find deps with --worker_sandboxing #127

tsawada opened this issue Aug 14, 2022 · 8 comments
Labels
enhancement New feature or request help wanted Aspect isn't prioritizing this, but the community could persistent workers Bug is only reproducible when using supports_workers

Comments

@tsawada
Copy link
Contributor

tsawada commented Aug 14, 2022

$ bazel build --worker_sandboxing //examples/simple:ts
...
ERROR: examples/simple/BUILD.bazel:15:11: Compiling TypeScript project //examples/simple:ts [tsc -p examples/simple/tsconfig.json] failed: (Exit 1): tsc_worker.sh failed: error executing command bazel-out/darwin_arm64-opt-exec-2B5CBBC6/bin/external/npm_typescript/tsc_worker.sh @bazel-out/darwin_arm64-fastbuild/bin/examples/simple/foo.js-0.params

error TS2688: Cannot find type definition file for 'node'.
  The file is in the program because:
    Entry point of type library 'node' specified in compilerOptions
examples/simple/foo.ts(1,17): error TS2307: Cannot find module '@myorg/js_lib' or its corresponding type declarations.
examples/simple/foo.ts(2,19): error TS2307: Cannot find module '@myorg/dts_lib' or its corresponding type declarations.
examples/simple/foo.ts(3,24): error TS2307: Cannot find module 'date-fns' or its corresponding type declarations.

Target //examples/simple:ts failed to build
...

I'm not sure if this should work in the first place but maybe it'd help if documented.

@thesayyn
Copy link
Member

--worker_sandboxing is not supported at the moment due to paths being different for every subsequent build. We should document this and why this is not needed as we already have a vfs in place to enforce sandboxing even without worker sandboxing.

Was there a reason why you needed this?

@tsawada
Copy link
Contributor Author

tsawada commented Sep 23, 2022

Thanks. Well, my understanding is that it is generally a good idea to use --worker_sandboxing for reproducibility and correctness. My repository has multiple different workers (javac, kotlin, etc) in addition to tsc and it'd be great we can keep them isolated.
It might be bazel's fault that it cannot selectively enable sandboxing per worker type though.

@thesayyn
Copy link
Member

thesayyn commented Sep 23, 2022

It might be bazel's fault that it cannot selectively enable sandboxing per worker type though.

This sounds like a missing bazel feature.

Thanks. Well, my understanding is that it is generally a good idea to use --worker_sandboxing for reproducibility and correctness.

tsc does not play well with changing paths. AFAIK there is no trivial way to get worker_sandboxing work. We will want to support this eventually but given the presence of vfs, we already have worker isolation

@cgrindel cgrindel added enhancement New feature or request help wanted Aspect isn't prioritizing this, but the community could labels Sep 27, 2022
@thesayyn
Copy link
Member

to clarify further why this doesn't work;

let's think about how tsc works outside of bazel for a moment. Usually, there is a directory with a bunch of ts files residing next to a tsconfig.json and you invoke tsc with tsc --watch and it will output to a dist directory. Next time you can a file, tsc will catch the change and recompile the file and write the output to the dist directory. tsc assumes the path of the files and output directory never changes, therefore using the absolute path to cache state.

this creates a few problems under bazel with --worker_sandboxing, but before getting into it, let's see how --worker_sandboxing works.

Anytime a ts_project target needs to be built, bazel will create a distinct folder and put all the inputs into that folder; then change the working directory of the worker to the new folder. Let's compare the working directory with and without --worker_sandboxing

--worker_sandboxing        <output-base>/bazel-workers/worker-<worker_idx>-TsProject/<workspace>

--noworker_sandboxing      <output-base>/execroot/<workspace>

A few problems with this;

  • tsc only checks the working directory once, at the beginning of the program. for the subsequent builds tsc will not check the working directory.
  • output of the subsequent builds will be written into the initial working directory. (the one determined at the beginning)
  • packages from node_modules will be cached with their absolute path, and this will lead to type-checking errors

The solution is to keep the working directory always the same. Unfortunately, this is not possible with --worker_sandboxing and in fact how it ensures hermeticity and reproducibility.

@thesayyn
Copy link
Member

thesayyn commented Mar 2, 2023

Update on this one; recently we landed a big refactor to tsc workers. It allows us to support --worker_sandboxing.

@ghost
Copy link

ghost commented Mar 27, 2023

This seems to still not work, can I get an update? Worker sandboxing is important to me.

@thesayyn
Copy link
Member

This seems to still not work, can I get an update? Worker sandboxing is important to me.

We do not support it yet but the prefactor made it possible to support it with insignificant changes.

@alexeagle
Copy link
Member

Due to bugs like this one, we are moving away from supporting the Persistent Worker in the next major 2.0 release of rules_ts, and likely will never fix this, sorry!

@github-project-automation github-project-automation bot moved this from 📋 Backlog to ✅ Done in Open Source Aug 8, 2023
@alexeagle alexeagle added the persistent workers Bug is only reproducible when using supports_workers label Aug 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Aspect isn't prioritizing this, but the community could persistent workers Bug is only reproducible when using supports_workers
Projects
Archived in project
Development

No branches or pull requests

4 participants