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
fix: properties accessed on singleton now reflect current state of instance #1366
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -837,6 +837,17 @@ describe('yargs dsl tests', () => { | |
}) | ||
}) | ||
|
||
describe('parsed', () => { | ||
it('should be false before parsing', () => { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I wonder if we could figure out the specific exception that was happening with There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I agree. A simple "showHelp should not fail" should do the job |
||
yargs.parsed.should.equal(false) | ||
}) | ||
|
||
it('should not be false after parsing', () => { | ||
yargs.parse() | ||
yargs.parsed.should.not.equal(false) | ||
}) | ||
}) | ||
|
||
// yargs.parse(['foo', '--bar'], function (err, argv, output) {} | ||
context('function passed as second argument to parse', () => { | ||
it('does not print to stdout', () => { | ||
|
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.
why don't we make this slightly more explicit:
if (['$0', 'parsed'].indexOf(key) !== -1) {
and we should be mindful about leaking any more stateful variables on the yargs object going forward.
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.
I don't get the point. If we support singleton mode, shouldn't every property/method call on Argv have the same effect as if directly called on the singleton?
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.
fair 👍 I just noticed that, I think,
argv
,$0
, andparsed
are the only properties that leak ... and honestly, I don't think we meant to leakparsed
as part of the API.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.
Oops! I did not realize this... However,
parsed
is already leaking from the API when not using the singleton. So, does it have a sense to filter properties we accept to leak from the singleton, and not to do it when not using the singleton?