Create npm-shrinkwrap.json, use 'npm ci' in build - Closes #2656

[fchavant]

What was the problem?

npm install was being used for builds meaning dependencies of dependencies could change between builds.

How did I fix it?

Use npm ci instead.

How to test it?

Run build; ensure node_modules tree is reproducible (and matches locked packages)

Review checklist

• The PR resolves Use npm ci for builds #2656
• All new code is covered with unit tests
• All new code was formatted with Prettier
• Linting passes
• Tests pass
• Commit messages follow the commit guidelines
• Documentation has been added/updated

[fchavant]

@MaciejBaj we're still using npm install for pm2 and lisk-commander. We already have pm2 in package{,-lock}.json but it's a devDependency. Can we move pm2 to dependencies? And add lisk-commander too?

[MaciejBaj]

@fchavant how about https://docs.npmjs.com/files/package.json#bundleddependencies?

[fchavant]

@MaciejBaj it's worth a try
EDIT: that won't work, bundledDependencies do not get added to package-lock.json

[diego-G]

@fchavant @MaciejBaj shall we somehow mention this in the Readme ?

[fchavant]

@diego-G we should, but we still need a solution for pm2 and commander

[diego-G]

PM2 is now installed as dependency. Shall we then remove this instruction? https://github.com/LiskHQ/lisk/blob/d69980d74bd60b9c203321d89f28c7b4a5fa37b6/README.md#pm2-recommended

Removed instructions to install pm2 separately from the README.

[Nazgolze]

@diego-G good catch 👍

[diego-G]

@fchavant please revert the commit and forget my comment. For usability, it's better to keep it otherwise we need to offer npm scripts to use the pm2 installed under ./node_modules/.bin/pm2.

[diego-G]

@fchavant I've noticed npm ci doesn't check the existing engines from package.json. This allows to install with a Node version we don't support.

I'm not sure either to use it regularly for development where you don't need to build every time. I would leave npm i in the Readme otherwise we need to find a solution to respect the engines.

[fchavant]

@diego-G
The engines field "specif[ies] which versions of npm are capable of properly installing your program" (source; emphasis mine) which might not be the same as the version that can build the application. Still we use enforce the version used at build time. Anyone followint the README will be using the right version of Node.js also.

Requested changes are addressed.

[diego-G]

Due to the explanation from the comment #2662 (comment) , we should have npm install here and clarify that npm ci is meant to be used when we are building the app not when installing. We must mention the finding in a comment to prevent users from bumping into the same issue:

• npm ci doesn't apply the restrictions engines set in the package.json.

[fchavant]

It seems we have to choose between having the right packages (from package-json.lock) or enforcing the engines; it is unfortunate we cannot have both.
I would argue it is easier to document which version of npm to use (and to follow that instruction) that to ensure the right packages are installed.

[MaciejBaj]

npm ci in the manual might look a bit odd, but it serves the purpose of having the exact same code including the one in dependencies across all of the same Lisk Core versions running from Sources, Binaries and Docker. I found this article convincing.

There is a high probability that the npm ci would fail for other versions of Node.js due to the engine settings in package.json of other dependencies anyway (even if Lisk Elements or Lisk Commander).