-
Notifications
You must be signed in to change notification settings - Fork 522
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
refactor: move yarn fetch to a separate external repo #3230
Conversation
06ba07f
to
04965f4
Compare
See https://github.com/bazelbuild/rules_nodejs/wiki/Migrating-to-5.0 | ||
""") | ||
# Back-compat: allow yarn_repositories args to be provided to node_repositories | ||
yarn_args = {} |
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.
based on working with the repo recently, I think it's too soon to deprecate the build_bazel_rules_nodejs repositories helper. Too spammy to log here, and too much work for users, with no benefit in the vast majority case where they don't care about toolchains
ceb6ab9
to
846d351
Compare
Previously every @node_some_platform had a yarn program in it, which doesn't make sense since yarn is platform-independent JS. Instead make a @Yarn repo and use that for getting our yarn binary. Trivially allows a vendored yarn by passing the yarn attribute to yarn_install overriding the default. BREAKING CHANGE: vendored_yarn attribute is removed
846d351
to
49cc951
Compare
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.
Haven't gone though super thoroughly, but looks the right shape. The npx
mentions is kinda nice, might be worth adding a line to doc of how to call it.
Is there somewhere in the docs that really explicitly calls out the labels? eg @yarn//:yarn
etc? I always forget those and all the many aliases.
"yarn_download_auth": attr.string_dict( | ||
default = {}, | ||
doc = """auth to use for all url requests | ||
Example: {\"type\": \"basic\", \"login\": \"<UserName>\", \"password\": \"<Password>\" } | ||
""", | ||
), |
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.
Was this here before? Seems kinda insecure
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.
yes, this block was moved without changes from the original to avoid making it more breaking than necessary
|
||
This repository is created by calling the `yarn_repositories` function in your `WORKSPACE` file. | ||
|
||
- Run the Bazel-managed version of yarn: `bazel run @yarn//:yarn` |
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.
@mattem here's what you were looking for
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.
Ah nice 👍
Previously every @node_some_platform had a yarn program in it,
which doesn't make sense since yarn is platform-independent JS.
Instead make a @Yarn repo and use that for getting our yarn binary.
Trivially allows a vendored yarn by passing the yarn attribute to
yarn_install overriding the default.
BREAKING CHANGE:
vendored_yarn attribute is removed