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
feat: allow strict mode to be applied globally #734
Conversation
@nexdrew one question for you, would it be more intuitive for to default to If this was a brand new feature being added, do you think you'd opt for this API surface? If so, I'm sold 👍 |
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.
LGTM 👍 Do we want to consider making it global by default in the next major? I like the backwards compatibility this PR maintains but a default global makes more sense I think and would also play in line with automatically globally options we wanted to introduce at some point
@bcoe @maxrimue If I were adding this as a new feature, I'd probably apply A new API method would allow us to use method name and boolean argument to represent the 2 main decisions concerning strict mode: (a) do you want to enable or disable it and (b) do you want to do so globally or not, satisfying the 4 use-cases I am aware of. Here's a table comparing the API proposed by this PR with two theoretical possibilities (for illustrative purposes only):
We could, of course, also just accept an object that allows an author to define In practical terms, I think we could get away with assuming global and just allow authors to enable and disable it as necessary via The current API being what it is, though, I'd rather avoid a major bump just for this. If it were coupled with a few other breaking changes, it'd be fine, but I'd like to avoid too many potentially unwarranted (meaning we could have introduced the change in a non-breaking way) major version bumps. |
@nexdrew I, too, think we should avoid a major bump just for this. This is why I would propose to land backwards compatible changes like you made them now and make a defaults change when we're about to land the next major bump which focuses on other major changes. I was also thinking about the object approach ( |
@maxrimue Very good points! I agree with your proposal. I think, when it comes to the most ideal API, yargs should make doing the "right" thing (i.e. the most prominent use-case, which should align with the user's intentions most of the time) as easy as possible. In this case, it sounds like we're all agreed that In the meantime, should I update the docs to clarify what |
To avoid introducing an API consistency between 6.x and 7.x, perhaps we should just go ahead and change it to "global by default" (as bcoe originally implied) and allow it to be disabled per command via |
@nexdrew I approve of this message, I think we have enough things queued up for a really solid |
@bcoe Definitely for making options global by default 👍 |
@nexdrew 👍 let's make this one of the first things we land on a |
@nexdrew once you dust this off, perhaps you can re-submit the pull against a |
While recently working on a yargs-based Slackbot with several nested commands, I wanted to apply
.strict()
mode at each level so that when an invalid command/subcommand is given (e.g. mistyped) it would return help text (with an error message) instead of not running/returning anything. Since there is currently no way to apply strict mode globally in yargs, I ended up having to call.strict()
in each command's builder function at every nested level. Ugh.This PR augments the
.strict()
API so that it can accept a boolean value to either (a) apply globally when passedtrue
or (b) be disabled when passedfalse
(to allow a command to override a global strict mode). The method maintains backwards-compatibility with the previous API of enabling strict mode in a non-global way when called without any arguments.It might be slightly odd interpreting a boolean value in two different ways based on its value, but I think it's acceptable since it provides for all use-cases:
.strict()
.strict(true)
.strict(false)
Note that strict mode defaults to "globally" disabled, so not calling the method at all satisfies that use-case.