Commit package-lock.json file (?)

[mjethani]

Background

The ​package-lock.json file of npm is supposed to be committed to the repository. This avoids problems like the ​event-stream incident, where a dependency of a dependency either inadvertently or maliciously causes a problem.

We use external code for building (e.g. ​webpack), but no one can say what version of the external code was used to build any given version of a generated file in Adblock Plus. If the package-lock.json were in the repository, it would be a matter of looking it up to say what version of a certain external dependency was used in a given version of Adblock Plus.

Note that if the package-lock.json is committed then it would also have to be updated from time to time (to get bug and security fixes, new features, and just general improvements), but this should be a conscious step. In general the longer a version has been out without any known serious issues the more stable and secure it should be considered (see ​Lindy effect).

What to change

[...]

[greiner]

I'd be happy giving package-lock.json a second chance. Unfortunately, it turned out to cause issues when it was first introduced which is why we started explicitly preventing npm from creating one for both the Flattr extension and Adblock Plus UI repo.

More specifically, we were encountering build errors using Node 8 due to using cached modules that were installed using Node 7.

[hfiguiere]

Does that mean we'd have to pin the NodeJS version ?

[sebastian]

Initially, we added package-lock.json to the repository, but it turned out that the file was rewritten every time you run npm install with inconsistent output across different versions of Node.js/npm which rendered it unpractical for version control.

Replying to hfiguiere:

Does that mean we'd have to pin the NodeJS version ?

I am not aware of a way to pin the version of Node.js itself. I guess you could require a specific version in your README though. But either way that would mean that every developer/contributor has to obtain that specific version of Node.js and install/use it in a rather obscure way (in order to avoid conflicts with other installed versions of Node.js, e.g. by your package manager on Linux), adding significant and unnecessary complexity to the setup of the development environment for our code base.

Generally, I'm not convinced that it's appropriate to lock the exact version of every package (we either directly or indirectly depend on). Sure, it helps in scenarios where a once legit package gets taken over by a malicious party. But this is only one scenario, and a not any less common scenario seems to be when a programming mistake by the original author of a legit package results into a vulnerability that isn't discovered until later. In that case you want to get the fix as soon as available and not be stuck with the version that got locked when you introduced (or last updated) the dependency.

[erikvold]

Replying to greiner:

I'd be happy giving package-lock.json a second chance. Unfortunately, it turned out to cause issues when it was first introduced which is why we started explicitly preventing npm from creating one for both the Flattr extension and Adblock Plus UI repo.

More specifically, we were encountering build errors using Node 8 due to using cached modules that were installed using Node 7.

I think the issue we ran in to ​https://github.com/flattr/rising-tide/pull/702#issuecomment-308581358 was about using github urls, which is supposed to be fixed now ​https://github.com/npm/npm/issues/17189

Replying to sebastian:

Initially, we added package-lock.json to the repository, but it turned out that the file was rewritten every time you run npm install with inconsistent output across different versions of Node.js/npm which rendered it unpractical for version control.

The reasons for this happening are mentioned at ​https://docs.npmjs.com/files/package-locks

In this case I think we just need to be aware to use the --no-save, --no-shrinkwrap, and/or --no-package-lock options when using npm i if we don't wish to update the package-lock file.

For example if you want the latest package versions installed and the package-lock file ignored, and you don't want to generate an updated lock file then we should use npm i --no-shrinkwrap --no-package-lock (I think) and run npm prune at some later point.

Generally, I'm not convinced that it's appropriate to lock the exact version of every package (we either directly or indirectly depend on). Sure, it helps in scenarios where a once legit package gets taken over by a malicious party.

It's helpful in more scenarios, like

• Making sure we are developing with the exact same dependencies and not slightly different ones. This avoids situations where a bug in one version doesn't appear in the other one, which means we need a way to reproduce the error and that becomes difficult without a lock file.

• Debugging builds (mentioned in issue description)

But this is only one scenario, and a not any less common scenario seems to be when a programming mistake by the original author of a legit package results into a vulnerability that isn't discovered until later. In that case you want to get the fix as soon as available and not be stuck with the version that got locked when you introduced (or last updated) the dependency.

I think we can avoid this scenario by using --no-shrinkwrap.