Repository: yargs/yargs
Pull request #1289 (2019-02-07 03:24:31)

chore: some slight modifications to applyBeforeValidation based on conversations
This makes a few changes to #1255, based on conversations with @trevorlinton, @evocateur, and a few other folks.

main differences:

* `applyBeforeValidation` now works on a per-command basis (to do so call `.middleware` in a builder function).
* the `yargs` object is now populated as a second parameter to middleware (I intend to in a separate pull request, add the ability to get the current aliases available).

@Khaledgarbaya it's my feeling that this simplifies things a bit, and makes the `applyBeforeValidation` work in the more general case ... making me an advocate.

TODO:

- [x] I'm still not using arsert for the middleware helper on the yargs object, it would be good to do so.


Patch:
diff --git a/docs/api.md b/docs/api.md
index e229d56..e7d6411 100644
--- a/docs/api.md
+++ b/docs/api.md
@@ -945,7 +945,7 @@ To submit a new translation for yargs:
 
 *The [Microsoft Terminology Search](http://www.microsoft.com/Language/en-US/Search.aspx) can be useful for finding the correct terminology in your locale.*
 
-<a name="middleware"></a>.middleware(callbacks)
+<a name="middleware"></a>.middleware(callbacks, [applyBeforeValidation])
 ------------------------------------
 
 Define global middleware functions to be called first, in list order, for all cli command.  
@@ -962,13 +962,44 @@ yargs
   })
   .middleware([mwFunc1, mwFunc2]).argv;
 ```
+
 When calling `myCommand` from the command line, mwFunc1 gets called first, then mwFunc2, and finally the command's handler.  The console output is:
+
 ```
 I'm a middleware function
 I'm another middleware function
 Running myCommand!
 ```
 
+Middleware can be applied before validation by setting the second parameter to `true`.  This will execute the middleware prior to validation checks, but after parsing.
+
+Middleware is passed two parameters `argv`, the current parsed options object,
+and `yargs` the yargs instance itself, which provides contextual information
+about the current state of parsing.
+
+A modified `argv` object will ultimately be what is passed to a command's
+handler function.
+
+```js
+// populating home directory from an environment variable.
+require('yargs')
+  .middleware(function (argv) {
+    if (process.env.HOME) argv.home = process.env.HOME
+  }, true)
+  .command('configure-home', "do something with a user's home directory",
+    {
+      'home': {
+        'demand': true,
+        'string': true
+      }
+    },
+    function(argv) {
+      console.info(`we know the user's home directory is ${argv.home}`)
+    }
+  )
+  .parse()
+```
+
 <a name="nargs"></a>.nargs(key, count)
 -----------
 
diff --git a/lib/argsert.js b/lib/argsert.js
index ed1d598..1c9134b 100644
--- a/lib/argsert.js
+++ b/lib/argsert.js
@@ -1,10 +1,12 @@
 'use strict'
+
+// hoisted due to circular dependency on command.
+module.exports = argsert
 const command = require('./command')()
 const YError = require('./yerror')
 
 const positionName = ['first', 'second', 'third', 'fourth', 'fifth', 'sixth']
-
-module.exports = function argsert (expected, callerArguments, length) {
+function argsert (expected, callerArguments, length) {
   // TODO: should this eventually raise an exception.
   try {
     // preface the argument description with "cmd", so
diff --git a/lib/command.js b/lib/command.js
index e643d16..7486592 100644
--- a/lib/command.js
+++ b/lib/command.js
@@ -2,7 +2,7 @@
 
 const inspect = require('util').inspect
 const isPromise = require('./is-promise')
-const {applyMiddleware} = require('./middleware')
+const {applyMiddleware, commandMiddlewareFactory} = require('./middleware')
 const path = require('path')
 const Parser = require('yargs-parser')
 
@@ -17,9 +17,10 @@ module.exports = function command (yargs, usage, validation, globalMiddleware) {
   let aliasMap = {}
   let defaultCommand
   globalMiddleware = globalMiddleware || []
-  self.addHandler = function addHandler (cmd, description, builder, handler, _middlewares) {
+
+  self.addHandler = function addHandler (cmd, description, builder, handler, commandMiddleware) {
     let aliases = []
-    const middlewares = (_middlewares || []).slice(0)
+    const middlewares = commandMiddlewareFactory(commandMiddleware)
     handler = handler || (() => {})
 
     if (Array.isArray(cmd)) {
@@ -224,6 +225,9 @@ module.exports = function command (yargs, usage, validation, globalMiddleware) {
       positionalMap = populatePositionals(commandHandler, innerArgv, currentContext, yargs)
     }
 
+    const middlewares = globalMiddleware.slice(0).concat(commandHandler.middlewares || [])
+    applyMiddleware(innerArgv, yargs, middlewares, true)
+
     // we apply validation post-hoc, so that custom
     // checks get passed populated positional arguments.
     if (!yargs._hasOutput()) yargs._runValidation(innerArgv, aliases, positionalMap, yargs.parsed.error)
@@ -231,9 +235,7 @@ module.exports = function command (yargs, usage, validation, globalMiddleware) {
     if (commandHandler.handler && !yargs._hasOutput()) {
       yargs._setHasOutput()
 
-      const middlewares = globalMiddleware.slice(0).concat(commandHandler.middlewares || [])
-
-      innerArgv = applyMiddleware(innerArgv, middlewares)
+      innerArgv = applyMiddleware(innerArgv, yargs, middlewares, false)
 
       const handlerResult = isPromise(innerArgv)
         ? innerArgv.then(argv => commandHandler.handler(argv))
diff --git a/lib/middleware.js b/lib/middleware.js
index f58593f..9f9c196 100644
--- a/lib/middleware.js
+++ b/lib/middleware.js
@@ -1,29 +1,61 @@
+'use strict'
+
+// hoisted due to circular dependency on command.
+module.exports = {
+  applyMiddleware,
+  commandMiddlewareFactory,
+  globalMiddlewareFactory
+}
 const isPromise = require('./is-promise')
+const argsert = require('./argsert')
 
-module.exports = function (globalMiddleware, context) {
-  return function (callback) {
+function globalMiddlewareFactory (globalMiddleware, context) {
+  return function (callback, applyBeforeValidation = false) {
+    argsert('<array|function> [boolean]', [callback, applyBeforeValidation], arguments.length)
     if (Array.isArray(callback)) {
+      for (let i = 0; i < callback.length; i++) {
+        if (typeof callback[i] !== 'function') {
+          throw Error('middleware must be a function')
+        }
+        callback[i].applyBeforeValidation = applyBeforeValidation
+      }
       Array.prototype.push.apply(globalMiddleware, callback)
     } else if (typeof callback === 'function') {
+      callback.applyBeforeValidation = applyBeforeValidation
       globalMiddleware.push(callback)
     }
     return context
   }
 }
 
-module.exports.applyMiddleware = function (argv, middlewares) {
+function commandMiddlewareFactory (commandMiddleware) {
+  if (!commandMiddleware) return []
+  return commandMiddleware.map(middleware => {
+    middleware.applyBeforeValidation = false
+    return middleware
+  })
+}
+
+function applyMiddleware (argv, yargs, middlewares, beforeValidation) {
+  const beforeValidationError = new Error('middleware cannot return a promise when applyBeforeValidation is true')
   return middlewares
     .reduce((accumulation, middleware) => {
+      if (middleware.applyBeforeValidation !== beforeValidation &&
+          !isPromise(accumulation)) {
+        return accumulation
+      }
+
       if (isPromise(accumulation)) {
         return accumulation
           .then(initialObj =>
-            Promise.all([initialObj, middleware(initialObj)])
+            Promise.all([initialObj, middleware(initialObj, yargs)])
           )
           .then(([initialObj, middlewareObj]) =>
             Object.assign(initialObj, middlewareObj)
           )
       } else {
-        const result = middleware(argv)
+        const result = middleware(argv, yargs)
+        if (beforeValidation && isPromise(result)) throw beforeValidationError
 
         return isPromise(result)
           ? result.then(middlewareObj => Object.assign(accumulation, middlewareObj))
diff --git a/yargs.js b/yargs.js
index ecae849..d7a6238 100644
--- a/yargs.js
+++ b/yargs.js
@@ -11,7 +11,7 @@ const Y18n = require('y18n')
 const objFilter = require('./lib/obj-filter')
 const setBlocking = require('set-blocking')
 const applyExtends = require('./lib/apply-extends')
-const middlewareFactory = require('./lib/middleware')
+const {globalMiddlewareFactory} = require('./lib/middleware')
 const YError = require('./lib/yerror')
 
 exports = module.exports = Yargs
@@ -33,7 +33,7 @@ function Yargs (processArgs, cwd, parentRequire) {
     updateFiles: false
   })
 
-  self.middleware = middlewareFactory(globalMiddleware, self)
+  self.middleware = globalMiddlewareFactory(globalMiddleware, self)
 
   if (!cwd) cwd = process.cwd()
 


Test patch:
diff --git a/test/middleware.js b/test/middleware.js
index a19a9fd..eb718c7 100644
--- a/test/middleware.js
+++ b/test/middleware.js
@@ -2,7 +2,7 @@
 /* global describe, it, beforeEach, afterEach */
 
 const {expect} = require('chai')
-const middlewareFactory = require('../lib/middleware')
+const {globalMiddlewareFactory} = require('../lib/middleware')
 let yargs
 require('chai').should()
 
@@ -10,22 +10,30 @@ describe('middleware', () => {
   beforeEach(() => {
     yargs = require('../')
   })
-
   afterEach(() => {
     delete require.cache[require.resolve('../')]
   })
+
   it('should add a list of callbacks to global middleware', () => {
     const globalMiddleware = []
 
-    middlewareFactory(globalMiddleware)(['callback1', 'callback2'])
+    globalMiddlewareFactory(globalMiddleware)([() => {}, () => {}])
 
     globalMiddleware.should.have.lengthOf(2)
   })
 
+  it('should throw exception if middleware is not a function', () => {
+    const globalMiddleware = []
+
+    expect(() => {
+      globalMiddlewareFactory(globalMiddleware)(['callback1', 'callback2'])
+    }).to.throw('middleware must be a function')
+  })
+
   it('should add a single callback to global middleware', () => {
     const globalMiddleware = []
 
-    middlewareFactory(globalMiddleware)(function () {})
+    globalMiddlewareFactory(globalMiddleware)(function () {})
 
     globalMiddleware.should.have.lengthOf(1)
   })
@@ -40,12 +48,10 @@ describe('middleware', () => {
         'adds func to middleware',
         function () {},
         function (argv) {
-          // we should get the argv filled with data from the middleware
           argv.mw.should.equal('mw')
           return done()
         }
       )
-      .exitProcess(false) // defaults to true.
       .parse()
   })
 
@@ -64,13 +70,12 @@ describe('middleware', () => {
         'adds func list to middleware',
         function () {},
         function (argv) {
-          // we should get the argv filled with data from the middleware
           argv.mw1.should.equal('mw1')
           argv.mw2.should.equal('mw2')
           return done()
         }
       )
-      .exitProcess(false) // defaults to true.
+      .exitProcess(false)
       .parse()
   })
 
@@ -103,7 +108,7 @@ describe('middleware', () => {
           argv.mw4 = 'mw4'
         }
       ])
-      .exitProcess(false) // defaults to true.
+      .exitProcess(false)
       .parse()
   })
 
@@ -137,6 +142,7 @@ describe('middleware', () => {
         .fail((msg, err) => {
           return done(Error('should not have been called'))
         })
+        .exitProcess(false)
         .parse()
     })
   })
@@ -158,7 +164,119 @@ describe('middleware', () => {
           count++
         }
       ])
+      .exitProcess(false)
       .parse()
     count.should.equal(2)
   })
+
+  it('allows middleware to be added in builder', (done) => {
+    yargs(['mw'])
+      .command(
+        'mw',
+        'adds func to middleware',
+        function (yargs) {
+          yargs.middleware(function (argv) {
+            argv.mw = 'mw'
+          })
+        },
+        function (argv) {
+          argv.mw.should.equal('mw')
+          return done()
+        }
+      )
+      .exitProcess(false)
+      .parse()
+  })
+
+  it('passes yargs object to middleware', (done) => {
+    yargs(['mw'])
+      .command(
+        'mw',
+        'adds func to middleware',
+        function (yargs) {
+          yargs.middleware(function (argv, yargs) {
+            expect(typeof yargs.help).to.equal('function')
+            argv.mw = 'mw'
+          })
+        },
+        function (argv) {
+          argv.mw.should.equal('mw')
+          return done()
+        }
+      )
+      .exitProcess(false)
+      .parse()
+  })
+
+  describe('applyBeforeValidation', () => {
+    it('runs before validation when "true"', function (done) {
+      yargs(['mw'])
+        .middleware(function (argv) {
+          argv.mw = 'mw'
+        }, true)
+        .command(
+          'mw',
+          'adds func to middleware',
+          {
+            'mw': {
+              'demand': true,
+              'string': true
+            }
+          },
+          function (argv) {
+            argv.mw.should.equal('mw')
+            return done()
+          }
+        )
+        .exitProcess(false)
+        .parse()
+    })
+
+    it('throws an error if "true" and async function provided', function () {
+      expect(() => {
+        yargs(['mw'])
+          .middleware([async function (argv) {
+            argv.mw = 'mw'
+            argv.other = true
+          }], true)
+          .command(
+            'mw',
+            'adds func to middleware',
+            {
+              'mw': {
+                'demand': true,
+                'string': true
+              }
+            },
+            function (argv) {
+              throw Error('we should not get here')
+            }
+          )
+          .exitProcess(false)
+          .parse()
+      }).to.throw('middleware cannot return a promise when applyBeforeValidation is true')
+    })
+
+    it('runs before validation, when middleware is added in builder', (done) => {
+      yargs(['mw'])
+        .command(
+          'mw',
+          'adds func to middleware',
+          function (yargs) {
+            // we know that this middleware is being run in the context of the
+            // mw command.
+            yargs.middleware(function (argv) {
+              argv.mw = 'mw'
+            }, true)
+          },
+          function (argv) {
+            argv.mw.should.equal('mw')
+            return done()
+          }
+        )
+        .demand('mw')
+        .exitProcess(false)
+        .parse()
+    })
+  })
 })