Add support for pnpm (don't merge) #667
Conversation
| @@ -0,0 +1,11 @@ | |||
| { | |||
There was a problem hiding this comment.
not having this here tripped me up a lot in the beginning, since prettier is a pretty common default config
| > This is a fork of [np](https://github.com/sindresorhus/np) which adds support for [pnpm](https://pnpm.io). The maintainers of np [understandably don't want to support every package manager](https://github.com/sindresorhus/np/issues/251#issuecomment-400717095), so that's why this fork exists. | ||
| > | ||
| > Aside from adding support for `pnpm`, this fork is equivalent to the upstream version. | ||
|
|
There was a problem hiding this comment.
this header notice would need to be removed if we decided to not go with the fork
|
|
||
| const exec = (cmd, args) => { | ||
| // Use `Observable` support if merged https://github.com/sindresorhus/execa/pull/26 | ||
| const cp = execa(cmd, args); | ||
|
|
||
| if (!cp.stdout || !cp.stdout.pipe) { |
There was a problem hiding this comment.
I kept getting errors when running the ava unit tests about cp.stdout.pipe and cp.stderr.pipe not existing.
Looking at the test impl using sinon.sub, I don't understand how this code was stubbing and working correctly previously, so hopefully someone can tell me what I'm missing 😄
| module.exports = async (input = 'patch', options) => { | ||
| if (!hasYarn() && options.yarn) { | ||
| if (options.yarn && !hasYarn()) { |
There was a problem hiding this comment.
I switched the order here so we don't run the extra FS functions is yarn and pnpm aren't used.
|
@transitive-bullshit I don't have objections to keep a fork in the pnpm org if you are willing to maintain it. |
|
I'm happy to add support for ppm here if someone can commit to maintain it. |
|
This would be epic 🔥 |
|
Would gladly award bug bounty or sponsor you on GitHub sponsors if you can help push this to the finish line and get it baked into |
|
This PR looks good to me. I'm willing to maintain pnpm support in np if this gets merged. |
@sindresorhus @wmertens I will also be using it, and therefore will also help to keep it running… |
|
@ the people on this thread: I missed that this exists, so I made an another: #730 That one is a bigger change, which refactors the package and adds a The diff is more significant, but the end result is that this repository itself is easier to grok, IMO. And it actually reduces the overall code. I've volunteered to maintain the pnpm config, but if there are existing maintainers on this thread who would like to take a look, feedback is welcome. There's a fuller description in the PR body. Note: since there's no build step, you can also try it out directly by putting |
|
Closing in favor of #730 |
This PR adds support for the
pnpmpackage manager, which has been steadily growing in popularity.The maintainers don't want to add support for every package manager (see #251 (comment)), which is totally understandable — and therefore I don't expect this PR to be merged.
I'm just opening the PR here for visibility and to have a chance to gather feedback.
Note that I haven't added appropriate top-level unit tests for
pnpmyet.cc @zkochan depending on what the
npmaintainers decide, would it possibly make sense to move this fork under thepnpmorganization? I haven't published it to NPM yet.