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

feat: support custom dts transpiler #697

Merged
merged 1 commit into from
Sep 27, 2024

Conversation

jbedard
Copy link
Member

@jbedard jbedard commented Sep 19, 2024

Support a custom transpiler for producing dts files.

Overview of targets created:

  • {name} outputs a JsInfo containing all transpiled files, no guarantee if this runs tsc, no guarantee it outputs anything such as no_emit = True

If any transpiler or no_emit is used then the following are also created:

  • {name}_types if declaration: True && not no_emit: dts files in DefaultInfo
  • {name}_tsc invokes tsc, maybe outputting something, maybe doing type-checking, maybe both
  • {name}_typecheck outputs something when type-checking passes
  • {name}_typecheck_test forces {name}_typecheck to build and therefor forces type-checking

Changes are visible to end-users: yes

  • Searched for relevant documentation and updated as needed: yes
  • Breaking change (forces users to change their own code or config): no
  • Suggested release notes appear below: yes

A custom dts transpiler can now be set using ts_project(declaration_transpiler). This aligns dts transpiling with js transpiling allowing fast transpiling of all files, only requiring invoking tsc for a type-checking action.

Test plan

  • Covered by existing test cases
  • New test cases added

Copy link

aspect-workflows bot commented Sep 19, 2024

Test

1 test target passed

Targets
//docs:update_0_test [k8-fastbuild] 25ms

Total test execution time was 25ms. 103 tests (99.0%) were fully cached saving 9s.


Buildifier      Format

@jbedard jbedard force-pushed the tsc-transpiler branch 2 times, most recently from a632daa to acc56ce Compare September 19, 2024 02:08
ts/defs.bzl Show resolved Hide resolved
@jbedard
Copy link
Member Author

jbedard commented Sep 19, 2024

TODO: tests, I think swc isn't ready (swc-project/swc#9512) so we might have to try something else, even just a js_binary hiding plain old tsc

Copy link
Contributor

@MichaelMitchell-at MichaelMitchell-at left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tested this out on our repo and I ran into some issues. I think if my comments are addressed we'll be good though!

ts/defs.bzl Outdated Show resolved Hide resolved
ts/private/ts_project.bzl Outdated Show resolved Hide resolved
ts/private/ts_project.bzl Show resolved Hide resolved
@jbedard jbedard force-pushed the tsc-transpiler branch 6 times, most recently from c618b8c to 11b0e5f Compare September 24, 2024 01:57
@jbedard jbedard marked this pull request as ready for review September 24, 2024 02:57
Copy link
Contributor

@MichaelMitchell-at MichaelMitchell-at left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested it out on our code base. Works great!

ts/private/ts_lib.bzl Outdated Show resolved Hide resolved
ts/defs.bzl Outdated Show resolved Hide resolved
ts/defs.bzl Outdated Show resolved Hide resolved
ts/defs.bzl Outdated Show resolved Hide resolved
@jbedard
Copy link
Member Author

jbedard commented Sep 26, 2024

Updated. See the outline of targets I put into the PR description.

IMO the {name}_types target should always be created, even if we are using tsc for everything (js + dts) this {name}_types target extracts the dts files into DefaultInfo. I don't see any reason why that should only be set when using a transpiler?

docs/transpiler.md Outdated Show resolved Hide resolved
@jbedard jbedard requested a review from gregmagolan September 26, 2024 08:30
@jbedard jbedard force-pushed the tsc-transpiler branch 2 times, most recently from fcf31d6 to e62bfb9 Compare September 26, 2024 08:49
ts/defs.bzl Outdated Show resolved Hide resolved
@jbedard jbedard force-pushed the tsc-transpiler branch 4 times, most recently from 1bc27d8 to 89b66fe Compare September 27, 2024 17:26
docs/transpiler.md Outdated Show resolved Hide resolved
@jbedard jbedard merged commit dd832db into aspect-build:main Sep 27, 2024
23 checks passed
@jbedard jbedard deleted the tsc-transpiler branch September 27, 2024 21:16
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

Successfully merging this pull request may close these issues.

3 participants