-
Notifications
You must be signed in to change notification settings - Fork 29.6k
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
enable corepack by default #50963
Comments
I don't think we can enable Corepack depending on the project, either Corepack What I would like to see added to Corepack would be some kind of integrity mecanism so Corepack can safely download newer versions of package managers from the internet (currently, it only relies on HTTPS, which is OK but not great for our use case). This is tracked at nodejs/corepack#10. There has been little to no progress on that issue because it requires coordinate work between Corepack maintainers and package manager maintainers. If we're OK moving forward without it, I don't think there's any blocker for enabling Corepack by default. Before Corepack is enabled by default, we would need to get consensus on the following:
/cc @nodejs/corepack @nodejs/tsc |
I want to call out that the npm team is still not in support of this. It is not how we want npm to be distributed. We can discuss this more at length if needed, but wanted to call this out. |
Why does npm not want this? |
Adding tsc-agenda to understand where we are with corepack and progress on it. |
TL;DR I support unflagging corepack and any package manager that wants to integrate. npm does not wish to integrate for a variety of reasons and we don't want to be forced into supporting this pattern. I wouldn't block any work to enable corepack by default in Node.js for other package managers, but I do object to it being used for npm, and for any sort of npm support being on by default. My request would be that if corepack is enabled by default that npm support remain behind an additional flag or command that would be opt-in for developers. If developers want a flow that includes corepack I personally think they should choose a different package manager developed and designed to use this pattern such as yarn. This is the beauty of having an ecosystem of tools, and I don't think that npm should be forced to use a pattern it is not designed to utilize. For the default npm use case I view using corepack to be a regression in a variety of ways. This could be improved over time but would require investment from the npm team that we are not willing to make as we have other priorities we need to focus on. A couple of the issues I have with corepack when it comes to npm include:
The introduces extra indirection / lack of clarity as to what version of the package manager is being used and where on disk it is if you need to debug or review source. It also introduces extra potential error cases / edge cases as you are consuming a package manager in a way it was not designed to be used. Perhaps for yarn, which is being developed with corepack in mind, this is fine... but for npm it is being designed / developed with the assumption that it lives on disk in a certain location. This introduces an extra support burden to our team when things don't work as expected and also introduces new "heisenbugs". My preference is that we explicitly don't support this pattern nor enable it by default and by association explicitly don't support problems developers would have with it.
While it is true that we pin the package manager within a version of Node.js, we do security updates to Node.js when the embedded version of npm is known to have security vulnerabilities. We extensively test to ensure there are no regressions and publish updates. With the pattern of corepack projects that opt to lock to a specific version of the package manager do not get updates even if the version in Node.js, or the default version in Corepack, updates. This is not too different from how a package-lock works, but there exists no ecosystem tooling today to warn developers when their pinned package manager is vulnerable (e.g. dependabot). This is imho a regression in security posture. We do significant work to guarantee consistency with npm moving forward and in general want people on the latest version ASAP, pinning the package manager in the project is an anti-pattern in this scenario.
Currently with the embedded version of npm there are a variety of tests, including CITGM, which test the version of npm that ships with Node.js. Corepack decouples this by updating the package managers internally and then updating the version of corepack in Node.js. We can develop process around this to ensure that it is as robust as what we do today, but to the best of my knowledge this hasn't been done yet.
First off, I recognize this is a bit of double speak... I point out the challenge in inconsistent versioning while also claiming pinned versions to be a problem. As a meta point I think this is part of the problem with Corepack for npm, it introduces a bunch of complexity and inconsistency w/o significantly improving the status quo. Currently core pack has npm 10.2.3 hard coded in it's config. This arguably undermines one of the biggest benefits that was originally claimed for corepack when it shipped, which was that the updating of package managers would be decouple from core. This adds an extra step where we need to update the package managers independently in corepack and then update corepack in Node.js. This is an additional step and additional work for our team and the Node.js team to maintain. This also couple the update of a package manager, with the update of other package managers and corepack itself. The most recent update PR has been open for over a month without review.
If we were to "unship" Node.js and only ship Corepack we would require dynamically fetching npm over the network in order to get started. Any project that has a pinned version of npm w/o that version being cached also needs to fetch something over the network to get started. Not only does this create additional friction in developer workflow it introduces an extra place for a MITM attack in which the package manager could be replaced with something compromised.
In summary I see a whole bunch of additional complexity to using npm, to maintaining npm in core, and additional work for the npm to support corepack without significant benefit to the overall npm experience. Arguably it offers a worse experience for npm developers based on the way in which the npm team has designed npm to work. Once again I want to call out that if other package managers are designing and developing their package managers to deeply integrate and work with corepack, that's awesome! For npm this does not align with our design philosphy and enabling it by default would be going against the wishes of the team. |
One that point, since nodejs/corepack#134 has landed, the pinned version hardly matters (it's only used if |
There are multiple data points from the community in nodejs/corepack#104 to make corepack stable. |
That should be fixed already; nodejs/corepack#276. |
My 2 cents is that corepack simplify the life of most developers that prefer to use yarn or pnpm. I also think there is value in shipping the reference implementation within Node.js (npm). Last but not least, most devs do not specify their packageManager field, making reproducible build harder when used in Dockerfiles and build steps. This poses a long term danger, mostly because in a few years the newer versions might not be compatible anymore. |
This argument feels a bit shallow when the > corepack npm@8 install
added …
…
npm notice
npm notice New major version of npm available! 8.18.0 -> 10.2.5
npm notice Changelog: https://github.com/npm/cli/releases/tag/v10.2.5
npm notice Run npm install -g [email protected] to update!
npm notice And AFAIK |
How? There is no hash for the actual archive. |
Not sure what you mean, I’m talking about a hash for the actual archive: when using Corepack it is recommended to specify the hash alongside the version. When you use |
Ah, I've learned something. That's not how I've seen folks use this. |
Watching the TSC meeting recording, here are the scenarii that have been mentionned:
Here are the upsides/downsides I can think of:
I think ideally I would go with 2., as it seems to be the fairest to me, and I tend to think the breakage would be manageable – in fact, it would probably be transparent to most folks. I'm happy to hear divergent opinions, or to be corrected if I missed something :) Footnotes
|
Can you describe what this option would look like to end users? Both for someone installing Node fresh on a new machine as well as someone upgrading from Node 21. For example, what would they see after installation when they run Tangentially related, we should consider whether we want to keep bundling Yarn 1 in our Docker image:
|
Thank you for considering Docker users as well. I think, when making a decision about corepack, it's important to take into account all official ways in which Node.js is delivered to users. Otherwise this may lead to fragmentation/different behavior. Regarding Yarn in the Docker image, there are many related issues/PRs on the docker-node repo, most recently this one (which also has links to other ones): nodejs/docker-node#1979 |
Assuming they run the command on a folder with a Corepack jumper binaries will look into the |
As was raised earlier I think there are two distinct, but related, conversations happening here, and I'd like to advocate for splitting the discussion.
When Corepack was originally created it was to unblock objections to adding yarn to Node.js, and the eventual vote to include corepack over adding yarn was explicitly framed to not make a decision regarding the bundling of npm. This specific conversation has been opened to discuss unflagging Corepack, and if that is all we plan to do I have no objections. I strongly believe that we should improve access to the top CLI clients, but I also strongly believe that we shouldn't force specific clients into using Corepack. If the decision to unflag corepack is dependent on a decision regarding unbundling npm, we should reach consensus on that first before pursuing the decision on corepack further. Conflating the two will only make discussion confusing IMHO. It does not feel in good spirits to expand the scope of this discussion to a much larger and tougher decision regarding unbundling npm, and with the npm team (and myself) having expressed on multiple occasions that we have issues with the technical decisions of Corepack and do not want to be forced to support it, it is dissapointing that it feels we are being forced into making a decision here. If yarn + pnpm want to use corepack, rather than directly bundling, that should be their perogative to do so. If npm does not want to be shipped with corepack we should have the option to do so. If we want to discuss the potential of Node.js unbundling npm, making corepack the only option to being distributed with node.js for npm, this should happen on it's own merits as a separate discussion. As the discussion is happening here I will respond to it here, but I do want to advocate strongly that we separate the two discussions. If those advocating for the inclusion of corepack do not support unflagging it if npm is not enabled by default / unbundled then I think we should block a decision here until we discuss the larger matter of unbundling npm. @anonrig mentioned 2 primary reasons for unbundling npm in the recent TSC call. The appearance of a competitive advantage for npm, and bundle size. I'd like to speak to those.
I want to advocate clearly for the inclusion of both yarn + pnpm in the source tree of Node.js. They should be bundled and included in the way that is most preferred by those teams. While I personally think using corepack is a mistake, and can get into the specifics if desired, I also don't think it is my place to tell other projects how to distribute their source. If corepack is the way those teams prefer it then that's how it should be done. When a developer installs Node.js on their machine they should be able to run This could start to get into a slippery slope argument regarding what does / does not get bundled in Node.js. I think we can agree on some fairly reasonable guidelines and the npm team is willing to share metrics to help make a data informed decision. Currently market share based on requests to npm is:
The remaining 1.05% are almost entirely from npm-registry-fetch and pacote... two internals of npm used by other clients like lerna. If we look at semver major usage the top 10 clients are:
Yarn 1 is responsible for close to 25% of all registry traffic by itself and has been the number client for years. Clearly it has accomplished this despite not being the default Node.js client. I 100% support anything we can do to make client access easier, to reduce steps to getting started, to making the experience better, and to levelling the playing field. I do not support forcing the adoption of corepack to teams that do not want to use it
I would like to challenge that reducing bundle size is a core value or even goal of the Node.js project at this time comparing Node.js 16 vs 18 vs 20 vs 22 we can see the distribution size continuing to grow over time with version 16 coming at 98.4 MB and version 21 coming in at 163.9 MB of which npm is ~10mb and corepack is ~2.4 MB. Could we optimize this, sure, but this seems like a micro-optimization that results in a poorer developer experience. There is no strategic initative focusing on reducing bundle size. Bundle size also doesn't come up in the Technical Values of the project, but #1 is developer experience and #2 is stability IMHO bundle size is not a reasonable technical argument to unbundle based on the current goals and history of the project To summarize one more time
|
I don't think vendoring Yarn and PNPM into Node.js like npm is is a realistic option (more security issues, more bundle size, and would likely not fit well with our LTS policy). I also don't think it would make much sense to discuss unbundling npm because of the ecosystem impact. If the Release Team wants that to happen (I think they are the ones who are the more likely to be affected by that decision), we could discuss it, but that doesn't seem very productive to go this route. (It'd be like Node.js and npm calling each other bluff until one project caves, or everyone loses). That being said, we can certainly go the 3. road (which is the most likely to get consensus it seems), and reconsider at a later time if we feel the need to unbundle npm in the future. |
What are the npm team’s concerns with Corepack? (A link is fine; my apologies for not following along with this topic.) I do like the idea of a package manager being automatically downloaded on first use; that spares us the trouble of needing to bundle it, and also means that we don’t need to take responsibility for the package manager’s security issues. If there’s some way to achieve this that the npm team is happy with, that would seem like a win for both sides. (Even better if it’s a solution that Yarn and pnpm are also on board with.) If Corepack’s current design doesn’t work, is there an alternative design that could? Edit: I assume the objections are here? #50963 (comment) |
a variety of concerns were listed earlier in the thread |
By going through the latest comments in this thread, it looks like we're reaching a consensus on There are multiple data points shared in nodejs/corepack#104 on how moving corepack out of experimental will benefit the Node.js community. |
@Zelgadis87 I agree that improving the usability and developer experience of the ecosystem should be the goal of us as maintainers! We have many users to support, including those who rely on how Node.js currently works - be that developers with CI that relies on how Node.js works currently, or developers who are learning form the past ~decade of content that exists about the platform. I don't think anyone's explicitly objecting here to improving the package manager experience. Literally, earlier in this thread, someone partially responsible for npm unequivocally advocated for the inclusion of the other major package mangers how they see fit. Generally, that seems to be the best strategy - and the one that most people seem to agree with? Start with the status quo and improve the experience from there. The debate here at this point is largely around whether or not Corepack is the best solution for improving the experience vs. other options. IMO it's not the best possible solution, but also IMO this is a subjective problem which results in subjectivity in what "best" is. |
Any Node.js Collaborator can propose a feature or change. If no one objects, it will land. If someone objects it goes to a TSC vote. It might be surprising that we consider and value every change that it is being proposed... but that's how we guarantee all voices are heard. So, yes, we take this seriously, as all changes from collaborators. Ultimately this one will land on the TSC table with the form of a vote. |
@Jarred-Sumner what solution would you adopt? |
Hi folks, this has seemed to spark some discussions in the OpenJS Package Metadata Interoperability collaboration space. We have a meeting coming up on March 5th and can spend the hour discussing things. Please join if you're interested. Here is the OpenJS calendar link: https://calendar.google.com/calendar/u/0/embed?src=linuxfoundation.org_fuop4ufv766f9avc517ujs4i0g@group.calendar.google.com |
I like the Unix philosophy: Write programs that do one thing and do it well. So yes, create a separate nodejs package and a separate npm package. Bundling them together isn't a good thing. That being said, most people don't know what Corepack is. So try to be backwards compatible for now, would be my advise. I do agree with Jarred, it's very kind that NPM allowed third party clients to interact with NPM package server for free (as in free buns). And this should not be undervalued. At the same time, to be honest, did we really had an alternative solution? |
Another vote to unbundle npm from node. Bundling is almost never a good idea for several reasons:
It's surprising that bundling even more software (via Corepack) is being suggested, IMHO that's going in the wrong direction. In Java land, it would be like the JRE first being bundled with Maven, then it being suggested that it also gets bundled with Ant and Gradle as well. A runtime and a package manager are two different things that do two different jobs - they should be treated as such. |
This is quite a long thread now. Can some summarise the current technical objections to enable corepack by default? @aduh95 @GeoffreyBooth? Is it mainly the security parts atm? |
me vote for having npm not bundled, but here's another perspective: rust: rust is installed with rustup, rustup installs cargo (package manager, which is also responsible for orchestrating a build) I find myself rarely used the npm package manager, instead for something like building minimal images. But though I vote for having corepack managing npm for it's cleaness, there're a perspective where many modern languages maybe, yes, violates the unix philosophy, yet still they're there... and having npm not bundled would break things that assumes that npm is shiped with node |
For anyone visiting this issue, the status as of 2024-04-26 is an Open TSC Vote tracked at nodejs/TSC#1527 |
oops nm. came here from a link in a pnpm discussion... |
That's not the point of this discussion, and regardless, corepark already supports pnpm |
What is the problem this feature will solve?
we try to use corepack and "packageManager" to force developers use the same pnpm version, but developers always forget to enable corepack(because it's turned off by default and when we switch node version it's turned off again) which makes packageManager not working at all
What is the feature you are proposing to solve the problem?
enable corepack by default if package.json contains "packageManager".
What alternatives have you considered?
we currently use "engine.pnpm" field to check whether developers use same pnpm version but engine field is annoying when we deals with tons of projects with different version
The text was updated successfully, but these errors were encountered: