rules_ts icon indicating copy to clipboard operation
rules_ts copied to clipboard

Support For Typecheck-Only Validation Action

Open jacobgardner opened this issue 10 months ago • 6 comments


Type of change

  • New feature or functionality (change which adds functionality)

My attempt at fixing #88 as a validation action.

For changes visible to end-users

  • Breaking change (this change will force users to change their own code or config)

    • Requires bazel 6 or higher for validation action support.
    • Removes some warnings about typecheck only not being supported.
  • Relevant documentation has been updated

  • Suggested release notes are provided below: ts_project no longer needs to emit declaration files in order to work with a non-tsc transpiler. If no declaration emit and no transpilation takes place, tsc --noEmit is still run as a validation action.

Test plan

  • Covered by existing test cases
  • New test cases added

jacobgardner avatar Mar 26 '24 01:03 jacobgardner

@jacobgardner Thanks for working on this! I tested it out in our repo at Figma, and it works well. I did have to make an addition on top of your changes, to support an explicit no_emit attribute for ts_project, which we needed for our use case. (It's a bit niche, but we have two ts_project targets with overlapping .ts inputs, each typechecking with a different tsconfig. These targets are to be used only for typechecking and linting. Our bundler is a separate target.) My additions are here: https://github.com/jfirebaugh/rules_ts/commit/f5df77ca655b7b0fbe9d83cfe8317ac4319f0a21

jfirebaugh avatar Apr 11 '24 20:04 jfirebaugh

Sweet. I will merge your changes in soon.

jacobgardner avatar Apr 15 '24 15:04 jacobgardner

I merged the no_emit code. I want to add some tests for that later this week as well though.

jacobgardner avatar Apr 17 '24 03:04 jacobgardner

I've found another hiccup with this approach: when using it in tandem with rules_lint / aspect lint, the typechecking actions will trigger even when you're just trying to run eslint. This is because the validations output group "is special in that its outputs are always requested, regardless of the value of the --output_groups flag, and regardless of how the target is depended upon" (https://bazel.build/extending/rules#validations_output_group).

I think perhaps aspect lint should pass --run_validations=false to the underlying bazel command it runs? Or provide a hook so that we can add that in .bazelrc. For example aspect lint could run bazel build --config=lint ... and then I could put config:lint --run_validations=false in .bazelrc.

jfirebaugh avatar Apr 23 '24 23:04 jfirebaugh

Test

:warning: GitHub Actions build #446 failed.

Failed tests (1)
//docs:update_0_test [k8-fastbuild] 80ms

:bulb: To reproduce the test failures, run

bazel test //docs:update_0_test

Buildifier      Format

aspect-workflows[bot] avatar Apr 24 '24 00:04 aspect-workflows[bot]

A more serious issue with this approach is that it negates the performance optimization of using ts_project#transpiler to avoid typechecking in situations when only the transpiled outputs are necessary. The behavior of validation actions makes typechecking always eager again.

Is a validation action actually providing value in this case? It still requires a "marker" output. Could we instead just have a normal action that outputs the same marker?

jfirebaugh avatar May 02 '24 20:05 jfirebaugh

Replaced by #681 where @jbedard has picked up your commits

alexeagle avatar Sep 05 '24 18:09 alexeagle