-
Notifications
You must be signed in to change notification settings - Fork 31.1k
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
src: add --install
runtime flag
#57593
base: main
Are you sure you want to change the base?
Conversation
Review requested:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What lockfile formats would this support, or are we inventing our own?
How would we avoid the secret key issue that recently affected Corepack?
@@ -27,6 +27,7 @@ declare_args() { | |||
"deps/cjs-module-lexer/dist/lexer.js", | |||
"deps/undici/undici.js", | |||
"deps/amaro/dist/index.js", | |||
"deps/corepack/dist/lib/download.js", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So this would make Corepack a dependency of Node.js core? I'm not sure how I feel about that. I still have concerns about how it differs so much from the Node codebase (written in TypeScript, dependent on Yarn) and it has an uncertain future. This seems to make the project responsible for Corepack's continued development. It also seems to contradict the recent vote to remove Corepack (I understand it would become a hidden dependency and no longer available as a binary, but presumably it would be accessible via --expose-internals
?)
We already bundle npm and it is the canonical dependency downloader; why not use that? Or more targeted dependencies of npm like Arborist.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
npm has arguably an even more uncertain future, it is not maintained by us. But in any case, which code does the download matters very little (Corepack as a dep, copy the code from Corepack into core, another lib, etc.), what’s more important to establish is whether we want the feature
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Define “the feature”. Is it installing npm's lockfile? Yarn's or pnpm's? All of them? Something else? Does it depend on the package manager being downloaded first?
It’s a public key, not a secret key. The lock files always contain a trusted hash, so there’s no signature to check with the current implementation (also it doesn’t have the concept of a registry, it only accepts URLs) |
If we are going to do some |
Installing packages seems like the sole purview of a package manager and not a platform; I think that conceptually this isn't a feature that node should add. |
I’m not sure who the target audience is for this feature. Presumably it’s users who want to install dependencies from a lockfile but really want to avoid using npm at all, either to do that install or to install their preferred package manager to do that install. I understand that there are such users out there, such as the maintainers of Corepack, but my sense is that the vast majority of our users aren’t so offended by npm that they wouldn’t consider using it at least as a means to get their preferred package manager. Even for the npm-avoiding users, they could use another method like We already ship a way to install dependencies from a lockfile: our bundled npm. Adding another method would need to add meaningful, useful functionality (like support for different lockfile formats or improved performance) to justify the maintenance burden. |
This PR adds a way to download stuff from a lock file. The idea is to give folks an option to download a package manager and a specific node version. It could also enables a use-case to use a project without needing any package manager (useful for CI, and users who don't need to add/remove/update any deps, just consume them as is).