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(yarn) detect yarn berry when running bin #1388

Merged
merged 3 commits into from
Jan 10, 2023

Conversation

xg-wang
Copy link
Contributor

@xg-wang xg-wang commented Nov 24, 2022

Summary

In Yarn 3, binaries should be run via yarn run <bin-name> instead of the assumption of node_modules/.bin/<bin-name>. This is required not only with pnp mode.

Note

I chose to detect .yarnrc.yml because this is how yarn uses yarnPath setup; this may not be required when using corepack, but volta does not support corepack yet.

Ref yarnpkg.com:

Starting from the v2, they must be written in valid Yaml and have the right extension (simply calling your file .yarnrc won't do).

I am very new to Rust, please correct me if there are things that are wrong or can be improved.

Test Plan

Built dev volta and verified it works on one of my yarn berry repo.

Note: cargo test --all --features mock-network seems to be failing because I don't have mockito locally

In Yarn 3, binaries should be run via `yarn run <bin-name>` instead of
the assumption of `node_modules/.bin/<bin-name>`. This is required not
only with pnp mode.
Copy link
Contributor

@charlespierce charlespierce left a comment

Choose a reason for hiding this comment

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

A small naming suggestion, but otherwise this look good, thank you!

crates/volta-core/src/project/mod.rs Outdated Show resolved Hide resolved
@charlespierce
Copy link
Contributor

Oh boo, sorry about that! It looks like GitHub requires us to manually approve the CI on follow-up pushes. And there's a small formatting issue failing the format check CI. Can you run cargo fmt to resolve the formatting?

@xg-wang
Copy link
Contributor Author

xg-wang commented Dec 27, 2022

@charlespierce oops sorry I missed the message. I pushed the fmt commit.

@charlespierce charlespierce merged commit 4aa49ff into volta-cli:main Jan 10, 2023
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