Repository: yargs/yargs
Pull request #1473 (2019-11-07 19:07:57)

feat: onFinishCommand handler

Example usage:
<img width="492" alt="image" src="https://user-images.githubusercontent.com/580229/68419291-faae0c80-01aa-11ea-9b69-524ec8f039c7.png">


mleguen: @bcoe Perhaps redundant with #1452?

Or perhaps not if #1452 only lands in yargsa...

Not incompatible, anyway.
bcoe: rather than adding additional functionality to yargs for handling async behavior, I'd rather see us combine efforts [here](https://github.com/yargs/yargsa/issues/4) and figure out what an async-first version of yargs might look like.
bcoe: @mleguen @petrgrishin if there was a stop gap between a fully async version of yargs, and something better than what we have today, I'd be open to this too; but would love for folks to collaborate and figure out what those two versions of the library are.
mleguen: @bcoe @petrgrishin Sure, this was the idea behind my comments on this issue.

I feel however that yargsa and this issue are complementary:
- yargs focuses on async API (ie medling with promises)
- this issue could bring the "tell application the command handler is done" feature for sync applications

I would then recommend sticking here to a standard sync API, ie:
- adding an optional done argument to the command handler
- calling onFinishCommand when done is called (if the handler uses it) or when the handler returns (if not using it)

And applications using promise-returning command handler should be encouraged to use yargsa.

What would you think of this?
bcoe: @mleguen your suggestion of using this work as an intermediate step towards a fully async API seems reasonable to me (my wish would be for us to get down to a healthier number of open issues on the existing yargs API, before we made the sweeping change proposed in [yargsa](https://github.com/yargs/yargsa/issues/4)).

@petrgrishin would you be open to addressing some of the code review brought up here?

Patch:
diff --git a/docs/advanced.md b/docs/advanced.md
index 3479799..3378b4d 100644
--- a/docs/advanced.md
+++ b/docs/advanced.md
@@ -449,6 +449,20 @@ yargs.parserConfiguration({
 See the [yargs-parser](https://github.com/yargs/yargs-parser#configuration) module
 for detailed documentation of this feature.
 
+## Command finish hook
+### Example
+```
+yargs
+    .comand('cmd', ..., async () => {
+        await this.model.find()
+        return Promise.resolve('result value')
+    })
+    .onFinishCommand(async (resultValue) => {
+        await this.db.disconnect()
+        process.exit()
+    }).argv;
+```
+
 ## Middleware
 
 Sometimes you might want to transform arguments before they reach the command handler.
diff --git a/lib/command.js b/lib/command.js
index 6f6dd0d..12a9d8a 100644
--- a/lib/command.js
+++ b/lib/command.js
@@ -240,15 +240,26 @@ module.exports = function command (yargs, usage, validation, globalMiddleware) {
         handlerResult = commandHandler.handler(innerArgv)
       }
 
+      const handlerFinishCommand = yargs.getHandlerFinishCommand()
       if (isPromise(handlerResult)) {
         yargs.getUsageInstance().cacheHelpMessage()
-        handlerResult.catch(error => {
-          try {
-            yargs.getUsageInstance().fail(null, error)
-          } catch (err) {
+        handlerResult
+          .then(value => {
+            if (handlerFinishCommand) {
+              handlerFinishCommand(value)
+            }
+          })
+          .catch(error => {
+            try {
+              yargs.getUsageInstance().fail(null, error)
+            } catch (err) {
             // fail's throwing would cause an unhandled rejection.
-          }
-        })
+            }
+          })
+      } else {
+        if (handlerFinishCommand) {
+          handlerFinishCommand(handlerResult)
+        }
       }
     }
 
diff --git a/yargs.js b/yargs.js
index 9ccb304..78bfd57 100644
--- a/yargs.js
+++ b/yargs.js
@@ -32,6 +32,7 @@ function Yargs (processArgs, cwd, parentRequire) {
   let preservedGroups = {}
   let usage = null
   let validation = null
+  let handlerFinishCommand = null
 
   const y18n = Y18n({
     directory: path.resolve(__dirname, './locales'),
@@ -172,6 +173,7 @@ function Yargs (processArgs, cwd, parentRequire) {
     frozen.parsed = self.parsed
     frozen.parseFn = parseFn
     frozen.parseContext = parseContext
+    frozen.handlerFinishCommand = handlerFinishCommand
   }
   function unfreeze () {
     let frozen = frozens.pop()
@@ -190,6 +192,7 @@ function Yargs (processArgs, cwd, parentRequire) {
     completionCommand = frozen.completionCommand
     parseFn = frozen.parseFn
     parseContext = frozen.parseContext
+    handlerFinishCommand = frozen.handlerFinishCommand
   }
 
   self.boolean = function (keys) {
@@ -483,6 +486,14 @@ function Yargs (processArgs, cwd, parentRequire) {
     return self
   }
 
+  self.onFinishCommand = function (f) {
+    argsert('<function>', [f], arguments.length)
+    handlerFinishCommand = f
+    return self
+  }
+
+  self.getHandlerFinishCommand = () => handlerFinishCommand
+
   self.check = function (f, _global) {
     argsert('<function> [boolean]', [f, _global], arguments.length)
     validation.check(f, _global !== false)


Test patch:
diff --git a/test/yargs.js b/test/yargs.js
index 2700318..be7e4bb 100644
--- a/test/yargs.js
+++ b/test/yargs.js
@@ -2355,4 +2355,40 @@ describe('yargs dsl tests', () => {
     // #1482.
     argv['--'].should.eql(['foo'])
   })
+
+  describe('onFinishCommand', () => {
+    it('use with promise', (done) => {
+      const result = 'noop-result'
+      let calledTimes = 0
+      yargs(['noop']).command('noop', 'a noop command', noop, async () => {
+        return result
+      }).onFinishCommand(async (commandResult) => {
+        commandResult.should.eql(result)
+        calledTimes++
+      })
+        .parse('noop')
+
+      setTimeout(() => {
+        calledTimes.should.eql(1)
+        done()
+      }, 5)
+    })
+
+    it('use without promise', (done) => {
+      const result = 'noop-result'
+      let calledTimes = 0
+      yargs(['noop']).command('noop', 'a noop command', noop, () => {
+        return result
+      }).onFinishCommand((commandResult) => {
+        commandResult.should.eql(result)
+        calledTimes++
+      })
+        .parse('noop')
+
+      setTimeout(() => {
+        calledTimes.should.eql(1)
+        done()
+      }, 5)
+    })
+  })
 })