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

Proposal: Alternative to enabling Corepack by default #51931

Open
GeoffreyBooth opened this issue Feb 29, 2024 · 29 comments
Open

Proposal: Alternative to enabling Corepack by default #51931

GeoffreyBooth opened this issue Feb 29, 2024 · 29 comments
Labels
experimental Issues and PRs related to experimental features. feature request Issues that request new features to be added to Node.js.

Comments

@GeoffreyBooth
Copy link
Member

This is a sketch of a proposal, so please don’t pepper me with questions about how it would work; I don’t know. It’s an early idea that I want to suggest to see if there’s interest.

So today, when you run corepack enable, a file named yarn is written to a path like /usr/local/bin/yarn. This is a #!/usr/bin/env node script that tells Corepack to run Yarn. Presumably #51886 will cause this file to exist upon Node installation, without the need to run corepack enable to create it. All well and good.

What if instead, we create a new #!/usr/bin/env node script that lives at the same path /usr/local/bin/yarn. This file would exist upon Node installation, and in this Node installation there’s no bundled Corepack (bear with me). When run, this new yarn script would do the following:

  1. Prompt the user “Do you want to download and install Yarn?” and if yes:
  2. npm install --global corepack
  3. corepack enable yarn
  4. Runs Yarn with the arguments originally passed to the script

From the user’s perspective, this is the same DX as if #51886 lands; though it takes slightly longer, because instead of downloading and installing just Yarn it’s downloading and installing both Corepack and Yarn.

Besides shrinking the Node bundle size slightly, the benefit of doing this is to get the Node project off the hook for the maintenance burden and security burden of Corepack. We would be responsible for this handful-of-lines download-and-install script, but that’s it. Corepack could be gifted to the Yarn org and they could become responsible for it.

This also becomes a pattern that we could use for any other CLI tools we may want to provide easy access to, but not bundle, in the future—even npm (downloaded and installed directly, not via Corepack). I don’t know what we might consider for such treatment other than Yarn and pnpm, but the capability would be there.

@nodejs/tsc @nodejs/corepack @nodejs/npm @nodejs/package-maintenance

@GeoffreyBooth GeoffreyBooth added feature request Issues that request new features to be added to Node.js. experimental Issues and PRs related to experimental features. labels Feb 29, 2024
@wesleytodd
Copy link
Member

wesleytodd commented Feb 29, 2024

Just a first quick take on this, why npm install corepack and not npm install yarn? I agree with the goals of this and generally that this is a light improvement on the existing corepack, but I stand by the stuff corepack is providing is not necessary at all since we already ship a package manager.

@GeoffreyBooth
Copy link
Member Author

Just a first quick take on this, why npm install corepack and not npm install yarn?

Because Yarn wishes to be installed via Corepack. npm install yarn installs Yarn 1.x.

I don’t know what pnpm’s preference would be. If they prefer direct npm install pnpm, that works. I think it would be up to whatever project we “bundle” this installer script for how they want the script to behave.

@wesleytodd
Copy link
Member

wesleytodd commented Feb 29, 2024

Because Yarn wishes to be installed via Corepack.

The yarn package could do npx corepack enable yarn as a postinstall to setup the binary instead of relying on npm bin scripts? I think this is a problem which isn't a real problem for the node project to solve, but just presenting alternatives.

I think it would be up to whatever project we “bundle” this installer script for how they want the script to behave.

IMO this should be avoided. Since we still don't have clarity on what would then be included in this new shim thing and what the requirements would be to apply for inclusion I think it just brings back some of the problems corepack introduced.

@anonrig
Copy link
Member

anonrig commented Feb 29, 2024

This proposal depends on npm to install corepack, and therefore requires npm to be existent. For future development, this feels like shooting our foot (one more time).

@wesleytodd
Copy link
Member

wesleytodd commented Mar 1, 2024

This proposal depends on npm

I think that this proposal would assume something like this lands (or at least assumes the status quo): #51918

Also, node ships a package manager so I think it is expected that node uses that package manager to install packages. (I am only being as silly here as this conversation necessitates I swear).

@GeoffreyBooth
Copy link
Member Author

If we remove npm, we just refactor this to download via fetch. Or it downloads via fetch in the first place. The implementation details are irrelevant.

The point is, is the use case of “As a user, I want to run Yarn without needing to install it first” one that we’re aiming to support? If so, this is a much lighter solution than shipping all of Corepack or all of Yarn.

@wraithgar
Copy link

we just refactor this to download via fetch

Unless the package manager in question bundles its dependencies, the only way for you to do this is to in fact implement your own package manager.

Package managers are a kind of unique snowflake because there has to be some point at which they can self-start. This is why npm has always bundled its own dependencies and why there is an install.sh file available via the npm website that lets folks install npm into a system with a missing or broken package manager.

Unless you're willing to solve that problem and take on the burden of maintaining your own package manager, or declaring bundled dependencies as a prerequisite to inclusion in this new process, it's going to be "turtles all the way down" as you will need a package manager to install your package manager.

npm install yarn installs Yarn 1.x.

This is because they have set 1.x as latest. The fact that corepack does something different breaks the convention of how the npm registry has always operated.

I was not around where and when corepack was being conceived or implemented, so I don't 100% understand what problems its trying to solve. It appears that there is a desire in at least part of the community to support having multiple versions of multiple package managers available at runtime. If that's the case perhaps that is an issue that can be solved specifically and intentionally, instead of having node get into the business of maintaining their own package manager. The current discussions around what is tentatively calling devEngines are promising in this regard.

@GeoffreyBooth
Copy link
Member Author

GeoffreyBooth commented Mar 1, 2024

I was not around where and when corepack was being conceived or implemented, so I don’t 100% understand what problems its trying to solve.

It was designed to solve the “problems from npm being the only package manager shipped by default” per https://github.com/nodejs/corepack/blob/967e2666b57f4ba33fb03397dbd6089251fc0503/DESIGN.md. A few days ago they deleted this design document without replacing it, and I’m unaware of any other documentation of Corepack’s intended goals or use cases, so it’s a bit unclear what problems Corepack is intending to solve. The TLDR is that it seemed to be originally intended as a way to remove npm from Node’s distribution, but over the years it has found its most usage as a version manager for pnpm to prevent pnpm from corrupting its own lockfile.

This proposal isn’t intended to be a way to unbundle npm, as I don’t think that that’s a goal that the project wishes to pursue at this time; however I think that if we change our minds on that, then any solution would involve the general approach described here. Yes it would presumably be complicated, and maybe use install.sh or something similar, but those are details we don’t need to work out unless and until we decide that removing npm is something we want to achieve. I don’t think any unbundling of npm from Node would involve Corepack, as the npm team is adamant that they don’t want npm installed by Corepack; so we could take the first step of adding these Yarn and pnpm installer scripts to unbundle Corepack and leave the question of npm for another day.

@wraithgar
Copy link

Thanks @GeoffreyBooth, this is not an easy conversation to be late into joining. I appreciate your help in fielding my basic questions.

I don't have much to add then at this point. If there are specific questions you feel npm can answer please ping me, or bring it up over in the package metadata interop repo (if applicable)

@aduh95
Copy link
Contributor

aduh95 commented Mar 1, 2024

declaring bundled dependencies as a prerequisite to inclusion in this new process

Bundled dependencies are a prerequisite for working with Corepack, so it wouldn't be too bad to have the same requirement.

@styfle
Copy link
Member

styfle commented Mar 1, 2024

This proposal depends on npm to install corepack, and therefore requires npm to be existent. For future development, this feels like shooting our foot (one more time).

Agreed. Something doesn't seem right.

The new script could potentially fetch and untar from the registry without invoking npm, but then you've basically re-implemented what corepack already does.

@styfle
Copy link
Member

styfle commented Mar 1, 2024

Also consider how this would work in CI - you don't want to prompt in that case.

@wesleytodd
Copy link
Member

wesleytodd commented Mar 1, 2024

Something doesn't seem right.

I would be interested if folks could dig into this point. I think it is valid to feel this way, but to make decisions we need more than hand wavy explanations. Why do people feel this way? What is not right about using a package manager to install packages (even if those packages are other package managers)?

@styfle
Copy link
Member

styfle commented Mar 1, 2024

Because it creates a dependency on npm that wasn't there before

@wesleytodd
Copy link
Member

that wasn't there before

I am not sure I follow? When all existing package managers launched they did so via npm install, do you mean that wasnt there with corepack?

@GeoffreyBooth
Copy link
Member Author

GeoffreyBooth commented Mar 1, 2024

Because it creates a dependency on npm that wasn’t there before

Whether Yarn or pnpm are downloaded via npm or via some other means is an irrelevant implementation detail. If and when someone eventually opens a PR to actually implement this PR, then you can object to any reliance on npm that such a PR may have, and propose an alternative. I agree it would be great if this script could depend on absolutely nothing, and just download via node:http.get or something really really certain to never be removed from Node; but again that’s just an implementation detail. For the purposes of understanding what I’m suggesting, npm install --global corepack gets the point across and any alternative implementation would need to do the same thing.

The new script could potentially fetch and untar from the registry without invoking npm, but then you’ve basically re-implemented what corepack already does.

The point of doing this is to hand off Corepack to be maintained by someone other than the Node organization. It’s much less of a burden to maintain a script that’s a few dozen lines long than it is to maintain all of Corepack. And Corepack does a lot more than just fetch and untar an archive.

Also consider how this would work in CI - you don’t want to prompt in that case.

So make the script accept an argument like -y. Again, this is an implementation detail.

@GeoffreyBooth
Copy link
Member Author

I just did some experimenting in a node Docker container and I think this is achievable in 9 lines of code. We could have this file be saved into /usr/local/bin/pnpm:

#!/usr/bin/env node
const rl = require('readline').createInterface({ input: process.stdin, output: process.stdout });
rl.question('Install pnpm? (y/n) ', answer => {
  if (answer.toLowerCase() === 'y') {
    require('node:fs').renameSync(__filename, `${__filename}-install`);
    require('node:child_process').fork('./npm', ['install', '--global', 'pnpm']);
  }
  rl.close();
});

This prompts the user to proceed, and if they agree, it uses npm to globally install pnpm. The child_process line can easily be changed to install pnpm via other means, such as via Corepack, or to install their other package @pnpm/exe. I’m sure others can figure out a way to achieve this without npm, but I feel like that would add a lot of complexity in terms of supporting Windows and various other environments and shells. Unless removing npm is a near-term goal, I think it’s okay to use it as a dependency. A Yarn version would be similar, but involve Corepack.

@mcollina
Copy link
Member

mcollina commented Mar 5, 2024

I'm ok as long as the script locks the major version.

@tniessen
Copy link
Member

tniessen commented Mar 5, 2024

I don't have much context here, so please let me know if this has been discussed elsewhere.

Having a yarn binary in the PATH that doesn't actually behave like yarn seems like it might break existing workflows, e.g., any control flows that check for the existence of yarn/pnpm/... before defaulting to a different package manager.

Also consider how this would work in CI - you don’t want to prompt in that case.

So make the script accept an argument like -y. Again, this is an implementation detail.

I don't think changing the CLI of a yarn binary in the user's PATH is an implementation detail. That doesn't seem like a sound approach for CI -- there should be a proper way to install yarn that doesn't go through a "may or may not be yarn" hack, and installation through this convenience wrapper should fail in any non-interactive environment.

Also, messing with /usr/bin etc. requires root privileges, which makes it dangerous to do things like fork('./npm', ...) in @GeoffreyBooth's code above.

@GeoffreyBooth
Copy link
Member Author

Having a yarn binary in the PATH that doesn't actually behave like yarn

That's what enabling Corepack does: it puts binaries in the path that are named yarn and pnpm but are symlinks to Corepack.

@tniessen
Copy link
Member

tniessen commented Mar 5, 2024

That's what enabling Corepack does: it puts binaries in the path that are named yarn and pnpm but are symlinks to Corepack.

And do those also display interactive non-yarn/non-pnpm prompts and install additional software when executed? I am genuinely curious. If they do, it certainly makes sense that they need to be explicitly enabled through corepack and are not in the PATH by default.

@targos
Copy link
Member

targos commented Mar 5, 2024

Since nodejs/corepack#360, I believe that's what they do.

@aduh95
Copy link
Contributor

aduh95 commented Mar 5, 2024

Since nodejs/corepack#360, I believe that's what they do.

I don’t think that’s what Tobias meant: running npm i -g pkg will also download pkg’s dependencies and run the postInstall scripts, running corepack pnpm does not.

@bnb
Copy link
Contributor

bnb commented Mar 6, 2024

I'd also like to recommend an alternative... alternative. We're solving all of this within Node.js and scope wise we're causing a lot of issues with that.

Rather than corepack, we could approach this as a runtime jumper. Effectively, nvm, asdf, fnm... any of those tools. They've all expressed interest in this future, and they could easily also solve the inclusion of package managers. The fast default install could be Node.js with npm for historical DX, and alternatives could be chosen via CLI. This would also give us another feature we've wanted for a long time - Node.js version management.

Presuming that we could work it out with pnpm and yarn, this could be a better first run experience for everyone? Especially given the context that pnpm technically requires npm to exist with the Node.js runtime.

I'm personally a huge fan of fnm. Rust + 90% of the DX I want + really fast. At the end, though, I don't care which we'd go with as long as we can get rough consensus and the community project would be on board.

Edit: an additional benefit of this is that if people don't want npm, we could ideally give that to them. I think we'd still need to provide the current binaries that exist with npm for download reasons, but there's a pretty simple matrix outside of that.

@Ethan-Arrowood
Copy link
Contributor

I like @bnb alternative the most. I and many many other users already use tools like fnm to switch between node versions. Why can't that tool also manage my package manager versions? Why does the package manager version manager have to come with the runtime? Maybe corepack could be distributed inside of those tools instead!

+1 keep npm in node
+1 encourage community tools we already use to become package manager version managers
+1 remove corepack or keep it experimental and off by default

@mcollina
Copy link
Member

mcollina commented Mar 6, 2024

I agree with @bnb. I think a node version manager is actually necessary. That can manage node and package managers.

@GeoffreyBooth
Copy link
Member Author

This came up in nodejs/package-maintenance#591 too, in that our webpage’s primary download link is for unmanaged Node, like an installer, yet our recommended way for installing Node is via a version manager. Which feels like . . . well if we recommend installing via a version manager, shouldn’t that be what the big primary call to action button does?

But there are all sorts of complications with that, not all that different from the complexities posed by Corepack. First of all, we’d need to pick a version manager, whether fnm or asdf or something we build; and it would need to run in all the various obscure platforms that Node supports. Picking a version manager means both choosing a winner (like how we long ago chose npm) and being stuck with it even if a better alternative comes along (also see npm, from some folks’ perspectives). We should also probably provide a distribution that doesn’t include the version manager, so that users could use their own platform’s version manager if they chose; I would expect that anyone building a Docker image probably doesn’t want a version manager there, as it wouldn’t be necessary. And all of this would entail more work on the build team, and they’ve already got a huge load on their shoulders already.

So I’m torn. On the one hand if we recommend something, we should probably ship it; but maybe shipping it entails so many downsides that not shipping our recommendation (maybe just putting it in writing as advice on the website) is the less bad of the two options.

@tniessen
Copy link
Member

tniessen commented Mar 6, 2024

On a side note, Refael and I tried to allow installing multiple versions of Node.js side-by-side on Windows through officially supported means back in 2018 or so (see #4603 and nodejs/version-management#16), but such a Windows-specific solution was blocked by the version management group, who advocated for a cross-platform solution at the time.

@wesleytodd
Copy link
Member

I am a strong +1 on reviving the effort to design and ship a holistic version manager with node.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
experimental Issues and PRs related to experimental features. feature request Issues that request new features to be added to Node.js.
Projects
Development

No branches or pull requests