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

Should we remove the yarn.lock file? #53

Closed
bidoubiwa opened this issue Nov 9, 2020 · 9 comments
Closed

Should we remove the yarn.lock file? #53

bidoubiwa opened this issue Nov 9, 2020 · 9 comments
Labels
help wanted Extra attention is needed JS integrations Only concerns JS tools of MeiliSearch need discussion Need discussion to make a decision

Comments

@bidoubiwa
Copy link
Contributor

bidoubiwa commented Nov 9, 2020

Based on this article

dependency lock files are not designed to be used by packages that are themselves dependencies of other packages.

Committing package-lock.json to the source code version control means that the project maintainers and CI systems will use a specific version of dependencies that may or may not match those defined in package.json. Because package-lock.json cannot be added to NPM registry (by design; see NPM shrinkwrap), projects that depend on a project that uses package-lock.json will themselves use package.json to resolve project's dependencies, i.e. what works for project maintainers/ CI systems might not work when the project is used as a dependency.

By design yarn.lock and package-lock.json are not added to NPM (see npm shrinkwrap)
Thus keeping the yarn.lock file is useful for two reasons:

  • If the project is not meant to be used as a dependency of another project.
  • To avoid conflict on non-devDependencies for contributors.

About the second reason, this is what the article says:

I would support a variation of package-lock.json if it could somehow only apply to devDependencies. I can see some (albeit small) benefit to wanting your development environment not break if there is a broken release. I would personally prefer my environment to break and become aware that a dependency in my toolkit requires attention (and depending on the nature of the issue either offer help, subscribe to an issue or replace the dependency). After all, you can easily patch your dependency tree if you need to lock down a specific version for development purposes.
However, there is no such option and using lock files at the moment will create the risks described in this article — namely that the dependencies that you use do not match those that your users will depend on. Responsible development requires that your script works with the latest versions of dependencies (and yes that includes transitive dependencies) satisfied by semver.

I also think that keeping yarn.lock file only for this use case is a lot of work. Updating minors using dependabot or asking contributors to manually rebase because of conflicts in yarn.lock is a pain.

On the other hand, having a failing development environment is not a pleasant experience for contributors. I would like your opinion on the matter.

This applies also to https://github.com/meilisearch/instant-meilisearch, https://github.com/meilisearch/docs-searchbar.js, https://github.com/meilisearch/vuepress-plugin-meilisearch

Vote

Keep yarn.lock 🎉
Remove yarn.lock 👍

@bidoubiwa bidoubiwa transferred this issue from meilisearch/meilisearch-js Nov 9, 2020
@curquiza curquiza changed the title Should we remove the yarn.lock file Should we remove the yarn.lock file? Nov 9, 2020
@curquiza curquiza added need vote When several solutions are suggested help wanted Extra attention is needed JS integrations Only concerns JS tools of MeiliSearch labels Nov 9, 2020
@curquiza
Copy link
Member

curquiza commented Nov 9, 2020

I haven't chosen my side for the moment, but I haven't found any "famous" JS library that removes the .lock file (Stripe, Algolia, Elastic, Twilio...)
We have to find out why: an old and obsolete practice or a real and important reason?

@fharper
Copy link
Contributor

fharper commented Dec 29, 2020

I always followed the suggestions of the CLI team at npm https://docs.npmjs.com/cli/v6/configuring-npm/package-lock-json

@fharper
Copy link
Contributor

fharper commented Dec 29, 2020

I asked Twitter, we'll see :D

https://twitter.com/fharper/status/1343967816597307394

@fharper
Copy link
Contributor

fharper commented Dec 29, 2020

Yarn says the same as npm (Maël just point me there): https://yarnpkg.com/getting-started/qa#should-lockfiles-be-committed-to-the-repository

@fharper
Copy link
Contributor

fharper commented Dec 29, 2020

Comment from Wes (used to work with him at npm)
image

@fharper
Copy link
Contributor

fharper commented Dec 29, 2020

Comment from Rebecca (my boss when I was at npm, and one of the main contributor to npm)
image

@fharper
Copy link
Contributor

fharper commented Dec 29, 2020

So, we are entitle to a different opinion, but when the lead maintainer of yarn, and past lead maintainer of npm said we should, it kinda weight into the scale :D

I hope that help this discussion...

@bidoubiwa
Copy link
Contributor Author

Thanks a lot @fharper but all those answers do not take into account that this project is a library and not a project.

One person in your twitter feed mentionned that they digged into the subject
Screenshot 2021-01-06 at 11 57 28

Thanks to @dkundel article I think I understand exactly what the problem is.

Let me try to explain by quoting a lot of passages of his article.

Lock files

Lock files contains the exact version of every dependency we use. With the exact version of their dependencies (sub-dependencies).
example:

cross-fetch@^3.0.5:
  version "3.0.6"
  resolved "https://registry.yarnpkg.com/cross-fetch/-/cross-fetch-3.0.6.tgz#3a4040bc8941e653e0e9cf17f29ebcd177d3365c"
  integrity sha512-KBPUbqgFjzWlVcURG+Svp9TlhA5uliYtiNx/0r8nv0pdypeQCRJ9IaSIc3q/x3q8t3F75cHuwxVql1HFGHCNJQ==
  dependencies:
    node-fetch "2.6.1"

Where we will install

  • cross-fetch 3.0.5 precisely
  • node-fetch (its sub-dependency) in version 2.6.1 precisely

Package.json

package.json file contains the name and the range of versions of every dependency.
For example:

{
  "dependencies": {
     "twilio": "^3.30.3"
  }
}

This caret (^ yep i also just learned this word) in front of ^3.30.3 means the following

^ actually means that any version bigger than 3.30.3 and smaller than 4.0.0 will be a valid version. So if any new version would be released and you don't have a lock file present, npm install or yarn install would install that one automatically and your package.json would not update. However the lock files would be different.

Why are lock files usefull ?

Lock files are amazing for web application or projects where you want everything to work precisely as it works on your local environment. Since they both have the same lock file, every dependency and sub-dependency will have the same version on every environment you install that project.
It is also usefull in the case of a CI where you would want to test your application with the same exact environment as anywhere else.

Why are lock files not usefull for libraries.

Libraries are puched on npm. When it is pushed, some files are not added to npm. For example .git is never added and lock files are also never added.
Which means that if a user adds our library to its project, they will probably not have the same version of our depedencies. But also their sub-dependencies.

As @dkundel explains

This might cause the "works on my machine" effect by accident since your CI and developer environment might pick up a different version of dependencies than your users. So what can we do instead?

(Of course, this does not happen in a world where minor increments only adds features and patches increments only fix problems. )

What are the benefits of not adding our lock files

Whenever a maintainer works on our library, when building the library it will install all dependencies on their latest versions (ex: 3.146.5 because in package.json it is written "twilio": "^3.30.3". it will also install every sub-dependency that might have updated as well to newer minor and patches versions.

Meaning that if we can not bundle the project by doing yarn build, we know that our users that will install our project from npm will also not being able to use our library. Or if the tests fails, we know that something has changed for the user.

That way, if we regularly maintain this project we will be able to know whenever this library doesnt behave the way it should for all users that download this library from npm.

Check could also be automated as a CI by using npm pack instead of npm install. That way we test our library with the same version a user that download this library will at that exact moment.

What are the downsides of removing the yarn.lock of our library.

As we ensure that we regularly check that everything is working the way it is supposed to for our users. We also remove the comfort of having the development environment always work perfectly.
As dependencies are update, devDependencies are updates as well. Without a lock file, maintainers may encounter some problems while trying to develop because of updated dependencies.

Conclusion

Based on this information:

npm will take the list of files and will package them all up together into a tarball using npm pack. If you want to check out what files are packaged you can run in a project npm pack --dry-run and you'll an output with all of the files.

That tarball will then be uploaded to the npm registry. One thing you might notice when you run this command is that if you already have a package-lock.json it is actually not being bundled. This is because package-lock.json will always be ignored as specified by the list in the npm docs.

I think there are two possible good solutions.

1. Keep lock file and tests against tarball

This means that we keep the lock file for our maintainers comfort. But instead of testing the library that has be build using the lock file, we test the library that has been build without a lock file. That way we ensure also that our users will have a working version that outputs the same results as the version with lock file. And that there are no breaking changes in any of the dependencies and sub dependencies updates.
We should also determine if this is possible. As a first observation I did not find a good way to do that but I will update here.

2. Remove the lock file.

At the cost of maybe having sometimes unexpected results with devDependencies (which again should not be the case if our dependencies correctly uses semver), we ensure that our users always have a working library.

@curquiza curquiza added need discussion Need discussion to make a decision and removed integration guides need vote When several solutions are suggested labels Apr 15, 2021
@bidoubiwa
Copy link
Contributor Author

yarn lock is kept until industry good practices are clearly in favor of removing it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed JS integrations Only concerns JS tools of MeiliSearch need discussion Need discussion to make a decision
Projects
None yet
Development

No branches or pull requests

3 participants