You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
util: parseArgs should support required in options[key] #44564
I'm building a CLI that creates some instances by id and name. But I have to check if all options are filled by the user one by one.
constargs=process.argv.slice(2)constresult=parseArgs({
args,options: {owner: {type: 'string'},repo: {type: 'string'}}})// imaging you will have tens of options in your CLIif(result.values.repo==null||result.values.owner==null){thrownewError('require parameters')}
./build.js --owner=himself65 --repo=xxx
What is the feature you are proposing to solve the problem?
Add required option.
parseArgs({
args,options: {owner: {type: 'string',required: true},repo: {type: 'string',required: true},force: {type: 'boolean',// by default, required is false},}})
changed the title [-]utils: parseArgs should support `required` in options[key][/-][+]util: parseArgs should support `required` in options[key][/+]on Sep 7, 2022
Aren't required positional arguments far more common than required options? I think Node.js should follow POSIX conventions when it comes to parsing arguments, and those seem to heavily favor positional arguments for required values (see GNU core utilities, for example).
Positional arguments are suitable for 1 or 2 arguments, but UX guides often recommend using options when there are more to make their meaning clearer. In your example with only two options, at least one of them would make sense as a positional argument and perhaps the other one could be deduced.
// imaging you will have tens of options in your CLI
Normally most or all options are optional. So it isn't very likely people will have lots of required options.
I agree with @shadowspawn. I would say that required options are relatively uncommon.
A required: boolean option gives little flexibility. For example, how would I implement a --help option when I have other options that are required unless--help is used?
On the contrary, usually, each option value has to be validated in some way anyway. Its existence or non-existence is usually not the only validation criterion.
Reacted by John Gee, Alex Yang, Kevin Gibbons and Marek Lukáš
This would probably be a separate issue, but what about incompatible and requires fields? While unconditionally required options are rare and frowned upon, dependent and incompatible options are far more common. Both these fields would be of type string[], containing names of other options in the current parseArgs object:
parseArgs({
args,options: {owner: {type: 'string',requires: ['repo'],incompatible: ['org'],},org: {type: 'string',requires: ['repo'],// no need to specify incompatible: ['owner']// it should work both ways on either one// but it can also be explicitly on both if desired},repo: {type: 'string',// details on requiresOne further downrequiresOne: ['owner','org'],},url: {type: 'string',incompatible: ['repo','org','owner'],}}});
All of these new fields would default to an empty array []. If any of them specify either their own option or a non-existent option it should throw a TypeError.
requires is directional, if --a requires --b but --b doesn't require --a, foo --b is valid but foo --a is not.
requiresOne is a variation of requires where the array is treated as a list of ORs instead of ANDs.
requires and requiresOne can be used together without issue, but naturally overlapping options between the two will have higher precedence on requires.
incompatible is bidirectional, if --a declared itself incompatible with --b, then --b is also implicitly incompatible with --a. Explicitly marking both incompatible with each other is fine and does nothing extra.
If a conflict between the fields requires* and incompatible of two options or one by itself happens, it should throw a TypeError.
This would probably be a separate issue, but what about incompatible and requires fields?
Nice detailed description. I suggest opening as a separate issue if you want more visibility.
I think of these as beyond-basic parsing options. I don't think parseArgs itself needs them and the "vision" for parseArgs was for a basic robust option parser, but that isn't currently a rule and I expect features will be considered case by case.
I think of these as beyond-basic parsing options. I don't think parseArgs itself needs them and the "vision" for parseArgs was for a basic robust option parser, but that isn't currently a rule and I expect features will be considered case by case.
I was not aware of the original "vision" for it and its a fair point, but as a counterpoint on the other hand, in this particular case I would argue it makes sense given it only adds optional slightly more advanced (but still arguably pretty basic) configuration, so in my opinion it'd be fine to add whilst still keeping that original vision.
I will make a separate issue for this soon for further discussion and update this comment linking to it when I do.
Separate issue opened at #45062
I don't think parseArgs itself needs them and the "vision" for parseArgs was for a basic robust option parser
IMHO, the same can be said about default values, which are trivial to do in userland (see, for example, #44631 (comment)) but which were stilled added.
My main concern here is adding complexity both to the API surface and to the implementation while still not providing a complete solution. The goal has not been to provide a full-fledged and feature-complete argument parser. In this particular case, even with the three newly proposed options (requires, requiresOne, incompatible) not all sets of subsets can be described, so it is not a complete solution to describing relations between the existence of options, while significantly increasing complexity.
IMHO, the same can be said about default values, which are trivial to do in userland (see, for example, #44631 (comment)) but which were stilled added.
Yes. Yes, indeed. Default values could have been left out! However, there were multiple people looking for defaults who still wanted it despite the relative ease of implementation or alternatives. (I speculate a partial factor may be people not familiar or comfortable with destructuring with defaults.)
I've been waiting over 10 minutes and no types load and it won't typecheck, but no matter, I just looked directly at the source of @types/node, and it seems that its allowing extraneous keys due to the argument being a constrained generic, which makes it extensible, I'm not sure whether whoever wrote this type intended this or not, but compared to how all the other node types and TS typings in general are done this is outside the norm. It should probably be fixed.
I've been waiting over 10 minutes and no types load and it won't typecheck
Well, you can of course always try it locally, if the playground isn't working for you.
its allowing extraneous keys due to the argument being a constrained generic
I wrote the types, and it's generic so that it can do precise type inference on the result. It's not something which can be "fixed" without making the experience of using the library significantly worse, as far as I'm aware.
But also, TypeScript does not generally prohibit having extra properties, outside of contexts where you're passing a literal (for which it can determine that's definitely an error). So I'm not sure why you'd expect to have an error here even without generics.
compared to how all the other node types and TS typings in general are done this is outside the norm
There's no other built-in functions in node which are comparable to parseArgs, where the result type depends so precisely on the input. Having generics which allow more precise inference is common to the types for other argument parsers, like yargs.
In any case, if you think the TypeScript types could be improved, best to open an issue on definitely-typed.
Fiddled around with this a bit more and you seem to be correct, this is indeed a limitation of TypeScript currently, pretty annoying if at some point node decides to be more strict at runtime with extraneous properties for new functions, but I guess until that happens and forces TS team to do something about it this will have to do, thanks for your insights.
To go back to the original topic, I'm still not a big fan of relying on something that isn't by-the-docs (Node-wise) as like I said they could decide to be stricter in the future, but until then I suppose it is a valid solution. If it was explicitly documented in the parseArgs docs that extraneous arguments for user-defined metadata was allowed, I'd be 100% for it.
There has been no activity on this feature request for 5 months and it is unlikely to be implemented. It will be closed 6 months after the last non-automated comment.
There has been no activity on this feature request and it is being closed. If you feel closing this issue is not the right thing to do, please leave a comment.
Activity
[-]utils: parseArgs should support `required` in options[key][/-][+]util: parseArgs should support `required` in options[key][/+]tniessen commentedon Sep 8, 2022
Aren't required positional arguments far more common than required options? I think Node.js should follow POSIX conventions when it comes to parsing arguments, and those seem to heavily favor positional arguments for required values (see GNU core utilities, for example).
shadowspawn commentedon Sep 8, 2022
Positional arguments are suitable for 1 or 2 arguments, but UX guides often recommend using options when there are more to make their meaning clearer. In your example with only two options, at least one of them would make sense as a positional argument and perhaps the other one could be deduced.
Normally most or all options are optional. So it isn't very likely people will have lots of required options.
An earlier version of the Python library discouraged required options on principle (optParse). The current version has this to say: https://docs.python.org/3/library/argparse.html#required
tniessen commentedon Sep 8, 2022
I agree with @shadowspawn. I would say that required options are relatively uncommon.
A
required: booleanoption gives little flexibility. For example, how would I implement a--helpoption when I have other options that are required unless--helpis used?On the contrary, usually, each option value has to be validated in some way anyway. Its existence or non-existence is usually not the only validation criterion.
jhmaster2000 commentedon Oct 15, 2022
This would probably be a separate issue, but what about
incompatibleandrequiresfields? While unconditionally required options are rare and frowned upon, dependent and incompatible options are far more common. Both these fields would be of typestring[], containing names of other options in the currentparseArgsobject:The above code would result in:
[]. If any of them specify either their own option or a non-existent option it should throw aTypeError.requiresis directional, if--arequires--bbut--bdoesn't require--a,foo --bis valid butfoo --ais not.requiresOneis a variation ofrequireswhere the array is treated as a list of ORs instead of ANDs.requiresandrequiresOnecan be used together without issue, but naturally overlapping options between the two will have higher precedence onrequires.incompatibleis bidirectional, if--adeclared itself incompatible with--b, then--bis also implicitly incompatible with--a. Explicitly marking both incompatible with each other is fine and does nothing extra.requires*andincompatibleof two options or one by itself happens, it should throw aTypeError.shadowspawn commentedon Oct 15, 2022
Nice detailed description. I suggest opening as a separate issue if you want more visibility.
I think of these as beyond-basic parsing options. I don't think
parseArgsitself needs them and the "vision" for parseArgs was for a basic robust option parser, but that isn't currently a rule and I expect features will be considered case by case.jhmaster2000 commentedon Oct 16, 2022
I was not aware of the original "vision" for it and its a fair point, but as a counterpoint on the other hand, in this particular case I would argue it makes sense given it only adds optional slightly more advanced (but still arguably pretty basic) configuration, so in my opinion it'd be fine to add whilst still keeping that original vision.
I will make a separate issue for this soon for further discussion and update this comment linking to it when I do.Separate issue opened at #45062
tniessen commentedon Oct 17, 2022
IMHO, the same can be said about default values, which are trivial to do in userland (see, for example, #44631 (comment)) but which were stilled added.
My main concern here is adding complexity both to the API surface and to the implementation while still not providing a complete solution. The goal has not been to provide a full-fledged and feature-complete argument parser. In this particular case, even with the three newly proposed options (
requires,requiresOne,incompatible) not all sets of subsets can be described, so it is not a complete solution to describing relations between the existence of options, while significantly increasing complexity.shadowspawn commentedon Oct 18, 2022
Yes. Yes, indeed. Default values could have been left out! However, there were multiple people looking for defaults who still wanted it despite the relative ease of implementation or alternatives. (I speculate a partial factor may be people not familiar or comfortable with destructuring with defaults.)
Checking for champion for not adding: pkgjs/parseargs#142 (comment)
bakkot commentedon Dec 2, 2022
Here's how I'd write this today:
This works just as well for ten required options as for two.
Given how easy this is to do in userland, and that it's a relatively unusual thing to want, I don't think it belongs in the standard library.
jhmaster2000 commentedon Dec 8, 2022
TypeScript will not allow this
bakkot commentedon Dec 8, 2022
Seems like it does to me? Here's a playground.
jhmaster2000 commentedon Dec 8, 2022
That playground isn't doing any typechecking because the
utilmodule is unresolved, so the Node types aren't known at all.bakkot commentedon Dec 8, 2022
Yes, it is doing typechecking. You just have to wait a second or two for it to load the types, after which the errors will go away.
jhmaster2000 commentedon Dec 8, 2022
I've been waiting over 10 minutes and no types load and it won't typecheck, but no matter, I just looked directly at the source of
@types/node, and it seems that its allowing extraneous keys due to the argument being a constrained generic, which makes it extensible, I'm not sure whether whoever wrote this type intended this or not, but compared to how all the other node types and TS typings in general are done this is outside the norm. It should probably be fixed.bakkot commentedon Dec 8, 2022
Well, you can of course always try it locally, if the playground isn't working for you.
I wrote the types, and it's generic so that it can do precise type inference on the result. It's not something which can be "fixed" without making the experience of using the library significantly worse, as far as I'm aware.
But also, TypeScript does not generally prohibit having extra properties, outside of contexts where you're passing a literal (for which it can determine that's definitely an error). So I'm not sure why you'd expect to have an error here even without generics.
There's no other built-in functions in node which are comparable to
parseArgs, where the result type depends so precisely on the input. Having generics which allow more precise inference is common to the types for other argument parsers, like yargs.In any case, if you think the TypeScript types could be improved, best to open an issue on definitely-typed.
jhmaster2000 commentedon Dec 8, 2022
Fiddled around with this a bit more and you seem to be correct, this is indeed a limitation of TypeScript currently, pretty annoying if at some point node decides to be more strict at runtime with extraneous properties for new functions, but I guess until that happens and forces TS team to do something about it this will have to do, thanks for your insights.
To go back to the original topic, I'm still not a big fan of relying on something that isn't by-the-docs (Node-wise) as like I said they could decide to be stricter in the future, but until then I suppose it is a valid solution. If it was explicitly documented in the parseArgs docs that extraneous arguments for user-defined metadata was allowed, I'd be 100% for it.
ErickWendel commentedon Jul 20, 2023
I'm +1 for this. I opened an issue that had the same goal. I think adding the
requiredfield would work well.github-actions commentedon Jan 17, 2024
There has been no activity on this feature request for 5 months and it is unlikely to be implemented. It will be closed 6 months after the last non-automated comment.
For more information on how the project manages feature requests, please consult the feature request management document.
github-actions commentedon Feb 17, 2024
There has been no activity on this feature request and it is being closed. If you feel closing this issue is not the right thing to do, please leave a comment.
For more information on how the project manages feature requests, please consult the feature request management document.