support variadic usage (rather than requiring an array)

[stephenmathieson]

this is just my ocd, but imo the latter looks much nicer:

app.use(route.get('/foo', compose([ middleware1, middleware2, routeHandler ])))

vs

app.use(route.get('/foo', compose(middleware1, middleware2, routeHandler)))

would you take a pr implementing this in a backward compatible way?

/cc @jonathanong

[jonathanong]

i would personally like:

function compose(...args) {
  const middleware = _.flatten(args)
  // ...
}

@tj @juliangruber @dead-horse @fengmk2 thoughts?

[tj]

SGTM

[stephenmathieson]

Awesome- I'll throw something together Monday if not sooner.

[jonathanong]

gonna hold on this for now... but i don't mind publishing this as v4 after we've understood the use-case for mutating middleware

[lewisdiamond]

Any updates? Can we just check if the first item is an array?

const _middlewares = Array.isArray(middlewares[0])
    ? middlewares[0]                              
    : middlewares                                 

[Runrioter]

IMO, it can be better if it supports both middleware and middlewares array compose([middleware1, middleware2], middleware3, [middleware4, middleware5])

BTW, it is overweight to use lodash

[lewisdiamond]

@Runrioter I'm not sure if that's really useful, but you can always add a check that composes them

if (Array.isArray(my_middleware_taken_from_input_list)) { middleware = compose(middleware) }

[lewisdiamond]

With this you get better performance (see below) and support both compose([m1,m2]) and compose(m1,m2). It passes all the tests but many of them need to be rewritten if you keep the unwrapped case for compose(m1), it may still hide bugs too.

function compose(...middlewares) {
    const _middlewares =
        Array.isArray(middlewares[0]) && middlewares.length === 1
        ? middlewares[0]
        : middlewares
    _middlewares.map(m => {
        if (typeof m !== "function")
            throw new TypeError("Middleware must be composed of functions!")
    })

    const noop = () => {}
    function _next(context, next, end) {
        let called = 0
        return Promise.resolve(
            next === _middlewares.length
            ? end(context, noop)
            : _middlewares[next](context, () => {
                if (!called++) return _next(context, next + 1, end)
                else throw new Error("next() called multiple times")
            })
        )
    }
    return function composed(context, next) {
        try {
            //Unwrap the case where there's a single middleware
            //It can catch test cases so tests need to be updated to test
            //with 2 or more middlewares as well as just 1.
            if (_middlewares.length === 1) {
                let called = 0
                return Promise.resolve(
                    _middlewares[0](context, () => {
                        if (!called++) return (next || noop)(context, noop)
                        else throw new Error("next() called multiple times")
                    })
                )
            // END
            } else return Promise.resolve(_next(context, 0, next || noop))
        } catch (err) {
            return Promise.reject(err)
        }
    }
}

Current implementation:

       1,130,866 op/s » (fn * 1)
         556,355 op/s » (fn * 2)
         280,713 op/s » (fn * 4)
         154,546 op/s » (fn * 8)
          76,328 op/s » (fn * 16)
          39,178 op/s » (fn * 32)
          18,403 op/s » (fn * 64)
           9,453 op/s » (fn * 128)
           4,572 op/s » (fn * 256)
           2,251 op/s » (fn * 512)
           1,042 op/s » (fn * 1024)

This implementation:

       1,308,892 op/s » (fn * 1) -- 15.7% (11.3% if you removed the optimization)
         610,887 op/s » (fn * 2)  -- 9.8%
         303,005 op/s » (fn * 4)  -- 7.9%
         158,465 op/s » (fn * 8)  -- 2.5% (Probably within margin of error from here on)
          77,234 op/s » (fn * 16) -- 1.2%
          38,029 op/s » (fn * 32) -- -3%
          18,585 op/s » (fn * 64) -- irrelevant
           9,410 op/s » (fn * 128)
           4,578 op/s » (fn * 256)
           2,232 op/s » (fn * 512)
           1,072 op/s » (fn * 1024)

[stephenmathieson]

I forgot the PR that added this got reverted and wasted a bunch of time debugging... any chance we'd be up for trying this again?

Also, any news on "the use-case for mutating middleware"?