Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Removes npm from the possible options #670

Closed
wants to merge 3 commits into from
Closed

Removes npm from the possible options #670

wants to merge 3 commits into from

Conversation

arcanis
Copy link
Contributor

@arcanis arcanis commented Mar 6, 2025

I never intended for Corepack to offer npm as an option without its maintainers enabling it themselves. It was supported through an explicit corepack enable npm mostly to make sure that whatever design we picked would keep npm in mind, so that in the event they'd one day be interested to join us it'd just be a flip of a switch. It wasn't a requirement (npm users already have their nice developer experience, my goal was to improve Yarn's and pnpm's devx at no cost for anyone).

To clarify that and make it super duper clear that making Corepack stable won't impact npm users in any shape or form, I propose this PR to remove npm from the possible options. It's no longer possible to activate it using corepack enable npm. I erased any mention of it, including from the tests.

Also, addressing my own comment:
#418 (comment)

get formal approval from someone from the actual npm team. As far as I'm aware you're working on a competing product, which makes you kind of the wrong person to make this request (part of why I didn't merge my own #407).

This was requested/suggested by TSC members unrelated to Yarn / pnpm. Since this is a Node.js project, I don't have objection to them requesting this change.

Closes #418

Copy link
Member

@BridgeAR BridgeAR left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

This seems to supersede and close #418

@styfle
Copy link
Member

styfle commented Mar 6, 2025

npm users already have their nice developer experience

I disagree. I dont think npm has any way to manage versions per working directory like you can do with corepack.

This was requested/suggested by TSC members unrelated to Yarn / pnpm

Where was this suggestion made, and by whom?

@arcanis
Copy link
Contributor Author

arcanis commented Mar 6, 2025

I disagree. I dont think npm has any way to manage versions per working directory like you can do with corepack.

Then they are the best people to lobby. Corepack wasn't intended to improve their devx. Don't get me wrong, I personally think it would be a good thing for npm to leverage Corepack, but it's not my castle and I don't think Yarn and pnpm users should suffer so much delay and uncertainty because of that.

Npm (and the Node TSC) will always have the ability to discuss eventual future plans through follow-ups, but they won't need me in that discussion 🙂

Copy link
Contributor

@aduh95 aduh95 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My previous concerns remain, I don’t think we gain anything from it, and unless the crurent npm maintainers are asking for this, I don’t think we should do it

@styfle
Copy link
Member

styfle commented Mar 7, 2025

I don't think Yarn and pnpm users should suffer so much delay and uncertainty because of that.

Can you elaborate? Why would those users suffer because of npm being available through corepack? 🤔

It would be great if you could share the context, perhaps linking to the TSC meeting notes?

@arcanis
Copy link
Contributor Author

arcanis commented Mar 7, 2025

Can you elaborate? Why would those users suffer because of npm being available through corepack? 🤔

I don't think I said that? What I did say/mean is that I share the opinion that the mere presence of npm in the Corepack debate is unfortunately making it radioactive. I believe it is in part why this proposal hasn't been going anywhere despite strong repeated support from the community.

I hope that by opening this PR it'll be clear that marking Corepack stable will not change anything to npm users - and perhaps quell some concerns.

@styfle
Copy link
Member

styfle commented Mar 8, 2025

the mere presence of npm in the Corepack debate is unfortunately making it radioactive.

Can you provide evidence of this? Who is debating and what are they saying specifically?

@darcyclarke darcyclarke mentioned this pull request Mar 11, 2025
@legobeat
Copy link

legobeat commented Mar 12, 2025

I hope that by opening this PR it'll be clear that marking Corepack stable will not change anything to npm users - and perhaps quell some concerns.

Corepack is bundled as part of Node.js distributions. It seems pretty obvious that this PR would impact NPM users in a breaking way if merged?

NPM maintainer opinions on the matter seems neither here nor there - shouldn't corepack users come at priority over pleasing such external stakeholders?

I believe it is in part why this proposal hasn't been going anywhere despite strong repeated support from the community.

By "this proposal", do you refer to dropping NPM support from corepack? From here it appears that if anything community sentiment is against and pushback has held it back?

@zanminkian
Copy link

zanminkian commented Mar 13, 2025

There 3 suggestions for Node and corepack team.

  1. Remove npm from node.
  2. Defaultly intercept npm command (just like pnpm and yarn) when enabling corepack.
  3. Remove npm from corepack and support devEngines like npm do. So that people can use correct package manager by using this field.

The 1st would be the best. If corepack decide not to intercept npm by default, I suggest removing npm too. Anyway, the 1st suggestion is the best, the 2nd one is second, and the 3rd one is acceptable unacceptable.

@legobeat
Copy link

legobeat commented Mar 13, 2025

There 3 suggestions for Node and corepack team.

Remove npm from node.
Defaultly intercept npm command (just like pnpm and yarn) when enabling corepack.
Remove npm from corepack and support devEngines like npm do. So that people can use correct package manager by using this field.

Just to clarify "Remove npm" in 1 and 3 are referring to removing different things, right? As npm is not actually part of node nor corepack themselves, per se. So to spell it out more verbosely what I think you mean here:

  1. Node.js: Stop bundling npm in official Node.js binary releases
    • Seems unrelated..? If PATH variable is consistently set and respected, having a global version alongside the ones managed by corepack should not be an issue. Same principle as for node/nvm, python/pip/venv, asdf, etc. Having different versions for user dev/build of runtimes and package managers compared to what may be globally installed on the system is normal. A major purpose of these environment- and system-managers is to not be restricted by presence of global installations.
  2. corepack: Defaultly intercept npm command (just like for pnpm and yarn) when enabling corepack.
  3. corepack: Remove all npm-related functionality from corepack
    • This PR
  4. Support devEngines like npm does.

Would that be accurate @zanminkian? (Broke apart 3 and 4 as they seem to be different points - or how would 4 depend on 3?)

@zanminkian
Copy link

If corepack has been removed from Node in the future (nodejs/node#51981), corepack should become an independent project who aims to improve development exprerience in using package managers. At that time point, will corepack support npm?

People need a tool to make life easier when using pacakge managers including npm, if corepack has been removed from Node.

Corepack is a manager of package managers. Why excluding npm? Because of license or law reasons?🤔️

@arcanis arcanis closed this Mar 19, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants