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
105 stars 59 forks source link

ts_project doesn't seem to support JS sources #73

Open paullewis opened 2 years ago

paullewis commented 2 years ago

If I use a JS source and a custom transpiler with ts_project like this:

load("@aspect_rules_swc//swc:defs.bzl", "swc_transpiler")
load("@aspect_rules_ts//ts:defs.bzl", "ts_project")
ts_project(
    name = "src",
    srcs = [
        "index.js",
    ],
    allow_js = True,
    declaration = True,
    transpiler = swc_transpiler,
    tsconfig = "//:tsconfig",
)

The JS is excluded and the length of js_outs doesn't match the length of srcs resulting in an error:

Error in fail: ERROR: illegal state: transpile_srcs has length 1 but js_outs has length 0

If instead I use the default tsc then it's unhappy because it would overwrite the copied source file:

error TS5055: Cannot write file 'src/index.js' because it would overwrite input file.

If I then set out an out_dir to something like "foo" it throws an error saying that:

output 'src/foo/index.d.ts' was not created

From the above it's not clear how I should be using ts_project with JS source files. Am I holding it wrong?

Thanks!

thesayyn commented 2 years ago

the first case is expected and is a result of how rules_js works. You can not have inputs & outputs that have the same name. you see that error because ts_project + transpiler fails before bazel itself.

the second one is expected to happen and can only be avoided using the out_dir attribute.

the third case ought to work but looks like something is broken there.

alexeagle commented 2 years ago

For case 1, I'd expect you to get this error: https://github.com/aspect-build/rules_swc/commit/245f582ee0abab5aacf6c32ed18ed5578d6d05ba#diff-1444361579758fc6be4458b5105b2ee3ee623d72154094c3fa1fd9056845903eR75 saying that you need an out_dir to get swc to do .js->.js

paullewis commented 2 years ago

In some codebases it's not uncommon to have TS & JS alongside one another, especially during a migration from – say – JS to TS. In those cases, where there's effectively a single target with a mix of source types, I'd have expected to be able to mix JS & TS in the same way as I can with the TS compiler directly outside of Bazel. In the most general sense the TS compiler can handle JS inputs, of course, so it's unusual that it's effectively disallowed here. I appreciate that the source file is being copied to the bin dir to ease the issues that were present in rules_nodejs, though.

I guess there are a couple of questions this raises for me:

  1. Is there a correct way to mix JS & TS sources with ts_project?
  2. If the out_dir works, how would you teach other targets that use this about the location of the files? The imports will expect to find the outputs in src/ and not src/foo (where foo is the out_dir).
alexeagle commented 2 years ago

Yup, agreed about the use case, it's reasonable and important.

The most practical answer is to give different filenames somehow. For example src/foo.jsx -> bazel-bin/foo.js or src/foo.js -> bazel-bin/foo.{mjs,cjs}. Then you don't need an out_dir.

For 2 - yes the added path segment in the out_dir needs to be known to tools that resolve imports. However, it's quite common that JS developers output things to a folder like build/ or dist/ so this is possible with all tools. For TS you'd use pathMapping like this example https://github.com/aspect-build/rules_ts/blob/main/examples/root_dirs/BUILD.bazel

There's a nuance here, which is whether you expect the .js file in the output folder to be different from the one in the source folder (e.g. syntax transpiled in some way). If not, then actually the behavior of copy_to_bin for those files is fine, and we just want to avoid ts_project yelling at you about it.

gregmagolan commented 1 year ago

Taking this off of the 1.0 milestone since a documentation change can land after the 1.0.0 release

alexeagle commented 1 year ago

It's over a year old, our last comment was about a documentation change, and I don't have a current repro. Closing and will let someone report again if we still have a bug.

alexeagle commented 1 month ago

Reopen based on thread on Bazel slack

https://bazelbuild.slack.com/archives/C04281DTLH0/p1724703871282619

benmcginnis commented 1 month ago

Ok, I believe we have figured out the cause here and it is mostly caused by a misunderstanding of how to configure rules properly for ts & js interop.

It seems that to get things set up to work correctly, you need to wrap any js_librarys that you depend on that don't include their own *.d.ts files in an additional library that uses types instead of srcs,

e.g. if the examples/js_lib were modified like this:

load("@aspect_rules_js//js:defs.bzl", "js_library")
# A package only using js_library() instead of npm_package()
js_library(
    name = "pkg",
    srcs = [
        # A typescript file is required to ensure types will exist within the `JsInfo(types)`
        # that can be passed along the package linking chain. Otherwise no files will be copied
        # into the sandbox for type-checking (even though tsc would pass if only the .js was there).
        "index.d.ts",
        # "index.d.ts", # commenting this line out breaks any ts_project rules that depend on this package.
        "index.js",
    ],
    visibility = ["//examples:__subpackages__"],
)

js_library (
  name = "types",
  types = [":pkg"],
  visibility = ["//examples:__subpackages__"],
)

the :types target can be safely depended on, but the :pkg target cannot be.

It also seems like it doesn't work if you have both srcs and types.

alexeagle commented 1 month ago

THat does seem like a bug to me. Either https://docs.aspect.build/rulesets/aspect_rules_js/docs/js_library#types should be documented as mutually exclusive to srcs or they should both work when present on the same js_library target.