[Bug]: isolated_typecheck & project references

[walkerburgin]

What happened?

I'm having a hard time getting the new isolated_typecheck to play nicely with TypeScript's project references.

A naive setup (repro here) without explicit ts_config() targets fails during .d.ts transpilation because it can't find the tsconfig.json files of dependency projects:

Adding explicit ts_config() targets with deps on in-repo dependencies (repro on this branch here) seems to fix the issue, but I can't get bazel configure to auto-generate the dependencies without failing like this:

I guess that's probably a separate issue for https://github.com/aspect-build/aspect-cli.

It's unintuitive that setting isolated_typecheck = True triggers the need for explicit ts_config() targets, almost as an accident (I think) of using tsc to do the .d.ts transpilation.

Version

Development (host) and target OS/architectures:

Output of bazel --version: aspect 2024.51.11+60ad64d6e

Version of the Aspect rules, or other relevant rules from your
WORKSPACE or MODULE.bazel file:

bazel_dep(name = "aspect_bazel_lib", version = "2.10.0")
bazel_dep(name = "rules_multitool", version = "1.0.0")
bazel_dep(name = "aspect_rules_js", version = "2.1.2")
bazel_dep(name = "aspect_rules_ts", version = "3.3.2")
bazel_dep(name = "aspect_rules_swc", version = "2.2.0")

Language(s) and/or frameworks involved: TypeScript

How to reproduce

See: https://github.com/walkerburgin/isolated-typecheck-project-references-repro

Any other information?

No response

[jbedard]

This is because the primary purpose of isolated_typecheck is to avoid blocking waiting for transitive dependencies to build. The extra deps required for composite are then no longer included when tsc is invoked which would be the cause of this bug.

This makes me think that those extra deps should not be automatically added like they are today. IMO if a tsconfig file has settings that require dependencies then ts_config(deps) should contain those deps and rules_ts shouldn't make any assumptions about those deps being somewhere else. That might also effect or fix #739

It seems like using ts_config(deps) works around this for you? If so I'd prefer working on getting bazel configure to work in that scenario instead of trying to add more magic to ts_project.

[walkerburgin]

Yeah I think it's much more legible for tsconfig file dependencies ("extends", "references", etc) to always live in ts_config(deps). Maybe a validation action could enforce that?

In this scenario it was jarring/confusing that changing isolated_typecheck = False to isolated_typecheck = True required introducing explicit ts_config() targets. I had to kind of just guess that it would help here.

[jbedard]

FWIW this should fix a lot of the confusion in the next major release: #762

[jbedard]

FWIW I think aspect configure will generate the correct ts_config(deps) if you have # gazelle:js_tsconfig enabled. Like we've agreed I think that is the correct place for these deps to live, so I don't want aspect configure to add the deps to ts_project(deps) (especially since #762 will remove that functionality).

[walkerburgin]

I might be doing something dumb, but when I add # gazelle:js_tsconfig enabled to walkerburgin/isolated-typecheck-project-references-repro and re-run bazel configure I get errors about ambiguity like this:

➜  isolated-typecheck-project-references-repro git:(develop) ✗ bazel configure
 Updating BUILD files for javascript
2025/01/16 17:41:52 Resolution Error: Import "pkgs/foo-lib" from "pkgs/foo-app/tsconfig.json" resolved to multiple targets (//pkgs/foo-lib:pkg, //pkgs/foo-lib:tsconfig) - this must be fixed using the "aspect:resolve" directive