[Bug]: ts_config() deps trigger type checking

[walkerburgin]

What happened?

I noticed that adding an in-repo dependency to ts_config() triggers type checking to run pre-maturely in dependencies as part of the TsValidateOptions action. Are types/transitive types actually necessary for validation?

More details and a repro here.

Version

Development (host) and target OS/architectures:

Output of bazel --version: aspect pro 5.11.0-alpha2.dev.41.gd94c177

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

bazel_dep(name = "aspect_bazel_lib", version = "2.9.4")
bazel_dep(name = "rules_multitool", version = "1.0.0")
bazel_dep(name = "aspect_rules_js", version = "2.1.1")
bazel_dep(name = "aspect_rules_ts", version = "3.3.1")
bazel_dep(name = "aspect_rules_swc", version = "2.0.1")

Language(s) and/or frameworks involved: TypeScript

How to reproduce

See https://github.com/walkerburgin/tsconfig-package

Any other information?

No response

[jbedard]

I think the issue is your ts_config(deps) contains :node_modules which cascades to include the full packages (types included). I'm not sure if including the full package was just an oversight and we never noticed because npm_package previously forced the types anyway, or maybe it is more intentional because a tsconfig that references a package can depend on types for more then just type-checking?

If you change the ts_config(deps) to only the required :node_modules/@monorepo/tsconfig I think it resolves the problem?

[walkerburgin]

Narrowing the dependency definitely lessens the impact (I was being lazy and over-declaring the dependency), but it still seems wrong to me. I'm struggling to think of why a dependency in this context actually needs the types 🤔

[jbedard]

For the validation action... maybe we don't need types? We essentially just need to be able to resolve extends: "my-package".

Then we need types from ts_config(deps) only when doing type-checking because compilerOptions.types might have packages that need the dts files, but that should be handled already... 🤔

So I think you're correct?

[jbedard]

Actually... I think this is related to composite. For composite projects the tsconfig file of all dependencies is required as inputs, removing the use of composite in your project seems to fix the issue? I'm not sure if there is still a bug there or of that is valid?

[walkerburgin]

Hrmm, I'm not sure I follow. We put project references in tsconfig.json so that type checking in the IDE will work out of the box (I updated the repo to reflect that better). That requires setting "composite": true, otherwise you get an error like this:

pkgs/foo-app/tsconfig.json(5,5): error TS6306: Referenced project '/private/var/tmp/_bazel_wburgin/87f483b03fef6537ca802a6b9e195262/sandbox/darwin-sandbox/42/execroot/_main/bazel-out/darwin_arm64-fastbuild/bin/pkgs/foo-lib' must have setting "composite": true.

I'm not sure I fully understand how the implementation of ts_project() and ts_config() work, but the thing that feels off to me is that the validation action triggers .d.ts generation on any package listed in the tsconfig target's dependencies.

[jbedard]

the thing that feels off to me is that the validation action triggers .d.ts generation on any package listed in the tsconfig target's dependencies.

Yeah I agree that feels off and might still be a bug, but I think that bug only occurs when ts_project(composite = True). See the extra deps when composite is used. Basically the tsconfig files from all the referenced composite projects are required, not only the tsconfig files directly passed to tsc. I still don't think that should cause this bug, but removing composite does stop the issue you've brought up here.

[jbedard]

Sort of related, but this one fixes an issue with tsconfig deps propagating where they shouldn't (but doesn't solve this composite issue): #755

[walkerburgin]

This is almost related in that it's a slightly more legitimate reason to have ts_config() deps: #757