Update package-lock to v2

[noahtallen]

I was trying to install a dependency the other day. I was using [email protected], and npm install was generating wild changes to the lockfile. (See the modifications to the lock-file in this PR, which are massive.) It looks like npm is updating the file format (https://docs.npmjs.com/cli/v7/configuring-npm/package-lock-json#lockfileversion), and npm 7 switches it automatically.

Gutenberg has a precommit check which requires us to use the latest npm version. However, it seems like the project is assuming npm 6 everywhere else. For example, CI runs with npm 6, and node LTS installs npm 6 by default. This isn't compatible with our precommit check, which requires package-lock changes to be committed with npm v7. I think this is creating some issues in the static CI checks.

I wanted to start a conversation about this: do we commit to npm 7 now, and update the package lock, or should we change our local tooling to require npm 6 and not npm 7?

[jsnajdr]

The current check-latest-npm script sends an online request to the NPM registry to figure out the latest version of the npm package and complains if you don't have it. I think that's unfortunate. This kind of check should provide a stable environment and controlled upgrades of tools. Now we have the opposite: when NPM is upgraded upstream, your local environment can be reported as obsolete although it was OK few minutes ago.

We already have a .npmrc file and a engine.npm field in package.json that says "npm": ">=6.9.0". So why don't we check against this field? There's already a wp-scripts check-engines script that does exactly that.

[youknowriad]

We already have a .npmrc file and a engine.npm field in package.json that says "npm": ">=6.9.0". So why don't we check against this field? There's already a wp-scripts check-engines script that does exactly that.

Because the guideline that was discussed a long time ago was not to rely on a specific version but to use the latest npm version always. That said, that guideline doesn't make sense to me though :P, it's better if the project defines what npm version it supports explicitly.

[jsnajdr]

the guideline that was discussed a long time ago was not to rely on a specific version but to use the latest npm version always.

That was fine while the latest versions were patch releases in the 6.x.x line. Now it means a spontaneous and uncontrolled upgrade to v7 🙂

[fluiddot]

👋 I also created some time ago a similar to PR for updating the lock file format version but the CI checks are also failing.

Looks like until NPM releases a new version with a fix we shouldn't use NPM 7, as a workaround I created a PR in case we want to limit the use of NPM 7 in the meantime.

[ockham]

Quoting #28834 (comment)

👋 Looks like the npm/cli bug related to the dependencies install error is already solved and aims to be released on NPM 7.5.4 🤞 .

I'll try rebasing this PR to restart tests.

[ockham]

Seems like we agree overall that we shouldn't depend on "latest available npm" @noahtallen @jsnajdr @youknowriad, for aforementioned reasons. Let's use either the engine.npm field from package.json, or .npmrc instead, as Jarda suggested.

[noahtallen]

Sounds good. I guess we just need to make sure that npm --version is the latest version in the range specified in this build script 🤔

[ockham]

Sounds good. I guess we just need to make sure that npm --version is the latest version in the range specified in this build script

@kevin940726 filed a PR to that effect: #29204 👏

[noahtallen]

I'll close this out then :)