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

[Bug]: Bazel fails to build a ts_project target that compiles with tsc. #606

Closed
cjjob opened this issue May 9, 2024 · 4 comments
Closed
Labels
documentation Improvements or additions to documentation more info needed More info needed to proceed further

Comments

@cjjob
Copy link

cjjob commented May 9, 2024

What happened?

Summary

We are observing that a package compiles correctly with tsc but fails to build with bazel build path/to:target, where the target is defined using the ts_project() rule. We believe this could be a bug as the documentation suggests "code that works with tsc should work with ts_project with a few caveats", and we do not appear to satisfy the caveat conditions (although, I have limited TypeScript knowledge so this might not be a bug).

There are different TypeScript compilation errors, for example:
error TS2339: Property 'autoEat' does not exist on type 'Bot'.
222 this.bot.autoEat.options.priority = "foodPoints";

error TS7006: Parameter 'item' implicitly has an 'any' type.
433 bot.on("autoeat_started", (item, offhand) => {

The full distinct list of errors we see are TS2339, TS2345, TS7006.

Debugging steps

We got the exact command that bazel is failing on during the build. For reference it is:
cd /root/.cache/bazel/_bazel_root/98709fea43daaa81054092f92e9928bc/sandbox/processwrapper-sandbox/4301/execroot/_main && exec env - BAZEL_BINDIR=bazel-out/k8-fastbuild/bin TMPDIR=/tmp /root/.cache/bazel/_bazel_root/install/c8ddc30f81d03fbb6c764cadbda081c8/process-wrapper '--timeout=0' '--kill_delay=15' '--stats=/root/.cache/bazel/_bazel_root/98709fea43daaa81054092f92e9928bc/sandbox/processwrapper-sandbox/4301/stats.out' bazel-out/k8-opt-exec-ST-13d3ddad9198/bin/external/aspect_rules_ts~~ext~npm_typescript/tsc.sh --skipLibCheck --project agents/minecraft/tsconfig.json --outDir agents/minecraft/dist --rootDir agents/minecraft --declarationDir agents/minecraft/dist

We have then dug into some potential issues.

tsconfig.json

We think this can be ruled out because if we run the following two commands to get the respective tsc and bazel transpilation configs the resulting files summarising config settings are identical:

  1. tsc only: tsc --skipLibCheck --project agents/minecraft/tsconfig.json --outDir agents/minecraft/dist --rootDir agents/minecraft --declarationDir agents/minecraft/dist --showConfig >> tsc.txt
  2. bazel build's subcommand: BAZEL_BINDIR=bazel-out/k8-fastbuild/bin bazel-out/k8-opt-exec-ST-13d3ddad9198/bin/external/aspect_rules_ts~~ext~npm_typescript/tsc.sh --skipLibCheck --project agents/minecraft/tsconfig.json --outDir agents/minecraft/dist --rootDir agents/minecraft --declarationDir agents/minecraft/dist --showConfig >> tsc_bazel.txt

Versions

We have verified that tool versions match:

  • tsc is version 5.4.5, verified with:
    • tsc --version
    • BAZEL_BINDIR=bazel-out/k8-fastbuild/bin bazel-out/k8-opt-exec-ST-13d3ddad9198/bin/external/aspect_rules_ts~~ext~npm_typescript/tsc.sh --version
  • node is version 18.18.2, verfied with:
    • node --version
    • Our MODULE.bazel installs this version explicity:
      • node = use_extension("@rules_nodejs//nodejs:extensions.bzl", "node", dev_dependency = True)
      • node.toolchain(node_version = "18.18.2",)

BAZEL_BINDIR

Where things begin to go wrong is with BAZEL_BINDIR.

If we switch the BAZEL_BINDIR everything will compile. So

  • This will work (change is BAZEL_BINDIR=.):
    • BAZEL_BINDIR=. bazel-out/k8-opt-exec-ST-13d3ddad9198/bin/external/aspect_rules_ts~~ext~npm_typescript/tsc.sh --skipLibCheck --project agents/minecraft/tsconfig.json --outDir agents/minecraft/dist --rootDir agents/minecraft --declarationDir agents/minecraft/dist
  • This will fail with the same error as when running bazel build:
    • BAZEL_BINDIR=bazel-out/k8-fastbuild/bin bazel-out/k8-opt-exec-ST-13d3ddad9198/bin/external/aspect_rules_ts~~ext~npm_typescript/tsc.sh --skipLibCheck --project agents/minecraft/tsconfig.json --outDir agents/minecraft/dist --rootDir agents/minecraft --declarationDir agents/minecraft/dist

So, given the error seems to suggest that TypeScript can't find the types, maybe it's an issue with Bazel installing the packages?

Version

Development (host) and target OS/architectures:

Output of bazel --version: bazel 7.1.1

Version of the Aspect rules, or other relevant rules from your WORKSPACE or MODULE.bazel file: We are using MODULE.bazel. Below is the contents with only JS/TS components retained.

bazel_dep(name = "rules_nodejs", version = "6.1.0")
bazel_dep(name = "aspect_rules_js", version = "1.41.2")
bazel_dep(name = "aspect_rules_ts", version = "2.3.0")
bazel_dep(name = "aspect_rules_jest", version = "0.20.0")

node = use_extension("@rules_nodejs//nodejs:extensions.bzl", "node", dev_dependency = True)
node.toolchain(
    node_version = "18.18.2",
)
use_repo(
    node,
)

pnpm = use_extension("@aspect_rules_js//npm:extensions.bzl", "pnpm")
use_repo(pnpm, "pnpm")

npm = use_extension(
    "@aspect_rules_js//npm:extensions.bzl",
    "npm",
    dev_dependency = True,
)
npm.npm_translate_lock(
    name = "npm",
    pnpm_lock = "//:pnpm-lock.yaml",
    verify_node_modules_ignored = "//:.bazelignore",
)
use_repo(npm, "npm")

# TypeScript support 2.3.0
# https://docs.aspect.build/rulesets/aspect_rules_ts/
# https://github.com/aspect-build/rules_ts/releases
rules_ts_ext = use_extension("@aspect_rules_ts//ts:extensions.bzl", "ext", dev_dependency = True)
rules_ts_ext.deps()
use_repo(rules_ts_ext, "npm_typescript")

Language(s) and/or frameworks involved: TypeScript/JavaScript

How to reproduce

No response

Any other information?

Missing module imports

When we built with bazel build path/to:target we also originally had some errors relating to missing modules that did not happen with the tsc compilation. We resolved by adding the packages to our dependencies in package.json but it's worth noting that this was necessary in the first instance.

@cjjob cjjob added the bug Something isn't working label May 9, 2024
@github-actions github-actions bot added the untriaged Requires traige label May 9, 2024
@jbedard
Copy link
Member

jbedard commented May 9, 2024

I assume it is because types are not being explicitly declared and depend on the underlying .ts files. When one ts_project (a) depends on another (b), the types from (b) must be fully exposed in the .d.ts file since the original .ts is gone (not in the sandbox when (a) is compiled).

This is what --isolatedDeclarations will prevent by forcing your .ts to be written such that .d.ts have all the type information without the underlying .ts implementation.

Can you verify this? I thought this was documented better so maybe we just need to update that troubleshooting page with more info.

@jbedard jbedard added documentation Improvements or additions to documentation more info needed More info needed to proceed further and removed bug Something isn't working untriaged Requires traige labels May 9, 2024
@cjjob
Copy link
Author

cjjob commented May 12, 2024

Sorry for the slow response. As far as I can tell, the flag you mentioned does not exist anymore but I presume (please correct me if wrong!) that --isolatedModules is either the same flag renamed, or the closest surviving option in terms of intended behaviour.

I tried two things.

  1. Adjusting our tsconfig.json to set this attribute to true.
  2. Passing it directly to Bazel's tsc.sh script as a command line arg (using the args attribute of ts_project rule).

Unfortunately, neither worked. Though, we have successfully sidestepped this issue compiling using the swc rule from aspect_rules_swc(after auto-converting our tsconfig.json file an .swcrc config).

@jbedard
Copy link
Member

jbedard commented May 13, 2024

That flag is new and will be in the next typescript release I believe. I mention it because it disallows code that normally causes the issue you're having, IIUC.

SWC only transpiles to .js, the type-checking or dts transpiling wouldn't be effected. So either something else changed to fix it for you, or nothing is causing tsc to run at all.

@cjjob
Copy link
Author

cjjob commented May 19, 2024

Hmm, I'm pretty certain that no other code changes took place, but, regrettably, don't have the bandwidth to dig further :(

I will close this as I've been unable to demonstrate to you that there is a genuine issue (ideally would have provided you with a minimal case to reproduce the issue). Thank you for time and comments!

@cjjob cjjob closed this as completed May 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation more info needed More info needed to proceed further
Projects
None yet
Development

No branches or pull requests

2 participants