Adds a warning when using npm through Corepack #407
Conversation
|
Alternatively, perhaps we could just extend the README to remove the npm references, explain there that supporting npm is a non-goal and provided on a best effort basis, and perhaps add an additional message on the prompt line to mention it there as well. Edit: That's what I did, it feels a less risky change. |
arcanis
changed the title
There was a problem hiding this comment.
One of Corepack's goal is to provide a levelled playing field, having npm being a special case seems to go against this goal. Also, if users want to use npm through Corepack, it should be their right to do so. npm can a warning on their side if they feel like it, it feels out of place for Corepack to add such warning.
|
|
||
| ### Official npm integration | ||
|
|
||
| Official npm integration is a non-goal at this time. The npm team made it clear they don't wish npm to be distributed through Corepack (some of their concerns are listed [here](https://github.com/nodejs/node/issues/50963#issuecomment-1862957925)). Corepack focuses on the use case from officially supported package managers. |
There was a problem hiding this comment.
That's no longer true since we added support for HTTP references, the goal is very much to help user run any package manager.
There was a problem hiding this comment.
Agreed. This warning might need to apply to pnpm and bun depending on how you define "officially supported" by their team. Its best to leave it out.
The log "Corepack is about to download" should be enough to differentiate Corepack usage vs calling directly.
| @@ -3,7 +3,7 @@ | |||
| Corepack is a zero-runtime-dependency Node.js script that acts as a bridge | |||
| between Node.js projects and the package managers they are intended to be used | |||
| with during development. In practical terms, **Corepack lets you use Yarn, npm, | |||
| and pnpm without having to install them**. | |||
| and other supported package managers without having to install them**. | |||
There was a problem hiding this comment.
Why did we remove pnpm from this list?
There was a problem hiding this comment.
Mistake; I meant to remove npm and leave pnpm.
|
|
||
| ### Official npm integration | ||
|
|
||
| Official npm integration is a non-goal at this time. The npm team made it clear they don't wish npm to be distributed through Corepack (some of their concerns are listed [here](https://github.com/nodejs/node/issues/50963#issuecomment-1862957925)). Corepack focuses on the use case from officially supported package managers. |
There was a problem hiding this comment.
No need to repeat the header of this section
| Official npm integration is a non-goal at this time. The npm team made it clear they don't wish npm to be distributed through Corepack (some of their concerns are listed [here](https://github.com/nodejs/node/issues/50963#issuecomment-1862957925)). Corepack focuses on the use case from officially supported package managers. | |
| The npm team made it clear they don't wish npm to be distributed through Corepack (some of their concerns are listed [here](https://github.com/nodejs/node/issues/50963#issuecomment-1862957925)). Corepack focuses on the use case from officially supported package managers. |
Where is this defined? |
The Corepack team always made it clear that enabling npm support in Corepack was contingent on the npm team being supportive of this effort.
The npm team mentioned in nodejs/node#50963 (comment) that they are welcoming of other package managers being shipped with Node througn Corepack, but don't wish npm to be distributed this way. That's entirely their right, we made sure that this project wouldn't have any impact on npm unless they wished it.
Still, since some people are getting fired up about what could have been but won't be, perhaps we should go one step further. This PR removes any mention of npm support from the README (adding it as a non-goal instead), and adds an additional warning to the prompt when npm would otherwise be downloaded.