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

Second argument (first in process.argv) is always ignored. #146

Closed
SpacingBat3 opened this issue Oct 28, 2022 · 5 comments
Closed

Second argument (first in process.argv) is always ignored. #146

SpacingBat3 opened this issue Oct 28, 2022 · 5 comments

Comments

@SpacingBat3
Copy link

Current implementation of @pkgjs/parseargs assumes this to be always correct:

parseargs/index.js

Lines 69 to 70 in 6774908

// Normally first two arguments are executable and script, then CLI arguments
return ArrayPrototypeSlice(process.argv, 2);

However, in some enviroments, either script is compiled to the executable like in case of pkg-packaged scripts or there's node binary (e.g. electron or any other web app framework using Node) that calls the script directly, without any need of passing an additional argument for a path to the script. Moreover, I think path could be passed after flags rather than before, which would also cause some inconstancy between the polyfill and native API.

Maybe the code should be prepared for those cases as well?

@SpacingBat3 SpacingBat3 changed the title First argument is always ignored. Second argument (first in process.argv) is always ignored. Oct 28, 2022
@ljharb
Copy link
Member

ljharb commented Oct 28, 2022

Such a compiler should probably be wrapping or building modules such that process.argv has a fictional argument added - otherwise all code that assumes “what node always does” would break.

@SpacingBat3
Copy link
Author

SpacingBat3 commented Oct 28, 2022

@ljharb thank you for the reply, I've actually verified the polyfill behaves exactly the same as Node (not sure why I expected it to work better than this polyfill in terms of the logic) and such compilance is what I think people should care about. That being said, I feel like parseArgs will still change, considering Electron already breaks with the default logic of the args polyfill.

I'll close this issue for now. It might be useful to reopen it after some time, given how many Node.js-based frameworks and tools are there that actually don't care about Node's argv structure.

@SpacingBat3 SpacingBat3 closed this as not planned Won't fix, can't repro, duplicate, stale Oct 28, 2022
@shadowspawn
Copy link
Collaborator

shadowspawn commented Oct 28, 2022

If your runtime environment follows different conventions than node does with process.argv, then the intended approach is to prepare the arguments yourself and pass them in the configuration:

const result = parseArgs({ args: process.argv.slice(1), strict: false });

The arguments to be parsed were originally a separate parameter, but the normal case is they can be auto-detected so after some discussion they were moved into the configuration.

(For interest, there was explicit support for detecting Electron in the initial submission to the node project, but that got dropped during the pull request to node. Likewise, there was also a suggestion for a process.mainArgs to consult instead of process.argv, still mentioned in the README.)

@SpacingBat3
Copy link
Author

If your runtime environment follows different conventions than node does with process.argv, then the intended approach is to prepare the arguments yourself and pass them in the configuration:

const result = parseArgs({ args: process.argv.slice(1), strict: false });

Yeah, that's what I want to do for now. I just thought Node will take care of the argv better, given how default args value were documented:

(…) Default: process.argv with execPath and filename removed.

This is very non-intuitive description of what's done for those who use Node-based frameworks or mess with argv on their own for some reason and expect that this still would work as it should be. It's not a problem on this project's side so I feel OK to leave it as it is.

@shadowspawn
Copy link
Collaborator

Oh right, makes it sound smarter than it is! Thanks for clarifying source of confusion.

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

3 participants