Use npm instead of Yarn

[nickserv]

No description provided.

[patrickhulce]

thanks @nickmccurdy! why though? this removes any package lock mechanism 😅

[nickserv]

This organization uses npm without a package lock for libraries, and with a package lock for apps. I'm just making pull requests to keep things consistent for contributors.

[patrickhulce]

Ah sorry I missed that discussion is there a link anywhere to the rationale? :)

[nickserv]

In the gitignores of other libraries:

# these cause more harm than good
# when working with contributors
package-lock.json
yarn.lock

https://github.com/testing-library/dom-testing-library/blob/master/.gitignore#L6-L7

I believe Kent has a more in-depth explanation somewhere but I couldn't find it.

[patrickhulce]

I see, thanks!

I can understand the library dependency angle, but I'm not sure I agree with the rationale when working with devDeps that actually produce distributable assets like pptr-testing-library does though (which is much more similar to the app situation than library dependencies). Removing the lockfile here feels a bit like policy matching for the sake of policy matching rather than actually following the reasoning of the policy :/

How would you feel about replacing with a package lock?

[nickserv]

Are you referring to how dependent projects would install versions of pptr-testing-library dependencies? If so, it would be the same, as package-lock.json and yarn.lock are not used to install published packages, only local packages. package.json can be used to lock top level dependencies and npm-shrinkwrap.json can be used to lock nested dependencies (npm only).

Personally I usually prefer using lock files, but it's not the preference of the organization it seems, and I think not locking dependencies can help with testing different installations in this situation.

[patrickhulce]

I'm aware of how lock files work :)

I am not referring to dependent projects. pptr-testing-library in CI builds a bundle of javascript that is distributed as part of our own npm package. The lock file ensures that there are no unexpected changes to this bundle and given that we do string replacement and relatively hacky things to make this work, I am concerned that unexpected transitive dependency changes could result in breakages.

I understand the spirit of the ban on lock files, that a library author should ensure their library can handle unexpected dependency versions that real users might install. This package goes out of its way to ensure we handle different versions, above and beyond what simply removing a lock file would do. Enforcing a lock file ban here would increase the volatility and reproducibility of published versions of this package while not actually reaping any of the stated benefits beyond the steps we've already taken.

I appreciate that you're seeking consistency across the organization, but consistency in reasoning for positive outcomes should prevail over consistency for file existence.

[nickserv]

Alright, I'll add a lock file for npm in this case.

We have two options, what would you prefer?

1. Use npm 6, which is stable and used by more users, guessing dependency versions from node_modules in a way that may conflict
2. Use npm 7, which is in preview and requires an explicit install for now, which can accurately import versions from yarn.lock

[patrickhulce]

Thanks @nickmccurdy!

Use npm 7, which is in preview and requires an explicit install for now, which can accurately import versions from yarn.lock

This sounds great :)

[patrickhulce]

thanks @nickmccurdy! 🎉

[nickserv]

This should work locally after running npm install --global npm@7 and npm install (or just npx npm@7 install to use it temporarily without upgrading your global npm). At this point I'm waiting to see if CI is using npm 7 yet.

[nickserv]

It seems like npm 7 is successfully installed now. Can we get another review please?

[patrickhulce]

still LGTM :)