It looks like consensus around this has changed over the years.

Historically, there was some debate, with some arguing that libraries shouldn't commit lockfiles to allow consumers more flexibility in resolving their own dependencies. However, the current strong recommendation is for libraries to also commit their lockfiles. This ensures that the library's own tests and development environment are stable and reproducible. The consuming application will still resolve its own complete dependency tree, incorporating the library's requirements. - Gemini

Framework and library authors should also check yarn.lock into source control. Don’t worry about publishing the yarn.lock file as it won’t have any effect on users of the library.
https://classic.yarnpkg.com/lang/en/docs/yarn-lock/#toc-check-into-source-control
https://classic.yarnpkg.com/blog/2016/11/24/lockfiles-for-all/

... include a lock file for the sake of team member collaboration and reproducible builds
https://snyk.io/blog/what-is-package-lock-json/#lockfiles-for-applications-and-libraries

We had a very similar discussion with amqplib - amqp-node/amqplib#702

I see the point for the CI environment maybe. But if non latest version is important among the dev dependencies they can be locked down in package.json.

Reproducable build shouldn't be a problem in this case as the library got 0 runtime dependencies.

The consensus does not seem to have changed for other languages (Ruby or Crystal)

The 5000 line lock file effectively doubles the number of lines of code in this repo, and should probably have to updated all the time.

Did a npm update today and >500 lines changed.

I have no problem with CI catching issues with the latest dev dependencies.

But if non latest version is important among the dev dependencies they can be locked down in package.json.

This does not pin transitive dependencies

The consensus does not seem to have changed for other languages (Ruby or Crystal)

Actually looks like it has for Ruby. I'm learning as we go here :) Or maybe not consensus, but the "official" standpoint:

Yes, you should commit it. The presence of a Gemfile.lock in a gem’s repository ensures that a fresh checkout of the repository uses the exact same set of dependencies every time. We believe this makes repositories more friendly towards new and existing contributors
https://bundler.io/guides/faq.html#:~:text=lock%20when%20writing%20a%20gem,set%20of%20dependencies%20every%20time.

But not for Crystal:

If it is only a library for other shards to depend on, shard.lock should not be checked in, only shard.yml. It's good advice to add it to .gitignore
https://crystal-lang.org/reference/1.16/man/shards/index.html#usage

Anyway, I don't have a strong opinion and ok to go in either direction here. I just wanted to research it a bit since I didn't think committing lock-files for libs was a thing.

I believe the reason for adding the lockfile was supply chain attacks, from https://cloud.google.com/blog/topics/threat-intelligence/supply-chain-node-js

Consider locking version numbers of packages to prevent from auto-installing a new package that may be malicious

That's one of the reasons this library has no dependencies, unfortunately there are some dev dependencies, we could try to remove as many as possible of them too, to reduce the risk.

But as no one will ever check each dependency manually i dont understand how a lock file would do any difference. Ppl will do npm update, no?

At least you have a chance to install what's in the lockfile with npm ci

I understand both sides of this, and I have myself avoided Gemfile.lock in Ruby library projects. :) I was onboard with this change and while I never said it oud loud in Slack, I was thinking Dependabot would be updating the lockfile here regularly.

Minimizing the number of dev dependencies sounds great whatever we do here, it would lower the churn if we keep the lockfile, and it would lower the number of third-parties we need to put trust in if we remove the lockfile. I don't have any strong feeling what we do here. I often run "unknown" things in Docker. :)

baelter marked this pull request as draft now

just to stop Slack bot from nagging me