Commit package-lock file

[wemeetagain]

Description:

Not committing a package-lock results in terrible DX for maintainers and new contributors alike. Building the package can break at any time (any time an updated dependency is released that breaks types), for reasons entirely unrelated to a contributor adding features / fixing bugs. This can also affect building the package in CI.

The reason for not including the package-lock is to alert maintainers to any breaking issues that result from using the package as a library (since installing a dependency does not use that dependency's lockfile). This unfortunately creates an urgent issue for all contributors (breaking the development of features / etc) which is not strictly necessary.

We should instead commit the package-lock file and rely on a cronjob workflow to alert us when the package fails to build with the latest dependencies.

Steps to reproduce the error:

git checkout github.com:libp2p/js-libp2p
cd js-libp2p
npm i
npm run build # who knows if this will work ¯\_(ツ)_/¯

[maschad]

I second this, it has been recommended by npm in the past to prevent these very issues as it relates to CI, deployments and contributions. I have also noticed a variety of issues have been caused by the abesence of the lockfile e.g.

#1364 (comment)
#712 (comment)
#1399 (comment)

If the concern is to be aware of breaking changes downstream, we could consider a shrinkwrap file or leave the dependency resolution process to the consumers, as well as @wemeetagain suggested, rely on some dep-check CI-job that checks for dependency breakages.

But primarily, this issue redirects the development cycle from shipping features to resolving dependency breakages, so I think the drawbacks of not having a lockfile versioned outweigh the benefits.

[achingbrain]

I feel like we've had this conversation before 😓

While it's true that a lockfile would stop CI being broken by patch updates of dependencies and dependencies of dependencies, it wouldn't necessarily stop actual users of libp2p being broken by the same thing.

Locking down patch updates also means we don't get patch updates to fix bugs, which sometimes we want.

We also then open ourselves up to the nonsense of having to update lockfiles on absurdly long-lived PRs, indeed any PR that touches a lockfile will almost immediately develop a conflict, which has to be resolved and run through CI again etc.

I get the feeling that a lot of the dependency hell we find ourselves in is often caused by having to update interfaces, then implementations, then consumers which may take several days and sometimes the tooling lets us down, releasing patches where they should be majors due to exporting types from a dep that has just had a major release.

My feeling is that at the root of this problem is the amount of dependency churn we have. If we move to a monorepo setup for most, if not all of libp2p, that churn should diminish since we can ship a feature to multiple modules with one PR instead of doing the merry dance we have to do now. Then this should become a lot less severe of a pain point.

[wemeetagain]

I feel like we've had this conversation before sweat

Probably so 😹 😅 Its just something I ran into again yesterday.
My story was:

- I want to run the js-libp2p tests to test a feature
- I pull the latest master
- I run `npm i`
- I run `npm run build`, it fails, some error about types being broken
- I delete my local lock file, rerun `npm i`
- I run `npm run build`, it still fails, some error about types being broken
- I give up :(

it wouldn't necessarily stop actual users of libp2p being broken by the same thing.

Right. But I guess my point is that there are other ways to alert us (maintainers of libp2p) that libp2p is broken by dependency updates.
What we have now is the nuclear option, where we have no build reproducibility, where all libp2p devs, driveby contributors, etc are subjected to the breaking thing so we're forced to address the unrelated issue or just be blocked. Thats problematic when you just want to PR a feature or just reliably test something. You have no guarantees.

We also then open ourselves up to the nonsense of having to update lockfiles

tbh I don't think that its very onerous, It's just a matter of a git merge unstable && npm i. And it happens less than you'd think, speaking from experience maintaining the lodestar codebase.
Its just a tradeoff, in return, we get the benefit of giving the branch a fully reproducible build where this flaky build issue just doesn't happen.

I get the feeling that a lot of the dependency hell ...
If we move to a monorepo setup...

Yea fair enough, the monorepo will help a lot. Would have prevented the specific build issue I saw yesterday.

[maschad]

Locking down patch updates also means we don't get patch updates to fix bugs, which sometimes we want.

As @wemeetagain mentioned, I think we should be utilize another means of alerting that does not interfere with the development cycle. @MarcoPolo had made a suggestion of it being potentially hosted somewhere.

Given this is an open source project, one of the main drawbacks is that potential contributors are going to be deterred if the main branch is continuously broken with very little recourse as to why this is so.

We also then open ourselves up to the nonsense of having to update lockfiles on semantic-release/github#487, indeed any PR that touches a lockfile will almost immediately develop a conflict, which has to be resolved and run through CI again etc.

This seems like more of an anomaly as opposed to the regularity of broken builds caused by dependency breakages.

My feeling is that at the root of this problem is the amount of dependency churn we have. If we move to a monorepo setup for most, if not all of libp2p, that churn should diminish since we can ship a feature to multiple modules with one PR instead of doing the merry dance we have to do now. Then this should become a lot less severe of a pain point.

This may take quite a bit of effort and although it may resolve the frequency in which this happens, I still think this problem is mitigated even further by committing the lockfile.

[wemeetagain]

triage notes: Most of our build errors will be resolved with the monorepo. We will hold off on committing the lock file and revisit this conversation if there are build issues in the future (post-monorepo).