Skip to content
This repository has been archived by the owner on Jan 19, 2021. It is now read-only.

Add package-lock.json to .gitignore #62

Merged
merged 1 commit into from Dec 6, 2018

Conversation

whymarrh
Copy link
Contributor

This PR adds the package-lock.json file to the Git ignore list.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 94.437% when pulling 6a59204 on whymarrh:ignore-package-lock into e6c8107 on ethereumjs:master.

@holgerd77
Copy link
Member

Since especially @krzkaczor had some arguments in favor of using lock files, I would very much like to have some discussion on this first.

I also have to do some read up on this first, haven't formed an opinion yet. @whymarrh, could you give a short summary of your arguments or link to some argumentation you go along with?

@whymarrh
Copy link
Contributor Author

whymarrh commented Nov 30, 2018

I'd be happy to.

There are two types of projects in any lockfile discussion: applications and libraries. Applications are projects that aren't consumed by other applications, where libraries are projects intended to be consumed by applications. That is, someone does npm install $project and requires it. Applications are well served by using lockfile as they are usually concerned about having a reproducible build with each build pulling in the same dependencies each time. Having a package-lock.json file in an application allows reproducible builds as well as verification that each of the dependencies downloaded are correct (checksums, registry signature verification). From the npm docs:[1]

[package-lock.json] describes the exact tree that was generated, such that subsequent installs are able to generate identical trees, regardless of intermediate dependency updates.

This file is intended to be committed into source repositories, and serves various purposes:

  • Describe a single representation of a dependency tree such that teammates, deployments, and continuous integration are guaranteed to install exactly the same dependencies.
  • [...]

Libraries, on the other hand, don't need to have lockfiles. Libraries are usually concerned about having a range of dependency versions that they'll work with to offer consuming applications some flexibility in their dependency resolution. One common, simple example is two dependencies: one that depends on ^3.0 and one that depends on ^3.4—having these ranges would allow a consuming application to install 3.4 and satisfy both ranges.

Ultimately, the package-lock.json file isn't respected when a project is installed into node_modues, so there isn't any reason to have it. (Quite the opposite, actually, it forces the same versions to be installed across CI runs and can hide incompatibilities with dependencies that are valid for the specified ranges.) From the npm docs again:[1]

One key detail about package-lock.json is that it cannot be published, and it will be ignored if found in any place other than the toplevel package.

From Understanding lock files in NPM 5 (ignore the first line, the article buries the lede), emphasis mine:

There's a couple of solutions to [the CI problem I outlined above]. First, you could sacrifice the exact reproducibility and not add the lock file to your version control system.

It's a common misunderstanding.[2] [3] [4] [5] [6]

@krzkaczor
Copy link

@whymarrh really nice writeup! I think we all agree that lockfiles are a good thing for applications, now the only question is: what about libraries.

Here's what yarn team has to say about this: https://yarnpkg.com/blog/2016/11/24/lockfiles-for-all/

So basically, they argue that you should always commit lockfile, even for libraries because:

  • without it you only make contributing to your project harder
  • without it you won't be able to catch any problems in with your dependencies because your users install your project much much more often then you change it

That being said I personally don't have strong opinion and I think since we maintain multiple smaller repositories, not having lockfiles can ease a pain of upgrading dependend projects.

@holgerd77
Copy link
Member

Ok, so let's stick to not using lock files for now.

Have added a note on this on a new Node best practices section in the docs.

Copy link
Member

@holgerd77 holgerd77 left a comment

Choose a reason for hiding this comment

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

Looks good.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants