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

Project Status (experimental to stable?) #104

Open
styfle opened this issue May 3, 2022 · 27 comments
Open

Project Status (experimental to stable?) #104

styfle opened this issue May 3, 2022 · 27 comments

Comments

@styfle
Copy link
Member

styfle commented May 3, 2022

I can't find any details on the current project status such as anything that needs to be solved before going stable.

  1. Are there any outstanding blockers?
  2. Are there known breaking changes coming up?
  3. Do we need to get npm working to be considered stable?
  4. Will a stable release remove the need to run corepack enable or will that always be a requirement?
@andersk
Copy link

andersk commented May 7, 2022

I’d hope for security that corepack starts checking integrity hashes before it’s enabled by default.

(Edit: This was briefly fixed, but it was then reversed by #134 and remains a problem.)

@trivikr
Copy link
Member

trivikr commented May 17, 2022

Visiting this GitHub issue from twitter thread.

I'd asked on Yarn Discord in Jan'22 if we should propose to make it stable in Node.js 18. There was no follow-up though.

Some useful links:

@arcanis has been the champion and lead author for corepack, along with other maintainers of yarn.
They're currently busy with the launch of yarn v4 in yarnpkg/berry#3591

I think we should revive this GitHub issue after yarn v4 release.

@arcanis
Copy link
Contributor

arcanis commented May 17, 2022

Are there known breaking changes coming up?

Mostly just #93 - many Node contributors expressed interest in seeing Corepack have a slightly more dynamic behavior, so that it stay up-to-date with minor/patch versions from the package managers (look at the thread for details, but in short it's currently painful for Node to have to make new releases each time npm has a security update).

It makes dealing with #37 a little harder though. Perhaps package managers releases could be signed instead, although it requires infra and can be be difficult to do for package managers that aren't distributed as a single JS bundle (I believe only Yarn is).

Are there any outstanding blockers?

Apart from the one described above, I'd personally like to have feedback from one or more cloud provider confirming that Corepack has all the tools they need to build a proper integration that doesn't break their customers' setup. That's something Vercel can certainly help with!

Do we need to get npm working to be considered stable?

Corepack has value for Yarn and pnpm users, and it'd be too bad if we were blocked going forward by something we have no control over, so in my opinion I'd tend to say it's not needed, although it would be nice to have.

Note that technically Corepack already supports npm, it's just not part of the default distribution until they decide to enable it. So when that happens it will be fairly fast and won't require much engineering work.

Will a stable release remove the need to run corepack enable or will that always be a requirement?

My hope (and goal when building this project) is that corepack enable won't be needed.

@styfle
Copy link
Member Author

styfle commented Jul 14, 2022

I'd personally like to have feedback from one or more cloud provider

We announced support today https://twitter.com/vercel_changes/status/1547581626279792640

I'll relay any feedback from our customers as they begin adoption 👍

In my own testing, the biggest problem I see today is the "Usage Error: This project is configured to use ". This happens when I run npm i -g vercel@latest in a project that is configured to use yarn or pnpm. It seems a little too strict because the global flag is not scoped to my current project. Similarly, vercel dev will shell out to npm or npx and corepack being enabled could block it from working properly.

Update: I created #157

@styfle
Copy link
Member Author

styfle commented Sep 29, 2022

Checking in here again. I can say that all outstanding issues are complete and everyone who tries out corepack is very happy with it. Its solving real problems. For example, npm changed behavior of peer dependencies in npm install in a minor version (8.6.0) suddenly causing builds to fail, and corepack allowed users to continue using the old behavior

That being said, the one thing missing today is default support for npm when I run corepack enable. However, corepack enable npm is a sufficient workaround and has proved it works well today already. I agree that this shouldn't be a blocker with going to "stable" status.

@aduh95
Copy link
Contributor

aduh95 commented Oct 9, 2022

Checking in here again. I can say that all outstanding issues are complete and everyone who tries out corepack is very happy with it. Its solving real problems. For example, npm changed behavior of peer dependencies in npm install in a minor version (8.6.0) suddenly causing builds to fail, and corepack allowed users to continue using the old behavior

That's really good to hear!

That being said, the one thing missing today is default support for npm when I run corepack enable. However, corepack enable npm is a sufficient workaround and has proved it works well today already. I agree that this shouldn't be a blocker with going to "stable" status.

I would assume that, on the contrary, Corepack being experimental would be a blocker to switch to using it for providing npm binaries with Node.js installation (instead of vendoring npm in the node repository like we do today).
One additional thing I would personally like to see before calling Corepack stable is the ability for package manager authors to sign their releases, as a security feature. Maybe it's not a blocker though.

@styfle
Copy link
Member Author

styfle commented Oct 11, 2022

I wouldn't consider signing releases a blocker to going stable. It would be a nice feature though.

Correct me if I'm wrong, but I don't think npm i -g [email protected] has signed releases either?

Unless the { "packageManager": "[email protected]" } config doesn't pull from the npm registry like I expected? 🤔

@aduh95
Copy link
Contributor

aduh95 commented Oct 11, 2022

I wouldn't consider signing releases a blocker to going stable. It would be a nice feature though.

When discussing with other devs, I've heard a few concerns regarding the decrease of security if Node.js doesn't ship npm anymore and instead expect users to download the latest version which could have been tampered with. IMO when we call Corepack stable, the next logical step is to stop bundling npm by default with Node.js in favor of Corepack, and the lack of signature verification might be a problem on that aspect.

Correct me if I'm wrong, but I don't think npm i -g [email protected] has signed releases either?
Unless the { "packageManager": "[email protected]" } config doesn't pull from the npm registry like I expected? 🤔

npm, pnpm, and Yarn v1.x are pulled from the npm registry, Yarn Berry is pulled from their own server. But it doesn't have anything to do with signed releases, right?

@styfle
Copy link
Member Author

styfle commented Oct 11, 2022

@yarinsa
Copy link

yarinsa commented Dec 29, 2022

@styfle What prevents corepack to be stable in Node20?

@arcanis
Copy link
Contributor

arcanis commented Dec 29, 2022

Mostly time. On my side I'd like to add signing support to Yarn before asking for it to become stable, but I'm currently focusing on another Node PR.

Perhaps in the meantime we can start to gather numbers / quotes demonstrating Corepack's usefulness in the wild ? I don't know what else would help the TSC make a decision, when comes the time.

@ljharb
Copy link
Member

ljharb commented Dec 29, 2022

I believe there's a pretty major licensing issue that needs to be resolved as well.

@arcanis
Copy link
Contributor

arcanis commented Dec 29, 2022

What are you referring to? That doesn't ring a bell.

@ljharb
Copy link
Member

ljharb commented Dec 29, 2022

My understanding is that corepack's npm "jumper" thing violates npm's license. I'm under the impression it's been brought to your attention in the past, and I'm pretty sure folks on the TSC are aware of it as well.

@aduh95
Copy link
Contributor

aduh95 commented Dec 29, 2022

I for one am not aware of it, do you have a link to a discussion that explains what's the issue?

@arcanis
Copy link
Contributor

arcanis commented Dec 29, 2022

My understanding is that corepack's npm "jumper" thing violates npm's license. I'm under the impression it's been brought to your attention in the past, and I'm pretty sure folks on the TSC are aware of it as well.

I can't find any thread here or any comment in the original PRs, and I don't have any recollection of such issue. It seems a little strange considering that npm isn't officially covered by Corepack at the moment; if you have more pointers it'd be useful to redirect them in a separate thread.

@ljharb
Copy link
Member

ljharb commented Dec 29, 2022

I'll try to track something down that mentions it.

@styfle
Copy link
Member Author

styfle commented Dec 30, 2022

Does the “jumper” modify the npm CLI source code or just invoke it?

IANAL, but it sounds like the license doesn’t apply if the jumper is just invoking

(9) Works (including, but not limited to, modules and scripts) that
merely extend or make use of the Package, do not, by themselves, cause
the Package to be a Modified Version. In addition, such works are not
considered parts of the Package itself, and are not subject to the
terms of this license.

https://github.com/npm/cli/blob/latest/LICENSE

@joshwlewis
Copy link

joshwlewis commented Feb 1, 2023

Apart from the one described above, I'd personally like to have feedback from one or more cloud provider confirming that Corepack has all the tools they need to build a proper integration that doesn't break their customers' setup.

@arcanis I run Node.js support for Heroku. I've recently built a buildpack integration for Corepack (here). I'm pretty eager to expand support for corepack.

The areas where I have the most concern is around package manager verification. It looks like #37 is a great start, but for folks wishing to pin to a non-default package manager version, the user has to determine and specify the checksum. Users pinning to a non-default version without specifying a checksum might be surprised that the package manager installed by corepack wasn't verified in any way. I would hope to see checksum verification as mandatory prior to moving ahead. I'd love to see #10 happen.

@aduh95
Copy link
Contributor

aduh95 commented Feb 1, 2023

I would hope to see checksum verification as mandatory prior to moving ahead.

I don't think that can ever happen, we would need to have a trusted source for the checksum hashes, and AFAIK that is not something we can have without trusting HTTPS and a remove server (so no better than what we are doing now), or bundle a list of accepted hashes within Corepack which would make it impossible for package manager authors to release a fix without also forcing an update upon Corepack (that's not acceptable, Corepack should let you use the most patched version always).
Am I missing something? To me it seems that #10 is the only reasonable approach.

@joshwlewis
Copy link

we would need to have a trusted source for the checksum hashes, and AFAIK that is not something we can have without trusting HTTPS and a remote server (so no better than what we are doing now)

Ah, fair point. I think #10 does make more sense here, then.

@boneskull
Copy link

@ljharb Did you ever find anything about this licensing issue?

@trivikr
Copy link
Member

trivikr commented Oct 24, 2023

Revisiting this request to make corepack stable, as both yarn and pnpm recommend their users to use corepack:

Some downstream dependencies, like GitHub Action to setup node, still do not support corepack actions/setup-node#651
It would be helpful for yarn/pnpm users on Node.js projects if corepack is enabled by default in Node.js.

@anonrig
Copy link
Member

anonrig commented Jan 11, 2024

Node.js TSC in the process of discussing and taking action for the status of the project. For more information, I recommend reading the meeting notes from January 10, 2024. nodejs/TSC#1490

@styfle
Copy link
Member Author

styfle commented Jan 11, 2024

The discussion is continuing in this issue: nodejs/node#50963

Me watching from the sidelines: https://media2.giphy.com/media/FibBZ3bFGGcoXCB1kd/giphy.gif

@benjamingr
Copy link
Member

Personal opinion (does not represent my employer):

  • +1 for corepack out of experimental to alleviate concerns
  • +1 for making it clear that npm is not owned by Node.js and making it clear package managers in corepack and tools still use the npm registry by default
  • +1 for keeping it opt in (corepack enable)

@aduh95
Copy link
Contributor

aduh95 commented Jan 17, 2024

  • making it clear package managers in corepack and tools still use the npm registry by default

I don't think we can guarantee that, and also I think package managers should be free to pick the package registry they want – I mean it is the case atm, Yarn and PNPM both default to the public npm registry, but future additions to Corepack might want to supply their own registry or whatever. IMO it's not reasonable to expect Corepack maintainers to audit the candidate to make any guarantees on what the software is doing.

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

No branches or pull requests