Repository: yargs/yargs
Pull request #1215 (2018-09-08 10:30:18)

Fix type check in the .middleware() method
Resolves Issue #1214 


aorinevo: Looks good!

Reassigning to @bcoe.
BigstickCarpet: @bcoe - any estimate as to when this PR might be merged & published?
BigstickCarpet: btw... I also submitted [a PR on DefinitelyTyped](https://github.com/DefinitelyTyped/DefinitelyTyped/pull/28725) to add a type definition for the `.middleware()` method
aorinevo: > @bcoe - any estimate as to when this PR might be merged & published?

Hi @BigstickCarpet, 

Can you squash the commits and update the commit message to follow conventionalcommits.org guidelines?

Once that's done, I can merge this to master. 

Also, I see that AppVeyor build step is stuck.  I can't tell why, but hopefully this will be resolved with the above changes.

I can't estimate when the next release of yargs is going to get published, but there are ways to install non-release versions of yargs, see https://github.com/yargs/yargs/issues/1134.


BigstickCarpet: @aorinevo - It looks like AppVeyor just failed to update GitHub (maybe the webhook failed), but if you click the "Details" link, [the AppVeyor build passed successfully](https://ci.appveyor.com/project/bcoe/yargs-ljwvf/build/1483).

As for squashing the commits and editing the comments... I can do that myself if you want, but GitHub recently made it possible for repo owners to do this automatically when merging the PR.  Click the drop-down next to the "Merge" button and you'll see an option to squash the commits and replace the commit message.  It'll even automatically populate the squashed commit message with the concatenated messages of all the commits.  Really handy feature.  I use it on all of my personsal repos.

![squash](https://user-images.githubusercontent.com/3453903/45597358-b05b4200-b990-11e8-9523-28ae4f7876bf.PNG)

aorinevo: That works too, as long as the resulting commit message is inline conventionalcommits.org.  
bcoe: @aorinevo 👋 we should add docs about landing work! since it's all in my brain. You can just rewrite the commit message when calling "Squash and Merge", I'm fine with folks not following the convention until we land 👍 

Patch:
diff --git a/lib/middleware.js b/lib/middleware.js
index ca4280b..567ba57 100644
--- a/lib/middleware.js
+++ b/lib/middleware.js
@@ -2,7 +2,7 @@ module.exports = function (globalMiddleware, context) {
   return function (callback) {
     if (Array.isArray(callback)) {
       Array.prototype.push.apply(globalMiddleware, callback)
-    } else if (typeof callback === 'object') {
+    } else if (typeof callback === 'function') {
       globalMiddleware.push(callback)
     }
     return context


Test patch:
diff --git a/test/middleware.js b/test/middleware.js
index 0423768..295ec95 100644
--- a/test/middleware.js
+++ b/test/middleware.js
@@ -24,11 +24,30 @@ describe('middleware', () => {
   it('should add a single callback to global middleware', () => {
     const globalMiddleware = []
 
-    middlewareFactory(globalMiddleware)({})
+    middlewareFactory(globalMiddleware)(function () {})
 
     globalMiddleware.should.have.lengthOf(1)
   })
 
+  it('runs the middleware before reaching the handler', function (done) {
+    yargs(['mw'])
+      .middleware(function (argv) {
+        argv.mw = 'mw'
+      })
+      .command(
+        'mw',
+        '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()
+  })
+
   it('runs all middleware before reaching the handler', function (done) {
     yargs(['mw'])
       .middleware([
@@ -56,11 +75,9 @@ describe('middleware', () => {
 
   it('should be able to register middleware regardless of when middleware is called', function (done) {
     yargs(['mw'])
-      .middleware([
-        function (argv) {
-          argv.mw1 = 'mw1'
-        }
-      ])
+      .middleware(function (argv) {
+        argv.mw1 = 'mw1'
+      })
       .command(
         'mw',
         'adds func list to middleware',
@@ -69,12 +86,20 @@ describe('middleware', () => {
           // we should get the argv filled with data from the middleware
           argv.mw1.should.equal('mw1')
           argv.mw2.should.equal('mw2')
+          argv.mw3.should.equal('mw3')
+          argv.mw4.should.equal('mw4')
           return done()
         }
       )
+      .middleware(function (argv) {
+        argv.mw2 = 'mw2'
+      })
       .middleware([
         function (argv) {
-          argv.mw2 = 'mw2'
+          argv.mw3 = 'mw3'
+        },
+        function (argv) {
+          argv.mw4 = 'mw4'
         }
       ])
       .exitProcess(false) // defaults to true.