fix: exit after async handler done by PengBoUESTC · Pull Request #2313 · yargs/yargs

PengBoUESTC · 2023-03-14T07:16:54Z

shadowspawn · 2023-03-15T08:13:59Z

Thanks. My suspicion is adding a callback is a work-around for the async handling not being correct. But it does seem to work! It will take me a while to understand the call paths.

PengBoUESTC · 2023-03-15T08:20:35Z

Thanks. My suspicion is adding a callback is a work-around for the async handling not being correct. But it does seem to work! It will take me a while to understand the call paths.

callback is not a good idea, i did this because showHelp is not declared as an async function, I don't want to make a break change

shadowspawn · 2023-03-20T00:06:19Z

.showHelp() can take a callback already, so do not need to add another one? Could do it like this:

          this.showHelp(message => {
            this.#logger.log(message);
            this.exit(0);
          });

shadowspawn · 2023-03-20T00:10:50Z

Ideally we would like a unit test for any code changes. Are you up for that?

(I have not looked at how to do this. I assume will need a little setup to prepare for an async operation that calls exit!)

PengBoUESTC · 2023-03-21T05:52:03Z

Ideally we would like a unit test for any code changes. Are you up for that?

(I have not looked at how to do this. I assume will need a little setup to prepare for an async operation that calls exit!)

it's not easy to give a test case =_=

shadowspawn · 2023-04-08T01:40:13Z

I'll have a look at doing a test.

shadowspawn · 2023-04-09T00:14:45Z

I got a test working, but uncovered an interesting issue in the process. I think the async triggered during the help processing is not returned as a promise, so calling await on the parse result does not block and allow the help to display before the test completes. I think this PR still makes sense on its own so not considering that a blocker! I have opened a PR to your branch to add the test.

Add test for async help being displayed before process.exit

shadowspawn

I think this is a simple change that should work same way for sync and more usefully for async.

| datasource | package | from | to | | ---------- | ------- | ------ | ------ | | npm | yargs | 17.7.2 | 18.0.0 | ## [v18.0.0](https://github.com/yargs/yargs/blob/HEAD/CHANGELOG.md#1800-2025-05-26) ##### ⚠ BREAKING CHANGES - command names are not derived from modules passed to `command`. - singleton usage of yargs yargs.foo, yargs().argv, has been removed. - minimum node.js versions now `^20.19.0 || ^22.12.0 || >=23`. - yargs is now ESM first ##### Features - commandDir now works with ESM files ([#2461](yargs/yargs#2461)) ([27eec18](yargs/yargs@27eec18)) - **locale:** adds hebrew translation ([#2357](yargs/yargs#2357)) ([4266485](yargs/yargs@4266485)) - yargs is now ESM first ([d90af45](yargs/yargs@d90af45)) - **zsh:** Add default completion as fallback ([#2331](yargs/yargs#2331)) ([e02c91b](yargs/yargs@e02c91b)) ##### Bug Fixes - addDirectory do not support absolute command dir ([#2465](yargs/yargs#2465)) ([3a40a78](yargs/yargs@3a40a78)) - allows ESM modules commands to be extensible using visit option ([#2468](yargs/yargs#2468)) ([200e1aa](yargs/yargs@200e1aa)) - **browser:** fix shims so that yargs continues working in browser context ([#2457](yargs/yargs#2457)) ([4ae5f57](yargs/yargs@4ae5f57)) - **build:** address problems with typescript compilation ([#2445](yargs/yargs#2445)) ([8d72fb3](yargs/yargs@8d72fb3)) - coerce should play well with parser configuration ([#2308](yargs/yargs#2308)) ([8343c66](yargs/yargs@8343c66)) - **deps:** update dependency yargs-parser to v22 ([#2470](yargs/yargs#2470)) ([639130d](yargs/yargs@639130d)) - exit after async handler done ([#2313](yargs/yargs#2313)) ([e326cde](yargs/yargs@e326cde)) - handle spaces in bash completion ([#2452](yargs/yargs#2452)) ([83b7788](yargs/yargs@83b7788)) - parser-configuration should work well with generated completion script ([#2332](yargs/yargs#2332)) ([888db19](yargs/yargs@888db19)) - propagate Dictionary including undefined in value type ([#2393](yargs/yargs#2393)) ([2b2f7f5](yargs/yargs@2b2f7f5)) - **zsh:** completion no longer requires double tab when using autoloaded ([0dd8fe4](yargs/yargs@0dd8fe4)) ##### Code Refactoring - command names are not derived from modules passed to `command`. ([d90af45](yargs/yargs@d90af45)) - singleton usage of yargs yargs.foo, yargs().argv, has been removed. ([d90af45](yargs/yargs@d90af45)) ##### Build System - minimum node.js versions now `^20.19.0 || ^22.12.0 || >=23`. ([d90af45](yargs/yargs@d90af45))

pengbo43 added 5 commits March 14, 2023 15:16

fix: return after then

285c873

fix: add callback for showHelp

583b9e1

chore: format

5eee6b5

chore: format

77a496c

chore: update showHelp assert

3bcfc2e

PengBoUESTC changed the title ~~fix: return after then~~ fix: exit after async handler done Mar 14, 2023

chore: lint fix

dfa441f

chore: remove callback

91c42f8

PengBoUESTC marked this pull request as draft March 28, 2023 02:00

shadowspawn and others added 2 commits April 9, 2023 12:16

Add test for async help being displayed before process.exit

772e011

Merge pull request #1 from shadowspawn/add-test-for-async-help

2281ace

Add test for async help being displayed before process.exit

PengBoUESTC marked this pull request as ready for review April 10, 2023 02:11

shadowspawn approved these changes Apr 10, 2023

View reviewed changes

shadowspawn and others added 2 commits April 28, 2023 18:00

Merge branch 'main' into main

48bc718

Merge branch 'main' into main

7728083

bcoe merged commit e326cde into yargs:main Apr 28, 2023

github-actions bot mentioned this pull request Apr 28, 2023

chore(main): release 18.0.0 #2325

Merged

shadowspawn mentioned this pull request Jul 18, 2023

how can I use default command and help simultaneously #2312

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: exit after async handler done#2313

fix: exit after async handler done#2313
bcoe merged 11 commits intoyargs:mainfrom
PengBoUESTC:main

PengBoUESTC commented Mar 14, 2023

Uh oh!

shadowspawn commented Mar 15, 2023

Uh oh!

PengBoUESTC commented Mar 15, 2023

Uh oh!

shadowspawn commented Mar 20, 2023 •

edited

Loading

Uh oh!

shadowspawn commented Mar 20, 2023

Uh oh!

PengBoUESTC commented Mar 21, 2023

Uh oh!

shadowspawn commented Apr 8, 2023

Uh oh!

shadowspawn commented Apr 9, 2023 •

edited

Loading

Uh oh!

shadowspawn left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

PengBoUESTC commented Mar 14, 2023

Uh oh!

shadowspawn commented Mar 15, 2023

Uh oh!

PengBoUESTC commented Mar 15, 2023

Uh oh!

shadowspawn commented Mar 20, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

shadowspawn commented Mar 20, 2023

Uh oh!

PengBoUESTC commented Mar 21, 2023

Uh oh!

shadowspawn commented Apr 8, 2023

Uh oh!

shadowspawn commented Apr 9, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

shadowspawn left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

shadowspawn commented Mar 20, 2023 •

edited

Loading

shadowspawn commented Apr 9, 2023 •

edited

Loading