aspect-build / rules_ts

Bazel rules for the `tsc` compiler from http://typescriptlang.org
https://docs.aspect.build/rules/aspect_rules_ts
Apache License 2.0
106 stars 61 forks source link

ts_config rule doesn't provide its deps in DefaultInfo, causing it to be unusable in 'data' attribute of other rules #190

Open gregjacobs opened 2 years ago

gregjacobs commented 2 years ago

Hey there. So I noticed that the ts_config rule doesn't provide its own transitive deps on the DefaultInfo provider.

I'm basically trying to use a ts_config target in the data attribute of a js_binary, but only the src of the ts_config is appearing in the runfiles directory without its deps.

Thoughts on adding its transitive deps to DefaultInfo?

matthewjh commented 2 years ago

This would be a sensible addition. You can write your own rule to move the deps into DefaultInfo.files, but I agree it should be built-in.

gregmagolan commented 1 year ago

This should be fixed in rules_ts 1.3.0. ts_config now provides a JsInfo and transitive deps should flow through that. @gregjacobs Can you please confirm it fixes it for your use case?

gregmagolan commented 1 year ago

Transitive deps are generally not added to DefaultInfo. For downstream _binary & _test targets, rules generally put everything needed at runtime in the DefaultInfo runfiles. I am curious what deps of a ts_config target you'd need at runtime. tsconfig.json files are typically only needed for typechecking in ts_project targets and not at runtime. What does your use case look like?

gregjacobs commented 1 year ago

Hey @gregmagolan, apologies for the delayed response - looks like I missed some email notifications.

So here's my use case: We make a lot of use of the TypeScript compiler APIs on our codebase to derive information from our source files (such as documentation of classes/functions/types), or perform code modifications to our existing files and write out new files. We do this via Node.js scripts executed via js_run_binary.

Via some layers of abstraction provided by the project ts-morph (highly recommend checking this out), we essentially need to instantiate a TypeScript project instance from its Node API, and all of the TypeScript settings for this 'project' instance are derived from reading a tsconfig.json file. Looks something like this:

import { Project } from 'ts-morph';

const project = new Project({
    tsConfigFilePath: 'src/tsconfig.json'
});

However, our src/tsconfig.json file "extends" 3 other tsconfig.json files in other directories. In order to provide all of those tsconfig.json files to the sandbox, I now need to write srcs attributes for js_run_binary as:

srcs = [
    # ... other dependencies

    ":tsconfig_src",   # the tsconfig.json file in the package's 'src/' directory (what will be read by ts-morph)
    ":tsconfig",       # the tsconfig.json file in the package's root directory (extension #1)
    "//:tsconfig_web", # the tsconfig_web.json file in the root of our repo with "web" settings (extension #2)
    "//:tsconfig",     # the tsconfig.json file in the root of our repo with "shared" settings (extension #3)
]

What I would like is to just include ":tsconfig_src" in js_run_binary's srcs and have all of the "extends" tsconfig.json files end up in the bazel sandbox tree. If this is best done by using runfiles, that's totally okay with me. I'm still learning the difference between DefaultInfo's 'files' and 'runfiles' :)

gregjacobs commented 1 year ago

Btw, the :tsconfig targets are defined with deps themselves, something like:

ts_config(
    name = "tsconfig_src",
    src = "src/tsconfig.json",
    deps = [":tsconfig"],
)

ts_config(
    name = "tsconfig",
    src = "tsconfig.json",
    deps = ["//:tsconfig_web"]
)
gregmagolan commented 1 year ago

I see. This sounds like you want to add the runfiles of the ts_config target and not the file of the DefaultInfo. In general files is the things that the particular target builds/produces, which in this case is the src tsconfig, and runfiles are all things that are needed to go along with what is produced at runtime if the target is unused in a downstream *_binary or *_test rule.

gregmagolan commented 1 year ago

Looking at the code some more, the ts_config target does now also provide a JsInfo which differentiates between sources and transitives_sources https://github.com/aspect-build/rules_ts/blob/1a07af0665cff5916ef70a2879a75929cd8201ee/ts/private/ts_config.bzl#L78. These are picked up by js_run_binary for targets in srcs. ts_config doesn't however currently pass the tsconfig.json file or transitive tsconfig.json files to sources, instead passing them to declarations since they are primarily used for typechecking and not at runtime.

I'm not sure if it is strictly better to always pass them to sources. Instead since it sounds like your js_run_binary needs them, you can set the include_declarations attribute on the js_run_binary and they should all be included in the inputs of that build action via the transitive_declarations of JsInfo.

https://docs.aspect.build/rules/aspect_rules_js/docs/js_run_binary#include_declarations

gregjacobs commented 1 year ago

Hey @gregmagolan. So I have an update for you on this one and another use case which I think may help. I think you're right in terms of the tsconfig files needing to be in runfiles.

I'm using webpack_devserver from rules_webpack along with ts-loader in order to attempt to create a fast-refreshing dev server (where type checking is done in a separate process).

In order to tell ts-loader about the tsconfig.json file and its extensions though, I need list each "extended" file individually under data, something like this:

webpack_devserver(
    name = "serve",
    webpack_config = "webpack.config.js",
    data = [
        # ... source files ...
        ":tsconfig",
        "//:tsconfig_web",   # tsconfig "extends" #1 - must be listed here
        "//:tsconfig",       # tsconfig "extends" #2 - must be listed here
)

ts_config(
    name = "tsconfig",
    deps = ["//:tsconfig_web"],
    src = "tsconfig.json",
)

Without adding the extra two tsconfig extensions under data, I'll get an error such as the following:

[tsl] ERROR
      TS5083: Cannot read file '/private/var/tmp/_bazel_<username>/ac46d63fc27ec40679bca3af349c6619/execroot/<project_name>/bazel-out/darwin-opt/bin/my-app/serve.sh.runfiles/<project_name>/tsconfig_web.json'.

This is with rules_ts@1.4.5 btw.