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 · 6 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
@klandergren
Copy link

@cjjob FYI I had a similar issue where invocations of tsc were resulting in a different set of errors compared to the ts_project's *_typecheck target and was able to resolve it by ensuring that my ts_project’s deps parameter had the exact same values as would a manual invocation of tsc in the workspace.

This advice from @jbedard pointed me in the right direction (thank you!!):

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).

To explain my situation, and its resolution, further:

Let’s imagine a BUILD.bazel containing:

ts_project(
    name = "src",
    srcs = glob(
        include = SRC_PATTERNS,
    ),
    deps = [
        "//:node_modules/foo",
    ],
    transpiler = TRANSPILER,
    tsconfig = "//:tsconfig",
    resolve_json_module = True,
    declaration = True,
    visibility = ["//:__subpackages__"],
)

The ts_project documentation for deps says:

List of targets that produce TypeScript typings (.d.ts files)

If this list contains linked npm packages, npm package store targets or other
targets that provide JsInfo, NpmPackageStoreInfo providers are gathered
from JsInfo. This is done directly from the npm_package_store_deps field
of these. For linked npm package targets, the underlying npm_package_store
target(s) that back the links is used. Gathered NpmPackageStoreInfo
providers are propagated to the direct dependencies of downstream linked
npm_package targets.

NB: Linked npm package targets that are "dev" dependencies do not forward
their underlying npm_package_store target(s) through
npm_package_store_deps and will therefore not be propagated to the direct
dependencies of downstream linked npm_package targets. npm packages that
come in from npm_translate_lock are considered "dev" dependencies if they
are have dev: true set in the pnpm lock file. This should be all packages
that are only listed as "devDependencies" in all package.json files within
the pnpm workspace. This behavior is intentional to mimic how
devDependencies work in published npm packages.

While I don’t fully understand the latter part about dev dependencies yet, in our case what is relevant is that the ts_project declaration says that the bazel target src relies on the target //:node_modules/foo to create the TypeScript typings. In your case this is whatever provides the Bot type.

Let’s also imagine the package.json looks something like this:

{
	"name": "MyLib",
	"version": "0.1.0",
	"private": true,
	"dependencies": {
		"foo": "0.0.1"
	},
	"devDependencies": {
		"@types/foo": "0.0.1"
	},
	"type": "module"
}

Note the devDependencies specification of @types/foo, which is a separate library from foo.

And finally a tsconfig.json that looks like:

{
	"compilerOptions": {
		"target": "es2020",
		"lib": ["dom", "dom.iterable", "esnext"],
		"noImplicitAny": false,
		"skipLibCheck": true,
		"esModuleInterop": true,
		"allowSyntheticDefaultImports": true,
		"strict": true,
		"forceConsistentCasingInFileNames": true,
		"noFallthroughCasesInSwitch": true,
		"module": "ES2020",
		"moduleResolution": "node",
		"resolveJsonModule": true,
		"isolatedModules": true,
		"declaration": true,
		"jsx": "react-jsx",
		"baseUrl": ".",
		"noUncheckedIndexedAccess": true
	},
	"include": ["src"]
}

In this case, assuming dependencies have been installed locally using pnpm (or whatever package manager) and have a ./node_modules directory:

  • manual invocation of tsc will resolve modules using baseUrl and look at the workspace node_modules which contains both foo and @types/foo
  • the ts_project bazel invocation will be sandboxed and its node_modules will only contain what is specified in deps (e.g. foo)

In the absence of the richer typings from the other dependencies the bazel-invoked tsc will attempt to infer types and possibly produce errors like the ones you encountered.

The fix is to modify the ts_project invocation to be:

ts_project(
    name = "src",
    srcs = glob(
        include = SRC_PATTERNS,
    ),
    deps = [
        "//:node_modules/@types/foo",
    ],
    transpiler = TRANSPILER,
    tsconfig = "//:tsconfig",
    resolve_json_module = True,
    declaration = True,
    visibility = ["//:__subpackages__"],
)

So that tsc does not infer any types and uses whatever @types/foo exposes.

(And if we wanted I think we could still include "//:node_modules/foo" so that it would be forwarded to any downstream libraries but I am not knowledgeable enough yet to know whether this is preferred over expecting downstream libraries to specify their runtime dependencies directly.)

Additionally:

  • I don’t know yet if there is an automated way to ensure that these dependencies stay in sync but it is where I check first when I encounter issues
  • I found the tsc flag --showConfig to be helpful in verifying that all the srcs were correct (this could also be the source of your issue, like a .d.ts file not being included).

Good luck!

@klandergren
Copy link

Also, using --traceResolution per troubleshooting instructions is verbose but definitive!

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

3 participants