Skip to content

Add recursive composition (semver-minor) #81

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 3 commits into from
Closed

Add recursive composition (semver-minor) #81

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
3faadfe
tools: added package-lock to gitignore
fl0w Jun 18, 2017
bac31e9
support recursive composition (semver-minor)
fl0w Jun 18, 2017
5bb378f
lint: --fix
fl0w Jun 18, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
node_modules
coverage
*.log
package-lock.json
13 changes: 11 additions & 2 deletions index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,11 @@
'use strict'

/**
* Module dependencies.
*/

const { isArray } = Array

/**
* Expose compositor.
*/
Expand All @@ -17,9 +23,11 @@ module.exports = compose
*/

function compose (middleware) {
if (!Array.isArray(middleware)) throw new TypeError('Middleware stack must be an array!')
if (!isArray(middleware)) throw new TypeError('Middleware stack must be an array!')
for (const fn of middleware) {
if (typeof fn !== 'function') throw new TypeError('Middleware must be composed of functions!')
if (typeof fn !== 'function' && !isArray(fn)) {
throw new TypeError('Middleware must be composed of functions, or array of functions!')
}
}

/**
Expand All @@ -36,6 +44,7 @@ function compose (middleware) {
if (i <= index) return Promise.reject(new Error('next() called multiple times'))
index = i
let fn = middleware[i]
if (isArray(fn)) fn = compose(fn)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

my concern here is that we compose every time we call dispatch. this might have performance issues.

i'd prefer to flatten the array at compose(), but that might not be what you want as you won't be able to edit nested middleware during runtime (which i don't think you should be doing)

Copy link
Contributor Author

@fl0w fl0w Nov 6, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I remember correctly, the point was not to mutate middleware at runtime, but to allow consumer (Koa) to pass nested/wrapped as an array - and instead of flattening the array I simply opted to compose recursively (saw it as a quality of life feature)

But you're right, I would compose on each dispatch. Maybe minuscule penalty, but definitely unnecessary as pre-flattening would avoid that.

But, this could be easily done by consumer as well. The whole point was to allow Koa to add some clean logic to wrap each middleware, but I'm not convinced this is of concern anymore. If anything, I think this can be closed and forgotten (I already had).

if (i === middleware.length) fn = next
if (!fn) return Promise.resolve()
try {
Expand Down
108 changes: 107 additions & 1 deletion test/test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,112 @@ function wait (ms) {
}

describe('Koa Compose', function () {
describe('when rescurive', function () {
it('should work', () => {
const arr = []
const stack = []

stack.push(async (ctx, next) => {
arr.push(1)
await wait(1)
await next()
await wait(1)
arr.push(8)
})

stack.push([
async (ctx, next) => {
arr.push(2)
await wait(1)
await next()
await wait(1)
arr.push(7)
},
async (ctx, next) => {
arr.push(3)
await wait(1)
await next()
await wait(1)
arr.push(6)
}
])

stack.push(async (ctx, next) => {
arr.push(4)
await wait(1)
await next()
await wait(1)
arr.push(5)
})

return compose(stack)({}).then(function () {
arr.should.eql([1, 2, 3, 4, 5, 6, 7, 8])
})
})

it('should be able to be called twice', () => {
const stack = []

stack.push(async (context, next) => {
context.arr.push(1)
await wait(1)
await next()
await wait(1)
context.arr.push(8)
})

stack.push([
async (context, next) => {
context.arr.push(2)
await wait(1)
await next()
await wait(1)
context.arr.push(7)
},
async (context, next) => {
context.arr.push(3)
await wait(1)
await next()
await wait(1)
context.arr.push(6)
}
])

stack.push(async (context, next) => {
context.arr.push(4)
await wait(1)
await next()
await wait(1)
context.arr.push(5)
})

const fn = compose(stack)
const ctx1 = { arr: [] }
const ctx2 = { arr: [] }
const out = [1, 2, 3, 4, 5, 6, 7, 8]

return fn(ctx1).then(() => {
assert.deepEqual(out, ctx1.arr)
return fn(ctx2)
}).then(() => {
assert.deepEqual(out, ctx2.arr)
})
})

it('should throw if next() is called multiple times within recursion', function () {
return compose([[
async (ctx, next) => {
await next()
await next()
}
]])({}).then(() => {
throw new Error('boom')
}, (err) => {
assert(/multiple times/.test(err.message))
})
})
})

it('should work', function () {
var arr = []
var stack = []
Expand Down Expand Up @@ -98,7 +204,7 @@ describe('Koa Compose', function () {
return compose([])({})
})

it('should only accept middleware as functions', function () {
it('should accept middleware as functions', function () {
var err
try {
(compose([{}])).should.throw()
Expand Down