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

fix: always declare ts_project_rule as primary ts_project target #710

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

jbedard
Copy link
Member

@jbedard jbedard commented Oct 7, 2024

This makes the ts_project macro produce more consistent underlying targets. The {name} target will now be a ts_project_rule 100% of the time. The only conditional logic is the the same as today: the addition of {name}_types, {name}_typecheck, {name}_typecheck_test.

We can maybe think of adding those 3 extra targets 100% of the time in the future (or maybe only doing {name}_types 100% of the time and adding the test only when {name} will not run tsc?).


Changes are visible to end-users: no

Test plan

  • Covered by existing test cases

Copy link

aspect-workflows bot commented Oct 7, 2024

Test

1 test target passed

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

Total test execution time was 28ms. 135 tests (99.3%) were fully cached saving 10s.


Buildifier      Format

ts/private/ts_lib.bzl Outdated Show resolved Hide resolved
ts/private/ts_project.bzl Outdated Show resolved Hide resolved
@jbedard jbedard requested a review from dzbarsky October 7, 2024 22:57
@jbedard jbedard marked this pull request as ready for review October 7, 2024 23:11
Copy link
Collaborator

@dzbarsky dzbarsky left a comment

Choose a reason for hiding this comment

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

nice, this looks simpler and makes our life better as well. We're trying this internally, will let you know

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.

2 participants