Skip to content

Do i need to commit .yarn/releases folder to my repo? #7741

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

Closed
nick-smit opened this issue Dec 6, 2019 · 11 comments
Closed

Do i need to commit .yarn/releases folder to my repo? #7741

nick-smit opened this issue Dec 6, 2019 · 11 comments

Comments

@nick-smit
Copy link

Do you want to request a feature or report a bug?
Feature (i think)
I believe i need to commit the full .yarn/releases folder into the repository for others to use. This means that a full build of yarn is added to my repo. It would be much more usable if the required release is automatically downloaded if it doesn't exist.

What is the current behavior?
Yarn crashes if the required version is not installed

module.js:549
    throw err;
    ^

Error: Cannot find module '/path/to/repository/.yarn/releases/yarn-1.19.2.js'
    at Function.Module._resolveFilename (module.js:547:15)
    at Function.Module._load (module.js:474:25)
    at Function.Module.runMain (module.js:693:10)
    at startup (bootstrap_node.js:188:16)
    at bootstrap_node.js:609:3

What is the expected behavior?
To install the specified yarn version if the file is not found.

@ericclemmons
Copy link

So far the answer seems to be yes. It becomes a hard-dependency of the project to run or even install as you intend.

@jpSimkins
Copy link

Sooo... I am new to yarn, this question is exactly what I am trying to figure out. I am using Typescript and I need to make sure that other devs on this repo won't have an issue. I am assuming I need to commit the .yarn folder since this is added when I run yarn set version berry.

I see thumbsdown so I am confused as to what is proper procedure for this.

@Maxim-Mazurok
Copy link

Also was confused.
From the docs: https://yarnpkg.com/cli/set/version

A very good use case for this command is to enforce the version of Yarn used by the any single member of your team inside a same project - by doing this you ensure that you have control on Yarn upgrades and downgrades (including on your deployment servers), and get rid of most of the headaches related to someone using a slightly different version and getting a different behavior than you.

So, I'm guessing it goes into the repo, indeed.

@merceyz
Copy link
Member

merceyz commented Jan 2, 2021

@Philzen
Copy link

Philzen commented Aug 9, 2022

As in git theory it's an anti-pattern

  • to commit compiled / executable code into your own repository
  • to commit code that is not developed in your repository, but comes from somewhere else (b/c of repetition and not having a single source-of-truth)

i was wondering: is there some alternatives?

For instance:

  • if there was some official repository containing the .cjs-releases one could just link the .yarn/releases-folder as a git submodule pointing to that repo
  • if there was a build job (e.g. as part of yarn install) that would guarantee to always install the correct version from the .yarnrc-file into the folder

… that'd would solve the above listed concerns and allow for "cleaner" and also smaller repositories. Is there something like that available or being worked on?

@gilbsgilbs
Copy link

gilbsgilbs commented Aug 24, 2022

I'd like to add that this could be a security risk in some cases: one could easily sneak malicious code into one of the bundles and make it look like a "normal" yarn release or plugin update. It would be quite complicated to notice anything abnormal as there isn't any integrity check.

Also, to elaborate on @Philzen response, it is considered an "anti-pattern" in git not only because it looks ugly (it does), but also because it is very sub-optimal as git isn't able to properly compute and compress deltas on such minified asset. Basically any repo that follows yarn guidelines will get slower and slower with time because of this. Most package managers have abandoned vendoring nowadays (for good). Yarn docs should at least recommend using git LFS or something similar to prevent this.

I see that this goes slightly towards the right direction thanks to corepack, but yarn could do this natively (i.e. without corepack) and plugins are, I think, still an unsolved issue. I honestly fail to see a justification for doing this: yarn releases, plugins and you-name-it could be put in a configuration file and a lockfile, and downloaded when needed, just like any dependency in any dependency management system?

I found pinyarn that attempts to workaround this issue.

@joshribakoff-sm
Copy link

joshribakoff-sm commented Aug 26, 2022

@gilbsgilbs did you see the part of the yarn docs that recommend this in gitattributes?

/.yarn/releases/** binary
/.yarn/plugins/** binary

From what I understand, this tells git to optimize the files (addresses your concern).

If you don't like it, why not just ignore the folder in your gitignore?

As for me, I want to commit mine but CI runs the wrong version. When I have CI run yarn set version, it re-downloads and overwrites the file which seems to defeat the point of committing it. What am I missing to get yarn to use the committed version, and not re-download it in CI? I have yarnPath: .yarn/releases/yarn-3.2.3.cjs in my yarnrc, but in docker and CI builds it uses yarn v1 instead of the specified version. I guess a workaround is to not run yarn, and instead run .yarn/releases/yarn-3.2.3.cjs directly but that seems to defeat the point of the yarnrc setting.

UPDATE - I guess I failed to copy .yarnrc.yml into the docker container :)

@gilbsgilbs
Copy link

gilbsgilbs commented Aug 26, 2022

@joshribakoff-sm

did you see the part of the yarn docs that recommend this in gitattributes?

Yes, I saw that, thanks for pointing it out. As far as I understood, this will prevent the files from showing up in diffs/PRs which indeed reduces the burden and may actually help a bit with performance (I'm not actually 100% sure on this part), but git will still have to calculate deltas on binary files for no reason. A better advice regarding performance would probably be to use git lfs that would at least allow git to store pointers in its index rather than whole blobs.

Those advices though make things slightly worse regarding security: you definitely won't be able to notice anything suspicious in the diffs if they don't even show up.

If you don't like it, why not just ignore the folder in your gitignore?

This is not sufficient because yarn then complains the module is missing. I ended up manually installing yarn through corepack (on top of gitignoring .yarn/releases) which works fine in my case. Still, this is way more cumbersome than the current "official way" (which is basically 100% automatic regardless of the version of yarn installed in the system), and I reckon vendoring yarn and its plugins shouldn't be promoted at all (not only to me but to any user):

  • it's still encouraging bad git and security practices to users
  • my workaround doesn't apply to plugins (or at least, if I want to do something similar with plugins, I'll unnecessarily loose the benefit of pinning)
  • gitignoring releases is not officially endorsed by yarn which may make it more subject to breaking changes and more likely to become clumsier with time and iterations. It's already more complicated than it should be because yarn could as well automatically download the release if the packageManager property is filled rather than just complaining the module is missing.
  • while I saw sensible arguments in the docs about why vendoring was implemented, I still fail to understand how those arguments reject the proposal of lockfiles (and I suspect the sole reason is that vendoring was simpler to do which would be perfectly fine with me). Following the exact same rationale described in the docs, why not vendoring nodejs itself, and even all of the project's dependencies? In the meantime, if there are actual good arguments to vendoring yarn releases and plugins over relying on lockfiles, I'd like them to be documented (even if it's just in some issue) and I'd like to understand them (even if I don't agree with them). I mean, if corepack is now the recommended way of installing yarn, then why vendoring is encouraged for yarn at all? What's the benefit? Am I using the tool wrong?

For instance, regarding your CI issue, I think it could be solved elegantly by relying on your CI's caching features to put yarn releases and plugins in a cache. It would be quite easy for yarn to invalidate the releases or the plugins from the cache in case of a version mismatch. But TBH, downloading a few kilobytes in a CI server is generally not a huge deal when pnp/node_modules usually consists in downloading tens if not thousands of megabytes of dependencies.

@joshribakoff-sm
Copy link

joshribakoff-sm commented Aug 26, 2022

git will still have to calculate deltas on binary files for no reason.

I'm not an expert on git, but I do believe it's efficient and not quite what you suggest, eg:

https://stackoverflow.com/questions/48301920/what-types-of-binary-files-does-git-keep-deltas-for
https://git-scm.com/book/en/v2/Customizing-Git-Git-Attributes

Also git LFS is not as portable (and uses similar storage mechanisms under the hood), and is more akin to committing the files to a separate repository, based on my limited understanding.

This is not sufficient because yarn then complains the module is missing
I ended up manually installing yarn through corepack

Isn't this the same with npm or yarn v1? You have to install it, prior to using it :) Either you need to install in CI every run, or commit it once to avoid the need to install every CI run. Seems reasonable to me.

it's still encouraging bad git and security practices to users

Having CI going out to the network and trusting 1,000s of packages from 1,000s of authors that you have no control over is arguably insecure, whereas committing code you reviewed is arguably secure. In either case, I also don't feel that yarn encourages one way over the other, they give you the option as noted above.

Yarn can also verify the integrity of the committed files by going out to the network with the checksums, if you want, which also addresses your concern (and if there's no way for yarn to verify the releases/ folder I think you should open an issue report for that since I do agree it should).

arn could as well automatically download the release if the packageManager property is filled rather than just complaining the module is missing.

That's fair, perhaps decoupling this suggestion into a new issue report would make it easier for others to comment on this proposal?

while I saw sensible arguments in the docs about why vendoring was implemented, I still fail to understand how those arguments reject the proposal of lockfiles

I don't think these are mutually exclusive. I commit my lockfile, and the .yarn/releases, works great for me.

I think it could be solved elegantly by relying on your CI's caching features to put yarn releases and plugins in a cache

There's nothing stopping me from doing that, but I prefer to commit them since it takes time for your CI's cache feature to restore the files, in the case of my CI provider it is much faster to git checkout than to wait on a separate step that has to download and un-gzip a separate cache - but the great thing here is that as far as I understand yarn is agnostic, if you want to restore the files via your CI's cache instead of commit them, it does not care, as long as the files are there it will leverage them.

@gilbsgilbs
Copy link

gilbsgilbs commented Aug 26, 2022

Seems like I missed the bigFileThreshold introduction and all these smart behaviors in git. Thanks for the pointers, that's insightful. I'll also agree that I'm not enough of a git plumbing expert to maintain my argument. I suspect that the proposed solution would still make you download all the revisions on clone if you don't specify --depth argument, which is still cumbersome if that's the case.

Isn't this the same with npm or yarn v1? You have to install it, prior to using it :) Either you need to install in CI every run, or commit it once to avoid the need to install every CI run. Seems reasonable to me.

With yarn v1/npm, you could always use the program that was installed with you system / docker image. I think the situation with berry is quite different as it may refuse to run in some cases.

Also, I disagree that encouraging committing a minified JS files in any repo is “reasonable” in any case, but I fully respect and appreciate that you have a different opinion. For example, if you have very strong optimization requirements or very strict firewall rules and can't afford making packages before going to production, it might make sense to commit dependencies along with your project. However I don't think this is the case for most people using yarn (I might be wrong though).

Having CI going out to the network and trusting 1,000s of packages from 1,000s of authors that you have no control over is arguably insecure, whereas committing code you reviewed is arguably secure. In either case, I also don't feel that yarn encourages one way over the other, they give you the option as noted above.

Not talking about the same vectors here. Downloading untrusted packages is not the same as allowing internal or external contributors making malware patch pass through code reviews. Both issues need to be taken care of, but berry definitely added opportunities for the latter by encouraging the vendoring minified and unverified assets.

Yarn can also verify the integrity of the committed files by going out to the network with the checksums, if you want, which also addresses your concern (and if there's no way for yarn to verify the releases/ folder I think you should open an issue report for that since I do agree it should).

I agree that a checksum on the releases and plugins is a way to address the security concern I'm raising. I've yet to find such option. Also, if such option exists, the fact that it isn't enabled by default is an issue in itself as most vulnerable people are those who aren't aware of the issue.

That's fair, perhaps decoupling this suggestion into a new issue report would make it easier for others to comment on this proposal?

👍

I don't think these are mutually exclusive. I commit my lockfile, and the .yarn/releases, works great for me.

I fully agree lockfiles and vendoring are not mutually exclusive and I don't think a breaking change is required to bring lockfiles for yarn releases and plugins. For example, you can commit your yarn.lock and your node_modules or just your yarn.lock (or none at all which isn't recommended). Golang used to promote vendoring which was later superseded by gomodules (+lockfiles) later on, but vendoring is still supported as far as I know.

Also, to eliminate any confusion, my point is about lockfiles for yarn releases and plugins, not much about the yarn.lock which as of now is only used to lock NPM dependencies. Although one implementation of yarn releases and plugins locking could very well be to put their checksums and versions somewhere in the yarn.lock file.

in the case of my CI provider it is much faster to git checkout than to wait on a separate step that has to download and un-gzip a separate cache

I'd be curious to know how significant the difference would be.

That makes me think: another rationale behind yarn vendoring could be to save them bandwidth. That would also be understandable, but I think it should be documented if this is the case.

@joshribakoff-sm
Copy link

joshribakoff-sm commented Aug 26, 2022

Also, I disagree that encouraging committing a minified JS files in any repo is “reasonable” in any case, but I fully respect and appreciate that you have a different opinion.

I agree. I would suggest [another] ticket for that. There's no reason to minify code being committed to the repo and run in a nodeJS context, and no one should be publishing minified code to npm (hot take, I know) since the consumer should be able to use their own build system and choose their own target (es5, es6, etc.)... so I totally agree with you here!

Edit: Actually on further thought it makes sense to commit minified code assuming yarn provides a command to "eject" and review the raw source, which I think it does. Looks like with yarn plugins, you can write your own custom 3rd party "linker" that does whatever you want, including encourage committing un-minified code :)

Also, if such option exists, the fact that it isn't enabled by default is an issue in itself as most vulnerable people are those who aren't aware of the issue.

Totally agreed!

That makes me think: another rationale behind yarn vendoring could be to save them bandwidth. That would also be understandable, but I think it should be documented if this is the case.

I see it as more like yarn is providing a pathed path for people who want to do it, which is nice. I don't feel like they're "pushing" it on me, but yeah it saves everyone bandwidth and I do recall that was alluded to somewhere in the docs :)

You do have some great feedback, I would just maybe filter it through a lens of understanding that yarn isn't meant to appease to one specific use case -- surely you understand yarn isn't going to prohibit an existing use case relied upon by tens of thousands of it's user-base, but you are correct about all the tradeoffs and you have some really great suggestions on how to optimize the tradeoffs :)

jonthysell added a commit to microsoft/react-native-windows-samples that referenced this issue Mar 21, 2025
## Description

This PR removes unnecessary `.yarn` folders from the repo and prevents
them from being checked-in / scanned by CodeQL in the future.

### Why
Having yarn binaries checked-in is a security risk and CodeQL complains
about issues in the scripts (which we can't fix, or even see, as they're
obfuscated as binary files).

Removing the `.yarn` folder for older samples is not a problem, but
newer samples which use Yarn 3, such as `NativeModuleSample/cpp-lib`,
need the Yarn 3 binaries checked-in.

Unfortunately there's no way to get around this. Yarn refuses to
acknowledge this anti-pattern and security issue (see
yarnpkg/yarn#7741) and the "solution" to
require users to install Yarn 3 via `corepack` does not work in GitHub
CI (see actions/setup-node#480). I've tried
every workaround combination in these two issues and none work.

Closes #1016 

## Screenshots
N/A

###### Microsoft Reviewers: [Open in
CodeFlow](https://microsoft.github.io/open-pr/?codeflow=https://github.com/microsoft/react-native-windows-samples/pull/1017)
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

No branches or pull requests

8 participants