If we want to lock the npm version, we can specify engine-strict=true in the .npmrc file and contributors will not have to set anything. They will not be able to install the project without an npm version that matches the range specified in engines.npm.

Also my thoughts on deleting the lockfile:
That does not seem reasonable considering that we have devDependencies (as @ericcornelissen said) and we can never ensure that we are using the same version as our users. Our dependency can publish a patch version and users will be using that new version. They will definitely catch problems before a contributor does. Here's a great article on this: https://classic.yarnpkg.com/blog/2016/11/24/lockfiles-for-all.

The PR now looks good if we decide to go with the no-package-lock approach 👍

If we want to lock the npm version, we can specify engine-strict=true in the .npmrc file and contributors will not have to set anything. They will not be able to install the project without an npm version that matches the range specified in engines.npm.

That is an interesting suggestion as well. I tested it and it does work (after changing the incorrectly named "engine" field to "engines", oops 😅, ref. #6347). However, for this project I'm not quite sure if we really want to prevent people with npm v7 or greater from being unable to install dependencies. I think it's quite common for our contributor base to not use a tool like nvm (if you don't switching to npm v6 can be a real pain).

Also my thoughts on deleting the lockfile:
That does not seem reasonable considering that we have devDependencies (as @ericcornelissen said) and we can never ensure that we are using the same version as our users. Our dependency can publish a patch version and users will be using that new version. They will definitely catch problems before a contributor does. Here's a great article on this: https://classic.yarnpkg.com/blog/2016/11/24/lockfiles-for-all.

I still share this concern, but I don't expect this to be a big problem for use as we 1) don't have a lot of dependencies and 2) don't do anything fancy with them. For this reason I'm open to trailing the no package-lock.json approach. But I completely agree that as soon as something goes wrong with that, we add it back in. What do you say @sachinraja?

An alternative to fix the problem we're having: we can add a pre-commit hook that reverts any changes to package-lock.json. New dependencies are rarely added and if you do you can easily circumvent the hook using --no-verify.

I think it's quite common for our contributor base to not use a tool like nvm (if you don't switching to npm v6 can be a real pain).

Unfortunately, I think you're right as many contributors here are new.

I still share this concern, but I don't expect this to be a big problem for use as we 1) don't have a lot of dependencies and 2) don't do anything fancy with them.

I think you're right, so I'm alright with experimenting with removing it.
Though there are a few things we should be aware of:

• We get a lot of new contributors, and it could definitely put them off if something went wrong after just an install
• We will lose out on installation optimizations: "Optimize the installation process by allowing npm to skip repeated metadata resolutions for previously-installed packages." (https://docs.npmjs.com/cli/v7/configuring-npm/package-lock-json#description)
• We will lose out on performance gains for npm v7: "As of npm v7, lockfiles include enough information to gain a complete picture of the package tree, reducing the need to read package.json files, and allowing for significant performance improvements."

An alternative to fix the problem we're having: we can add a pre-commit hook that reverts any changes to package-lock.json.

This would be better handled in CI anyway (looks like we are correctly using npm ci). Our lockfile should not change after a CI installation because otherwise someone could manually edit it to add some malicious package and we wouldn't know.

I think you're right, so I'm alright with experimenting with removing it.

Cool, in that case I think we can move ahead. @LitoMore can you resolve the conflicts for us?

We will lose out on installation optimizations

That is a fair point, but again we don't have a lot of dependencies so I don't think our gains are big here. Not to mention the fact that it takes way more time to comment on unwanted changes and reverting said changes.

We will lose out on performance gains for npm v7

If we don't have a lockfile you do get those benefits, whereas now you don't because you can't run npm install with npm v7. If we want that we would need to update the lockfile version first.

This would be better handled in CI anyway (looks like we are correctly using npm ci). Our lockfile should not change after a CI installation because otherwise someone could manually edit it to add some malicious package and we wouldn't know.

I disagree. The CI can only check if a change was made to the lockfile that shouldn't be there, not revert it (at least for PRs from forks, this is difficult).

I'm talking about reverting changes to the lockfile (such as upgrading to v2 or downgrading to v1) in a precommit hook to prevent the change from showing up in any commit. If we use husky for this, pretty much everyone who runs npm install (and therefore possibly changes the lockfile inadvertedly) is going to have this hook. And anyone that has husky disabled should understand Node/NPM/git enough not to accidentally commit changes to the lockfile.

I'm talking about reverting changes to the lockfile (such as upgrading to v2 or downgrading to v1) in a precommit hook to prevent the change from showing up in any commit.

I was thinking that reviewers would see that CI failed and understand the issue, but I get that a precommit hook is friendlier to new contributors and just easier for everyone overall.

I think this is ok to merge then.

Just for curiosity: how renovate security updates are handled after this? Are just ignored? Additionally, if we do this, we must remove this part of the renovate configuration?

Just for curiosity: how renovate security updates are handled after this? Are just ignored?

I'm not 100% and my idea was to just merge this in an see how it goes, if we encounter problems we bring back package-lock.json.

What I'm pretty sure about is that Renovate will (should) no longer address "deep" dependencies (i.e. dependencies of our dependencies). This isn't of much concern because:

• If a patch is available for a "deep" dependency and Renovate can update it, than it will now be installed by default as npm install will always install the most up-to-date version of a package it can.
• If a patch is available for a "deep" dependency that is not compatible with our dependencies, than Renovate couldn't open a PR to fix it to begin with.

Additionally, if we do this, we must remove this part of the renovate configuration?

Good point. I don't think we must remove it, but we should as it is no longer relevant. @LitoMore can you take care of that?