volta-cli / volta

Volta: JS Toolchains as Code. ⚡
https://volta.sh
Other
11.01k stars 231 forks source link

Installing a scoped package from a registry without using = #1039

Open bjornstar opened 2 years ago

bjornstar commented 2 years ago

This breaks:

npm install @package-scope/package-name --registry https://my.package-registry.com/api
   note: Volta is processing each package separately
npm WARN invalid config registry="@package-scope/package-name" set in command line options
npm WARN invalid config Must be full url with "http://"
npm ERR! Cannot destructure property 'name' of '.for' as it is undefined.

However if we use an equal sign (=) after --registry everything goes just fine.

This is not a problem with npm, when using the same command with npm the scoped package is installed correctly from the registry. This appears to be a problem with the CLI parsing done by volta.

ljharb commented 2 years ago

Double-dashed commands always are supposed to have an equals sign after it. npm allows incorrect usage here.

bjornstar commented 2 years ago

Double-dashed commands always are supposed to have an equals sign after it. npm allows incorrect usage here.

Yes, I understand that, but taking over the npm command line implies you're going to support the same syntax. My non-volta using colleagues are all saying "It works on my machine" and are not so interested in making things work specifically for volta-flavored npm.

ljharb commented 2 years ago

Also a reasonable expectation.

Although, you may want to tell them that in npm 9, that command will break, since npm plans to overhaul the config system - so regardless they'll need to migrate to properly specifying double-dashed arguments eventually.

charlespierce commented 2 years ago

Hi @bjornstar, sorry this is created unnecessary friction! Are you doing a global installation? That is currently a limitation in our global installation process, tracked in #836. However, if this is on a local npm install, then that's a different bug because Volta shouldn't be looking at those at all.

The underlying complication is that without the equals sign, the only way to know which flags take an argument (like --registry <URL>) and which flags don't (like --legacy-peer-deps) is to mostly re-implement the npm (and yarn) argument parser. The alternative, suggested in the above-linked issue, is to curate a list of flags that take an argument and use those, which will improve the process slightly but may still have some missed edges.

@ljharb That's interesting to know! Out of curiosity, do you have a link handy for discussion around npm dropping support for those kinds of flags?

ljharb commented 2 years ago

No, nothing I'm aware of at the moment. I'll try to cc you in when that config replacement comes up.

To be clear, i don't think there's a specific intention to drop that, but no reasonable args parser would support it by default, and i doubt anyone would want to support it intentionally.

bjornstar commented 2 years ago

@charlespierce It is indeed a global install. I feel your pain because I also maintain a CLI that passes flags to node. We had to add custom handling for inspect and require because the "broken" behavior is what the community expects to work. It would be much appreciated if volta supported some of the more common options at least until npm deprecates support.

@ljharb It would be nice if node and npm didn't accept "broken" behavior, but I can imagine it's going to be a bit painful to get there. I'd love to follow the progress if you can drop a link.