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

Yarn 2 (berry) support #52

Open
JanMatas opened this issue Nov 13, 2020 · 13 comments · Fixed by #53
Open

Yarn 2 (berry) support #52

JanMatas opened this issue Nov 13, 2020 · 13 comments · Fixed by #53

Comments

@JanMatas
Copy link

JanMatas commented Nov 13, 2020

Hi,

thanks for the awesome package. We have been using it for a couple of months, but since we migrated to yarn berry, we are having issues since --frozen-lockfile flag was deprecated in favour of --immutable. Hence, the hook no longer works.

Would it be possible to detect whether yarn classic or yarn 2 is used?

@devinrhode2
Copy link

yarn 2 could be easily detected by just checking for the presence of some yarn 2 files and/or directories. Maybe also yarn -v would work too.

@frontsideair
Copy link
Owner

I think checking files would be faster and more consistent with the existing infra. I googled around a bit and found this blog listing generated files. I'd rather check a single file so which one would you think works the best?

I'm also thinking about the possibility making these flags configurable in a future version.

@devinrhode2
Copy link

Checking yarn -v without a doubt is the most robust way
If we don't want to do that, next best thing is to ask:
"How does yarn cli determine if a project is yarn 2?"

develop ➤ yarn -v
2.3.2
develop ➤ rm -rf .yarnrc.yml
develop ➤ yarn -v
1.22.4

I think presence of .yarnrc.yml file is pretty good

Here's some thinking that led to this: https://gist.github.com/devinrhode2/97b2f3c664052946ee0fe4b9cd746251

@frontsideair
Copy link
Owner

That sounds pretty solid to me, thanks for the investigation! I think using the same way yarn uses to determine its version is the best.

I'm also still thinking about using yarn version and would like to do a simple benchmark before ruling it out as slow.

@frontsideair
Copy link
Owner

I'm experimenting with yarn 2 and it seems like none of the flags work anymore. I think we'll only pass --immutable flag in the end.

@frontsideair
Copy link
Owner

@devinrhode2 @JanMatas would you like to take a look? I'll merge and cut an alpha release if it seems okay.

@JanMatas
Copy link
Author

Hi, thanks for addressing this so fast! It certainly works for us, but I am a bit worried about deciding based on .yarnrc.yml since it seems like the file might exist with classic yarn as well (https://classic.yarnpkg.com/en/docs/yarnrc/). However, I have never seen it in practice so far, so maybe it is ok to live with that.

Alternatively, making the flags configurable and just mentioning the recommended setup in the readme (--immutable) would solve our issue and not introduce a possibly backwards incompatible change.

@JanMatas
Copy link
Author

Actually, I apologize for the previous comment - it seems like .yml extension is indeed only present with yarn2 (based on the blog you linked). In that case, I am very happy about this PR #53 👍

@frontsideair
Copy link
Owner

Merging and cutting a beta release then, thanks for review!

@frontsideair
Copy link
Owner

Reopened the issue until the fix is confirmed. Also published an old version accidentally and had to publish again, so it's not alpha. @JanMatas can you try 0.5.1 and let me know if it works? Thanks!

@JanMatas
Copy link
Author

Hi, just played around and I am afraid it does not work. yarnhook now uses .yarnrc.yml not only to determine which package manager is used but for yarn2, it also uses it as lockfile. However, in yarn2, the lockfile is still yarn.lock. So now the git diff command incorrectly compares .yarnrc.yml instead of yarn.lock. I think ideally a list of file to be checked could be configured - so both .yarnrc.yml and yarn.lock are compared. However, yarn.lock should suffice for vast majority of use-cases

@frontsideair
Copy link
Owner

Thanks so much for reporting, I have proper testing in a branch but I didn't have time to merge and release a major version so your feedback is very important.

I see that I have mixed up the file to check to determine the package manager and the lockfile. I have a PR almost ready and will have a pre-release soon.

@frontsideair
Copy link
Owner

I published a prerelease, please install with the tag yarnhook@prerelease and let me know if it works. Thanks again!

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 a pull request may close this issue.

3 participants