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

ts_project does not work with labels in srcs #2301

Closed
fischor opened this issue Nov 23, 2020 · 8 comments
Closed

ts_project does not work with labels in srcs #2301

fischor opened this issue Nov 23, 2020 · 8 comments

Comments

@fischor
Copy link

fischor commented Nov 23, 2020

🐞 bug report

Affected Rule

ts_project

Is this a regression?

No

Description

I have a rule that generates .ts files. I want ts_project to compile these .ts files. But it wont recognize them at all.

# BUILD.bazel
generate_ts_code_from_proto(
   name = "generated",
   proto = ":my_prot_target",
)
 
ts_project(
   name = "project",
   srcs = [":generated"],
   ts_config = "//:tsconfig.json",
   deps = ["..."]
)

Anything else relevant?

https://github.com/bazelbuild/rules_nodejs/blob/681c6683dac742f1e375a401c0399ec7783ac8fd/packages/typescript/internal/ts_project.bzl#L314-L320

Above is the code that is used to define the outputs for the ts_project rule in the ts_proto_macro.
In my case, this will just result in an empty list and thus nothing will build.

@fischor
Copy link
Author

fischor commented Nov 23, 2020

Also, I think it would be better if the rule/macro would fail if it encounters an input in srcs that it can not process instead of silently ignoring it. That would make debugging of your build easier.

@alexeagle
Copy link
Collaborator

This is a limitation of ts_project. One design principle is to predeclare all the outputs to Bazel (see https://docs.bazel.build/versions/master/skylark/rules.html#outputs ) so that you can rely on //my:generated.js and //my:generated.d.ts labels being provided. In order to do that, we need to know what all the source files are.

Even a simple example fails, doesn't need to involve code generation:

load("//packages/typescript:index.bzl", "ts_project")

filegroup(
    name = "srcs",
    srcs = ["a.ts"],
)

# Repro for https://github.com/bazelbuild/rules_nodejs/issues/2301
ts_project(
    srcs = [":srcs"],
)

->

% bazel query --output=build packages/typescript/test/ts_project/no_srcs:tsconfig
# /Users/alex.eagle/Projects/rules_nodejs/packages/typescript/test/ts_project/no_srcs/BUILD.bazel:10:1
ts_project(
  name = "tsconfig",
  generator_name = "tsconfig",
  generator_function = "ts_project_macro",
  generator_location = "packages/typescript/test/ts_project/no_srcs/BUILD.bazel:10:1",
  args = [],
  deps = ["//packages/typescript/test/ts_project/no_srcs:_validate_tsconfig_options"],
  link_workspace_root = False,
  srcs = ["//packages/typescript/test/ts_project/no_srcs:srcs"],
  supports_workers = select({"@bazel_tools//src/conditions:host_windows": False, "//conditions:default": False}),
  tsconfig = "//packages/typescript/test/ts_project/no_srcs:tsconfig.json",
  js_outs = [],
  map_outs = [],
  typing_maps_outs = [],
  typings_outs = [],
)

The indirection through filegroup means we can't calculate labels for our outputs.

I like your idea of improving the error messaging. However there's also a feature of ts_project which is that it accepts arbitrary inputs, this lets you do things like pass a foo.svg file as an input, because the compiler will want to read that at compile time. I think the best we can do is error if the ts_project is configured to produce zero outputs.

@fischor
Copy link
Author

fischor commented Nov 30, 2020

Already thought this is not really a bug but rather by design. At the moment, one is able to compile .ts files that are passed through labels using ts_library. I am wondering how to approach compiling these if ts_library is going to be deprecated?

@alexeagle
Copy link
Collaborator

If you want to take an opaque directory of .ts files and compile all of them to an opaque directory of .js files, then I think you just load("@npm//typescript:index.bzl", "tsc") and use vanilla tsc with output_dir=True.
should document that in https://bazelbuild.github.io/rules_nodejs/TypeScript.html

alexeagle pushed a commit to alexeagle/rules_nodejs that referenced this issue Dec 1, 2020
Also fix a violation of this constraint in our tests.
That in turn exposed a subtle bug where the `files` block in the generated tsconfig.json would reference non-.ts inputs

BREAKING CHANGE:
any ts_project rule that produces no outputs must be fixed or removed

Fixes bazel-contrib#2301
alexeagle pushed a commit that referenced this issue Dec 1, 2020
Also fix a violation of this constraint in our tests.
That in turn exposed a subtle bug where the `files` block in the generated tsconfig.json would reference non-.ts inputs

BREAKING CHANGE:
any ts_project rule that produces no outputs must be fixed or removed

Fixes #2301
@alexeagle
Copy link
Collaborator

fixed by 3ca6cac

@reddaly
Copy link

reddaly commented Oct 11, 2022

I'm not sure if this is the issue I'm running into or not, but it seems my ts_project rule doesn't work propertly when one of the srcs files is the output of a genrule. In my case, all the input files are listed explicitly - there is no glob or anything.

ts_project(
    name = "client",
    srcs = [
        "generated.ts",
    ],
    deps = [
        "//frontend/utils",
        "@npm//google-protobuf",
    ],
)

genrule(
    name = "codegen",
    outs = [
        "generated.ts",
    ],
    cmd = ("$(location //blah:generate_typescript) " +
           "--output_dir $(RULEDIR)"),
    exec_tools = ["//blah:generate_typescript"],
)

Is the above expected to fail? If so, it's quite surprising. In most bazel rules, using an explicit src file and a generated file works the same way.

@gregmagolan
Copy link
Collaborator

gregmagolan commented Oct 11, 2022

With rules_nodejs ts_project this can be an issue because typescript isn't aware of the output tree (where the generated ts file lives) unless you add the appropriate rootDirs to your tsconfig. You also can't mixed generated sources with actual sources in ts_project from rules_nodejs. At its core, the issue is that Bazel puts outputs in a separate tree while typescript is used to node ecosystem tooling which typically puts generated files in the same tree as sources but .gitignored.

These issues have both been resolved in ts_project based on rules_js.

@reddaly
Copy link

reddaly commented Oct 11, 2022 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants