-
Notifications
You must be signed in to change notification settings - Fork 11.9k
First iteration of rules_nodejs
cleanups
#29531
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
Conversation
Replaces usages of `rules_nodejs` `copy_to_bin` with the `rules_js`/Aspect bazel lib equivalent.
Migrates to `rules_js` and simplifies! the `ts_json_schema` rule
Migrates the CLI schema generation to `rules_js`, also significantly simplifying the rule boilerplate.
This is necessary so that we can delete the `pkg_npm` macro and fully leverage the `rules_js` variant.
This file is currently no longer necessary after migrating all consumers to their `rules_js` variants, so we can delete the file. In follow-ups we will consider renaming `defaults2.bzl` back to this file, or have a better name altogether.
This dependency does not provide any value for the `ts_project` compilation, nor does it seem to be necessary; so we are cleaning up this dependency.
Switches the beasties bundling to `rules_js`, using rollup directly from the node modules installation. Notably we are facing a small issue that doesn't cause any issues right now, because rollup tries to dereference symlinks by default given a bug: aspect-build/rules_js#1827. This means we can't rely on the jailed resolution, but in practice it shouldn't cause an issue at this point.
@@ -1,5 +1,5 @@ | |||
load("@aspect_rules_js//js:defs.bzl", "js_library") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice!!!
@@ -1,5 +1,9 @@ | |||
load("@aspect_bazel_lib//lib:copy_to_bin.bzl", _copy_to_bin = "copy_to_bin") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
NIT: as a followup we can probably rename this now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
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. |
See individual commits