pnpm support

[mmkal]

Fixes #729

~This is starting out as a draft, because it ended up being a fairly big change. If it's unwanted, I can scrap this and do a smaller change that adds pnpm support similarly to yarn, but it was already very messy, so I thought I'd clean up.~

In the end by adding pnpm support there are net ~50 lines of code fewer, and one dependency dropped.

User-facing changes:

• new option --package-manager - this acts like the packageManager field in package.json. np will default to reading package.json, and look for lockfiles to determine the best package manager as a last resort.
• removed option --yarn (and --no-yarn). This is a breaking change. The functionality is replaced by --package-manager (if you want yarn, instead of --yarn set "packageManager": "[email protected]" in package.json or --package-manager [email protected] via CLI. If you don't want yarn, set "packageManager": "[email protected]" or whatever in package.json)
• minor/I think nobody would notice: np now does npm run test/yarn run test/pnpm run test instead of npm test etc.

Outside of the above, I tried as hard as I could do make the logic identical as far as the user is concerned.

Implementation:

• add a package-manager subfolder in source/
• this defines a type for a package manager config - the config replaces all the scary ternary logic that was everywhere with npm vs yarn vs yarn berry
• each config defines its main CLI command, how to install, how to bump a version, how to lookup the registry url, how to publish, and its expected lockfiles. There are configs defined for npm, pnpm, yarn and yarn-berry

Potential simplifications:

• np could be stricter about package managers. We could say you can only resolve a package manager based on the packageManager field, no looking at lock files. I think that would simplify things, and still make it easy for anyone who wanted to override with a specific behaviour to get what they want. But that would be a more significant breaking change, so I didn't do it for now.

Other stuff:

• I added JSDoc typescript types on a few internal methods, mostly so I could get autocomplete which I have become unhealthily dependent on over the years. I can remove if people find it ugly, but I think it's helpful and may help port to typescript some day, if that's deemed a good idea.
• For the sake of inferring types, I broke up some parts of cli-implementation.js and exported functions. It doesn't affect the API surface of this package but is potentially a bit... weird.
• I saw some pieces that looked like they already wouldn't be working with yarn berry, will comment inline
• tests are passing with minor updates ~tests were failing for me on main, so I haven't really touched them beyond removing tests for deleted code~
• I updated docs, ~but not tests. I didn't want to touch tests until I got some direction on whether maintainers would be willing to accept a PR this big. I did notice they are broken for me on main, though.~

I use np regularly and would happy to join as a maintainer if that'd be helpful.

Testing:

• I successfully published a real package using npm with this to make sure it wasn't regressing existing functionality
• I also published a package using pnpm, although I saw an issue with the bumped package.json not being committed. This happened with np v9.2.0 too, though, so I think it's a separate issue
• I haven't tested yarn because I don't use it. Would be great to get some help with that.
• Failing that, or even with that, IMO this should be published as a prerelease of a major version bump, because of the changed CLI arg, the fairly big diff, and how much mocking is involved in the tests

FYI @sindresorhus @zkochan

[mmkal]

Ah, I should have checked pull requests as well as issues: https://github.com/sindresorhus/np/pull/667

That one is the smaller, backwards compatible, but messier change, adding --pnpm/--no-pnpm args etc.

I guess that PR going quiet isn't a good sign in terms of maintenance, but if this change would be accepted, I'd be happy to maintain it going forward.

[sindresorhus]

This looks like a good start to me. I agree with the general direction. 👍

[sindresorhus]

// @transitive-bullshit @mifi

[mifi]

I like these simplifications, and can help to test the yarn berry functionality once ready

[mmkal]

Tests now passing for me locally (with the 1 known failure).

Edit: oops, forgot to run xo. Will do soon, but this is ready for review if you're willing to ignore missing semicolon etc.

[sindresorhus]

np could be stricter about package managers. We could say you can only resolve a package manager based on the packageManager field, no looking at lock files. I think that would simplify things, and still make it easy for anyone who wanted to override with a specific behaviour to get what they want. But that would be a more significant breaking change, so I didn't do it for now.

Open an issue about this. I think we could do this in a couple of years when the packageManager field is more commonly used.

[sindresorhus]

Tried this out for a bit now. This seems to work well for me.

[sindresorhus]

@mifi I believe this is ready for review.

[tommy-mitchell]

np could be stricter about package managers. We could say you can only resolve a package manager based on the packageManager field, no looking at lock files.

I think automatically determining the package manager via the lockfile makes np that much nicer to use - it "just works."

I think we could do this in a couple of years when the packageManager field is more commonly used.

That's also fair.

[tommy-mitchell]

Sorry for taking so long to review, this all looks really good to me as well. Thanks for putting this together. I use Yarn so I can give it a try for publishing with it.

[tommy-mitchell]

This worked great with Yarn v1.

[mmkal]

Thanks for the feedback - will try to incorporate it this week.

[mmkal]

@tommy-mitchell pushed your suggestions minus the () => enabled -> enabled one.

@mifi I squashed in https://github.com/mmkal/np/pull/1 - thank you v much. I like the installCommandNoLockfile better than what I had before. I made some changes in https://github.com/sindresorhus/np/pull/730/commits/3163835537d8b20290c0a77e80f422c42d153da6 though:

• instead of having a publishCli and some "prefix args", I made publishCommand a function which returns a command, similar to versionCommand.
  • That makes it less awkward/rigid about what it can do. If yarn v3 remaps every single publish cli arg, it will be possible to handle that by doing publishCommand: args => ['yarn', remapAllTheArgsWithArbitraryComplexRules(args)].
  • Similarly if future supported package managers like bun, deno, nuget, etc. etc. have totally different publishing interfaces, it could still be supported (although in that case, it may make more sense to just expose the PackageManagerConfig and let downstream users define their own configs or something).

@mifi would you be able to test on yarn berry one more time 😬? I just successfully did with pnpm again.

[sindresorhus]

@mmkal Looks good. Thanks for working on this 👍