Skip to content

Commit

Permalink
chore: address @aorinevo's code review so that we can land
Browse files Browse the repository at this point in the history
  • Loading branch information
bcoe committed Jan 28, 2019
1 parent f3a4e4f commit bc0ee40
Show file tree
Hide file tree
Showing 6 changed files with 71 additions and 55 deletions.
8 changes: 7 additions & 1 deletion docs/advanced.md
Original file line number Diff line number Diff line change
Expand Up @@ -478,6 +478,9 @@ const normalizeCredentials = (argv) => {
}
return {}
}
// Add normalizeCredentials to yargs
yargs.middleware(normalizeCredentials)
```

### Example Async Credentials Middleware
Expand All @@ -496,6 +499,9 @@ const normalizeCredentials = (argv) => {
}
return {}
}
// Add normalizeCredentials to yargs
yargs.middleware(normalizeCredentials)
```

#### yargs parsing configuration
Expand All @@ -508,7 +514,7 @@ var argv = require('yargs')
.option('password')
} ,(argv) => {
authenticateUser(argv.username, argv.password)
},
},
[normalizeCredentials]
)
.argv;
Expand Down
25 changes: 3 additions & 22 deletions lib/command.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
'use strict'

const inspect = require('util').inspect
const isPromise = require('./is-promise')
const {applyMiddleware} = require('./middleware')
const path = require('path')
const Parser = require('yargs-parser')

Expand Down Expand Up @@ -232,24 +234,7 @@ module.exports = function command (yargs, usage, validation, globalMiddleware) {

const middlewares = commandHandler.middlewares || []

innerArgv = middlewares
.reduce((accumulation, middleware) => {
if (isPromise(accumulation)) {
return accumulation
.then(initialObj =>
Promise.all([initialObj, middleware(initialObj)])
)
.then(([initialObj, middlewareObj]) =>
Object.assign(initialObj, middlewareObj)
)
} else {
const result = middleware(innerArgv)

return isPromise(result)
? result.then(middlewareObj => Object.assign(accumulation, middlewareObj))
: Object.assign(accumulation, result)
}
}, innerArgv)
innerArgv = applyMiddleware(innerArgv, middlewares)

const handlerResult = isPromise(innerArgv)
? innerArgv.then(argv => commandHandler.handler(argv))
Expand Down Expand Up @@ -449,7 +434,3 @@ module.exports = function command (yargs, usage, validation, globalMiddleware) {

return self
}

function isPromise(maybePromise) {
return maybePromise instanceof Promise
}
3 changes: 3 additions & 0 deletions lib/is-promise.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
module.exports = function isPromise (maybePromise) {
return maybePromise instanceof Promise
}
23 changes: 23 additions & 0 deletions lib/middleware.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
const isPromise = require('./is-promise')

module.exports = function (globalMiddleware, context) {
return function (callback) {
if (Array.isArray(callback)) {
Expand All @@ -8,3 +10,24 @@ module.exports = function (globalMiddleware, context) {
return context
}
}

module.exports.applyMiddleware = function (argv, middlewares) {
return middlewares
.reduce((accumulation, middleware) => {
if (isPromise(accumulation)) {
return accumulation
.then(initialObj =>
Promise.all([initialObj, middleware(initialObj)])
)
.then(([initialObj, middlewareObj]) =>
Object.assign(initialObj, middlewareObj)
)
} else {
const result = middleware(argv)

return isPromise(result)
? result.then(middlewareObj => Object.assign(accumulation, middlewareObj))
: Object.assign(accumulation, result)
}
}, argv)
}
32 changes: 0 additions & 32 deletions test/command.js
Original file line number Diff line number Diff line change
Expand Up @@ -1458,36 +1458,4 @@ describe('Command', () => {
return done()
})
})

// addresses https://github.com/yargs/yargs/issues/1237
it('fails when the promise returned by the middleware rejects', (done) => {
const error = new Error()
const handlerErr = new Error('should not have been called')
yargs('foo')
.command('foo', 'foo command', noop, (yargs) => done(handlerErr), [ (yargs) => Promise.reject(error) ])
.fail((msg, err) => {
expect(msg).to.equal(null)
expect(err).to.equal(error)
done()
})
.argv
})

it('calls the command handler when all middleware promises resolve', (done) => {
const middleware = (key, value) => () => new Promise((resolve, reject) => {
setTimeout(() => {
return resolve({ [key]: value })
}, 5)
})
yargs('foo hello')
.command('foo <pos>', 'foo command', () => {}, (yargs) => {
yargs.hello.should.equal('world')
yargs.foo.should.equal('bar')
done()
}, [ middleware('hello', 'world'), middleware('foo', 'bar') ])
.fail((msg, err) => {
return done(Error('should not have been called'))
})
.argv
})
})
35 changes: 35 additions & 0 deletions test/middleware.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
'use strict'
/* global describe, it, beforeEach, afterEach */

const {expect} = require('chai')
const middlewareFactory = require('../lib/middleware')
let yargs
require('chai').should()
Expand Down Expand Up @@ -105,4 +106,38 @@ describe('middleware', () => {
.exitProcess(false) // defaults to true.
.parse()
})

// addresses https://github.com/yargs/yargs/issues/1237
describe('async', () => {
it('fails when the promise returned by the middleware rejects', (done) => {
const error = new Error()
const handlerErr = new Error('should not have been called')
yargs('foo')
.command('foo', 'foo command', () => {}, (argv) => done(handlerErr), [ (argv) => Promise.reject(error) ])
.fail((msg, err) => {
expect(msg).to.equal(null)
expect(err).to.equal(error)
done()
})
.parse()
})

it('calls the command handler when all middleware promises resolve', (done) => {
const middleware = (key, value) => () => new Promise((resolve, reject) => {
setTimeout(() => {
return resolve({ [key]: value })
}, 5)
})
yargs('foo hello')
.command('foo <pos>', 'foo command', () => {}, (argv) => {
argv.hello.should.equal('world')
argv.foo.should.equal('bar')
done()
}, [ middleware('hello', 'world'), middleware('foo', 'bar') ])
.fail((msg, err) => {
return done(Error('should not have been called'))
})
.parse()
})
})
})

0 comments on commit bc0ee40

Please sign in to comment.