fix: populate correct value on yargs.parsed and stop warning on access

[bcoe]

This should address a couple minor issues that lead to #1144:

1. self.parsed was being populated with an argv instance rather than a yargsParser.detailed instance, leading to the missing fields that raised exceptions.
2. once 1. was addressed, the help message was incorrect, because async handlers prior to this relied on state not being reset; to solve this, we now cache the help message that would be displayed at the time of a promise's invocation.
3. there was one final issue related to our self.parsed, TypeScript executes all methods on a class, making our warning message display even though it wasn't explicitly invoked (I've reverted the warning message).

TypeScript Issue:

var __importStar = (this && this.__importStar) || function (mod) {
    if (mod && mod.__esModule) return mod;
    var result = {};
    if (mod != null) for (var k in mod) if (Object.hasOwnProperty.call(mod, k)) result[k] = mod[k];
    result["default"] = mod;
    return result;
};

@gajus could I get you to try this out?

fixes: #1144

[gajus]

Can confirm that this fixes the issue I have reported originally.

[mleguen]

I do think the unhandled rejection issue should be addressed before merging.

[mleguen]

@bcoe IMO, we have a conception issue here: we should not accept async handlers in sync chained functions.

I recommend:

• either deprecating the use of async command handlers, and ignoring command handler results, without trying to display help (which would be consistent with what happen when a sync handler throws: no help is displayed)
• or having runCommand and all depending functions become async too, which would break the ability to chain calls after such methods

[gajus]

or having runCommand and all depending functions become async too, which would break the ability to chain calls after such methods

What is the impact of breaking ability to chain methods?

The last suggestion sounds reasonable. First one sounds like a quick workaround that will leave up to the individual yargs consumer to figure out their own way of handling async errors.

[mleguen]

This conception issue is what leads to your last test being so difficult to write.

Any client program using async handlers and wanting to perform actions after yargs is done parsing will have the same problem: we cannot know when yargs is done parsing when using async handler.

[mleguen]

What is the impact of breaking ability to chain methods?

In fact, I think it would only prevent chaining showHelp(), as parse() is already not chainable, and I cannot see any use case for chaining showHelp().

However, the major impact would be that, when using async handlers, parse would no longer return directly the parsing results, but a promise. This would be great for people wanting to do things after parsing, but could break existing code.

Next major release?

[mleguen]

In order not to block this fix, here is what I suggest:

• fix the unhandled rejection issue
• remove the last test, as not being writable yet
• merge this PR
• open a new issue about parse() and showHelp() having to return a promise when a command handler does so
• write a PR for this new issue, including the missing test (I can handle this)

@bcoe what is your opinion on this?

[bcoe]

@mleguen I'll take another stab at the patch with the unhandledRejection fixed. I tend to agree that the async implementation was too clever, but I think that some folks rely on the behavior now and my preference would be to get this feature back to a healthy state.

I think we should avoid expanding on async behavior further without something resembling a design document; personally I think it's better to treat yargs as sync, as a philosophy, and then your can perform async operations post-hoc with the parsed argument object.

[bcoe]

@mleguen ready for review 👍