karma icon indicating copy to clipboard operation
karma copied to clipboard

Update to the v2 npm lockfile.

Open jonathanKingston opened this issue 2 years ago • 9 comments

This change just updates the lockfile to the version 2 which is backwards compatible with v1.

This simplifies updating dependencies, as otherwise the author will need to downgrade to <v7 npm.

jonathanKingston avatar Apr 06 '22 13:04 jonathanKingston

Shouldn't there also be a change in package.json? Otherwise, how did you generate this PR?

jginsburgn avatar Apr 12 '22 05:04 jginsburgn

Shouldn't there also be a change in package.json? Otherwise, how did you generate this PR?

I wish, unfortunately this change just comes from having npm v7.0.0 or greater.

The one thing to note is that whilst it is backwards compatible you'll want to be using npm >7 onwards to generate otherwise this will come back. Fixing the npm version in the package.json is an option here and providing an .nvmrc for ease of use.

jonathanKingston avatar Apr 12 '22 06:04 jonathanKingston

I think we should hold off this a bit and migrate to the NPM 7+ as part of the next major release.

devoto13 avatar Apr 12 '22 09:04 devoto13

SGTM thanks!

jonathanKingston avatar Apr 12 '22 09:04 jonathanKingston

I think we should hold off this a bit and migrate to the NPM 7+ as part of the next major release.

If the change is backward compatible, why should we hold off for the next major release?

jginsburgn avatar Apr 13 '22 02:04 jginsburgn

My bad! I was certain that NPM 7 does not support Node 10, so that's why I wanted to postpone it. That's actually not the case, so let's consider this.

Caveat is that NPM 6 is the default version in Node prior to 15 and the development installation will fail with the updated lock file. IMO it's okey as the active LTS is Node 16 which comes with NPM 8 out of the box. With the older Node versions developers can also install newer npm with npm i -g npm@latest. I don't think we can provide a smooth transition and will have to choose whether to provide support for developers using NPM 6 or NPM 7+. I'm fine upgrading now, but let me know if you would prefer to postpone it @jginsburgn?

devoto13 avatar Apr 14 '22 17:04 devoto13

My bad! I was certain that NPM 7 does not support Node 10, so that's why I wanted to postpone it. That's actually not the case, so let's consider this.

Caveat is that NPM 6 is the default version in Node prior to 15 and the development installation will fail with the updated lock file. IMO it's okey as the active LTS is Node 16 which comes with NPM 8 out of the box. With the older Node versions developers can also install newer npm with npm i -g npm@latest. I don't think we can provide a smooth transition and will have to choose whether to provide support for developers using NPM 6 or NPM 7+. I'm fine upgrading now, but let me know if you would prefer to postpone it @jginsburgn?

I think this raises a more general issue: establishing the minimum npm version supported.

jginsburgn avatar Apr 25 '22 03:04 jginsburgn

@devoto13 how can you tell which is the default npm version for each Node.js and the lifecycle for each npm release?

jginsburgn avatar Apr 25 '22 03:04 jginsburgn

@jginsburgn You can find the default npm version here: https://nodejs.org/en/download/releases/. I'm not sure if there is a particular npm version lifecycle defined.

I think this raises a more general issue: establishing the minimum npm version supported.

The problem is that we only want to enforce it for the karma developers, not karma users, so we can't set engines.npm constraint in the package.json. I think we primarily have two choices here:

  1. Migrate to NPM 7 now (potentially in a major release where we drop Node 10 and 12 and add Node 16 and Node 18) and ask developers staying on Node 14 (the only LTS version shipping with npm 6) to install npm 7+ manually.
  2. Migrate to NPM 7 in a year when the Node 14 goes EOL and all supported Node versions come with npm 7+.

devoto13 avatar Apr 25 '22 20:04 devoto13