chore: Add package lock to fix CI

[nickserv]

What: Enables npm's package lock (package-lock.json) and keeps it committed

Why: The CI build is currently failing because it is installing dependencies differently from my local machine, in such a way that for some reason dtslint's version is causing dozens of errors. My local build validates just fine, so I uploaded my own package-lock.json, which is now working perfectly in CI. It's important that we keep it committed to avoid issues like this in the future. Fortunately npm obeys, updates, and even resolves merge conflicts in the lock file automatically, so we shouldn't have any issues with contributors understanding how to use it.

How: Removed npm config and committed package-lock.json

Checklist:

• Documentation added to the
  docs site N/A
• Tests N/A
• Typescript definitions updated N/A
• Ready to be merged
  

[codesandbox-ci]

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 41bec35:

Sandbox	Source
objective-zhukovsky-jzvlk	Configuration

[codecov]

Codecov Report

❗ No coverage uploaded for pull request base (master@b9900f8). Click here to learn what that means.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             master      #624   +/-   ##
==========================================
  Coverage          ?   100.00%           
==========================================
  Files             ?        24           
  Lines             ?       559           
  Branches          ?       140           
==========================================
  Hits              ?       559           
  Misses            ?         0           
  Partials          ?         0           

Impacted Files	Coverage Δ
src/events.js	100.00% <0.00%> (ø)
src/queries/test-id.js	100.00% <0.00%> (ø)
src/role-helpers.js	100.00% <0.00%> (ø)
src/queries/alt-text.js	100.00% <0.00%> (ø)
src/helpers.js	100.00% <0.00%> (ø)
src/queries/label-text.js	100.00% <0.00%> (ø)
src/wait-for-element-to-be-removed.js	100.00% <0.00%> (ø)
src/wait-for.js	100.00% <0.00%> (ø)
src/queries/placeholder-text.js	100.00% <0.00%> (ø)
src/queries/title.js	100.00% <0.00%> (ø)
... and 14 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b9900f8...41bec35. Read the comment docs.

[smeijer]

so we shouldn't have any issues with contributors understanding how to use it.

About that last part, I have my doubts.

1. As I understand it, there is a difference between npm i and npm ci.

   npm ci removes your node_modules prior install and installs dependencies based on package-lock.json data. It will also never update package.json or package-lock.json.

   npm i installs your (new) dependencies, does not remove existing ones, and can update package.json and package-lock.json. It only honors package-lock.json if it satisfies the requirements of package.json.

2. Different versions of npm can create different versions of package-lock.json

   More than once I reviewed pull-requests that included an updated package-lock, even while there was no change in dependencies. Not a huge deal, but kind of annoying.

   To prevent issues contributors would need to use npm ci to install their dependencies, and stop using npm i. Only use npm i to add or update dependencies.

Besides that, I'm okay with adding package-lock.js. So thanks for opening this PR 👍

[kentcdodds]

I'm pretty sceptical of using lock files in libraries. But if one other maintainer gives their approval then we can merge this.

[nickserv]

1. As I understand it, there is a difference between npm i and npm ci.

@smeijer Since we're just talking about users manually installing to contribute here (and not CI), they should only be using npm i(install), so the differences wouldn't affect them directly. That being said if you'd rather have one command for all environments, Yarn does this better.

2. Different versions of npm can create different versions of package-lock.json
   More than once I reviewed pull-requests that included an updated package-lock, even while there was no change in dependencies. Not a huge deal, but kind of annoying.
   To prevent issues contributors would need to use npm ci to install their dependencies, and stop using npm i. Only use npm i to add or update dependencies.

@smeijer This sounds like a workaround that could have unintended issues. This behavior is just there for automated installs. Users should always be keeping the package-lock.json format up to date as it improves. If the concern is more about how frequently the lock file format updates, we can lock the npm version or use Yarn (which arguably has more stable lockfile semantics as it adopted more locking features early on and has more community-driven RFC processes).

I'm pretty sceptical of using lock files in libraries. But if one other maintainer gives their approval then we can merge this.

@kentcdodds npm refuses to publish package-lock.json to packages, so they're strictly used in development and won't cause any issues with dependent packages. However, keeping development dependencies consistent is just as important as keeping dependencies consistent for a production application in my mind. For example, scripts like CI will rely on their consistent package versions, and inconsistencies can cause subtle bugs to be published with the library if testing tools only run properly in some environments. Fortunately in this case I was able to figure out the issue and easily solve it with my local dependencies versions, but I've been bitten before by not knowing what the correct package versions were after a bad update and no backup of the old lockfile, and committing the lock file has saved me time. I'd also be fine with Yarn to be clear, I just chose npm to be consistent with our npm scripts and the org's preference to npm over Yarn.

[smeijer]

If the concern is more about how frequently the lock file format updates, we can lock the npm version or use Yarn (which arguably has more stable lockfile semantics as it adopted more locking features early on and has more community-driven RFC processes).

No, my concern is that if you use npm version x and I'm using y, we can create different package-lock.json files.

That means, that if we both keep our package-locks "up to date", that would mean that we can keep submitting package-locks in our pull-requests all the time, even while the dependencies itself never changed. That is until we sync our npm versions.

Yes, yarn is better in this. But that brings the discussion to a whole other level. (do we really want this project to move from npm to yarn?)

I'm using package-lock in projects myself, but I haven't been a huge fan of it. I see the benefits. But it does have issues. So I'm not against adding it, but I doubt if it's worth the pain.

[nickserv]

I understand that lock files aren't perfect (especially npm because of how unstable the format is), but they solve this CI issue pretty easily, and I can't really think of a better alternative in this situation.

[kentcdodds]

I definitely recommend lockfiles in apps. But for libraries, I'm mostly concerned with the stuff I publish to work with the latest versions (including transitive dependencies) of everything and if we use lockfiles then we have to regularly update those versions (no I don't want to use a bot to keep things updated, that's a nightmare). It's unfortunate that we can't use lockfiles for only our devDeps because that would be nice.

However, even in that case, @smeijer's arguments are solid. Any time I've tried to use a lockfile in an open source project that I get contributions to, people inevitably and inexplicably update the lockfile. That leads to confusion (and is potentially a security vulnerability if someone manually updates the lockfile to install a malicious package).

The alternative that I prefer is figure out why the latest resolved version of packages is breaking our build and fix that. If it means we need to remove a ^ in our package.json that's fine with me (though sometimes the issue is in a transitive dep and I really don't have a great answer for that). Most of the time such issues go away after a little while. It's never been a huge problem for me.

[nickserv]

Unfortunately it's hard to fix these issues without a package lock tracked in version control. I personally find it easier to just commit them and be cautious about when updates are needed.

[kentcdodds]

I deleted the cache and re-ran the build and now it's passing.