build: update bazel typescript rules #13408
Conversation
devversion
added
pr: merge safe
target: major
googlebot
added
the
cla: yes
WORKSPACE
Outdated
| # `rules_typescript_dependencies()` would also load the NodeJS rules, but we specifically need | ||
| # at least v0.14.1 which includes: https://github.com/bazelbuild/rules_nodejs/pull/341 | ||
| http_archive( | ||
| name = "build_bazel_rules_nodejs", |
There was a problem hiding this comment.
tools/BUILD.bazel uses the nodejs_binary rule for the sass bundling custom rule, so I think we need to keep this.
There was a problem hiding this comment.
Yeah, but rules_typescript also transitively loads the according rules_nodejs archive which is required for the TypeScript rules.
I'm not sure what's better. This is consistent with what's going on in angular/angular.
There was a problem hiding this comment.
It's always better to explicitly depend on the things you use directly; in general depending on transitive dependencies is an anti-pattern.
There was a problem hiding this comment.
cc @alexeagle in case he has a comment on this
There was a problem hiding this comment.
I agree with that, just not sure how it would work the best with the TypeScript rules.
Based on my understanding, if we explicitly specify a version for rules_nodejs, we'll also overwrite the rules_nodejs version that comes with the TypeScript rules. This kind of feels like it could cause unexpected behavior if we just update one of the dependencies, but since we always do the updates manually, it shouldn't be a problem.
There was a problem hiding this comment.
you can supply an explicit rules_nodejs dependency, if you choose something too old, rules_typescript's ts_setup_workspace will throw an error
There was a problem hiding this comment.
@jelbourn Done. @alexeagle Cool, if it throws, that's perfect. Any reason why angular/angular uses the transitive dependency?
* Updates the Bazel TypeScript rules. * Fixes a test case which doesn't match the name of the upgrade data it runs with (`css-names` --> `css-selectors`) * Removes Bazel version check (the NodeJS rules automatically check the Bazel version based on the version of the NodeJS rules)
devversion
force-pushed
the
build/update-typescript-rules
branch
from
4b0a42c to
50db44f
Compare
* Updates the Bazel TypeScript rules. * Fixes a test case which doesn't match the name of the upgrade data it runs with (`css-names` --> `css-selectors`) * Removes Bazel version check (the NodeJS rules automatically check the Bazel version based on the version of the NodeJS rules)
|
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
css-names-->css-selectors)