[Bug]: `transitive_typecheck` doesn't work across NPM package boundaries

[walkerburgin]

What happened?

I'm in the process of migrating to --isolatedDeclarations and isolated_typecheck = True.

I naively assumed that building the transitive_typecheck target would trigger type checking across all transitive ts_project() dependencies. At the moment it doesn't seem to work across NPM package / js_library() boundaries.

Version

Development (host) and target OS/architectures:

Output of bazel --version: bazel 8.5.0

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

bazel_dep(name = "aspect_rules_js", version = "2.8.1")
bazel_dep(name = "aspect_rules_ts", version = "3.7.1")

Language(s) and/or frameworks involved: TypeScript

How to reproduce

• Clone https://github.com/walkerburgin/transitive-typecheck-repro
• Run bazel build //packages/foo-app:foo-app_transitive_typecheck and observe that the build does not fail, even though there's a type error in //packages/foo-lib

Any other information?

No response

[jbedard]

I think it will only work across ts_project(deps) and not across :node_modules/ links.

It may work when gathering those :node_modules/ targets if building the npm package forces type-checking (such as npm packages requiring .d.ts which get generated by the same action that does type-checking).

Does that align with what you're observing?

[jbedard]

As for a solution... I'm not sure.

To force type-checking across npm package boundaries the npm package targets would have to carry along all the typecheck targets which I think is not ideal and wouldn't want to do.

Really the tests in //packages/foo-lib should catch this. Does that not work with your workflow?

[walkerburgin]

I think it will only work across ts_project(deps) and not across :node_modules/ links.

That's consistent with what I've seen. In that repro repository, if I manually re-write the deps to point directly to the other ts_project() then transitive_typecheck does what I was expecting. As soon as there's a js_library() involved it stops workin because the output group isn't propagated.

Really the tests in //packages/foo-lib should catch this. Does that not work with your workflow?

Yeah, the tests in //packages/foo-lib will fail on CI and prevent something like this from merging. The thing that prompted this is developers wanting to type check all of the transitive dependencies of an app locally during development, e.g. while refactoring (even if it would be faster to let CI do it). I can give them a bazel query, but noticed the transitive_typecheck target and was surprised by how it behaved. OOC, what's the use case for it if not this?

As for a solution... I'm not sure.

I guess the type check status files could be added to JsInfo provider and propagated, but I understand that's a big change.

[jbedard]

I guess the type check status files could be added to JsInfo provider and propagated, but I understand that's a big change.

I think that's more then I'd like to do. I feel like that will just become a never ending chain of "you must pass output_group X along".

I can give them a bazel query, but noticed the transitive_typecheck target and was surprised by how it behaved. OOC, what's the use case for it if not this?

I assume it was added along with ts_project(transpiler) to add a way to force tsc to run.

I think validation_typecheck would be a better way to do this, and maybe the output groups should be deprecated in favour of this?

I've seen things like bazelrc entries to opt-in to this with --config=typecheck:

common:typecheck --@aspect_rules_ts//ts:validation_typecheck