using `compose` for returning 3 and 4 arity middlewares

[xgbuils]

Hi @blakeembrey

I find your package and it's just what I was looking for. I really like this package because it can helps to split my middlewares into little pieces and compose them. My congrats!

However I would like to understand why is errors function needed. I think it's possible to have just one compose function:

• If the first middleware has less than 4 arguments, then it returns a 3-arity middleware.
• If the first middleware has 4 arguments, then it returns a 4-arity middleware.

I'm basing this proposal on that usual composition returns a function with the same arity than first called function.

What do you think?

[blakeembrey]

I'm not particularly interested because it makes it harder, maybe impossible, to type the return type correctly. It'd also be a breaking change, which seems pretty unnecessary at this point in time. I might re-visit at some point and decide it's worth doing, but right the functionality is simple enough I don't see any major benefit of combining the APIs - happy to hear if that's not true.

[xgbuils]

I think it could be an oportunity for design a better API.

1. Intuitive API

Coming from APIs like Ramda, it's usual to have utilities for composing functions:

const R = require('ramda');

const addThenIncrement = R.pipe(
    (a, b, c) => a + b + c,
    (n) => n + 1
);

// (5 + 10 + 20) + 1
addThenIncrement(5, 10, 20) // 36

and the same happens with lodash:

const _ = require('lodash');

const addThenIncrement = _.flow(
    (a, b, c) => a + b + c,
    (n) => n + 1
);

// (5 + 10 + 20) + 1
addThenIncrement(5, 10, 20) // 36

2. More strict API

Currently, compose and errors accept to receive a Middleware or an arbitrary deep nested array of Middlewares.

At first point, following the single responsability principle, it would be nice to remove flattening concern from this project. Flattening or no flattening middlewares is an optional feature that could be used or not by the client of this library.

Then we would have compose and errors with a variadic number of Middleware arguments:

function compose(...handlers: Middleware<T, U, V>[]): Middleware<T, U, V>;

Once we have this signature and after applying my proposal, we can make compose more strict for better type inferences. We can give multiple signature types for the compose function:

function compose(): RequestHandler<T, U, V>;
function compose(requestHandler: RequestHandler<T, U, V>, ...handlers: Middleware<T, U, V>[]): RequestHandler<T, U, V>;
function compose(errorHandler: ErrorHandler<T, U, V>, ...handlers: Middleware<T, U, V>[]): ErrorHandler<T, U, V>;

3. Simple API

Then, errors function won't be necessary. We just will have a compose method which works like other composing utilities. We won't have counter-intuitive behaviours like:

   const middleware = compose(
      function (err: Error, req: any, res: any, next: Next) {
        req.one = true
        next()
      }
    )
    // middleware.length 3 !?

As I'm composing just one middleware, I'm expecting that compose returns the same middleware. However currently it's returning a RequestHandler middleware when I'm passing a ErrorHandler middleware to compose.

PS. It seems that my proposal works well with counter-intuitive example exposed in #4 issue without need of clarification.

[blakeembrey]

@xgbuils I think we have a misunderstanding. I have no qualms with the proposal, I have qualms with backward compatibility. If you'd like to put up a PR, I'll review it - I agree it's probably worth the changes.

[xgbuils]

Hi @blakeembrey, I would love to create a PR.

In the sense that I'm proposing to remove errors function and flattening inputs, my PR won't be backward compatible. First I'm interested to proceed in this way:

1. create a first PR that removes flattening input feature:

function compose(...middlewares: Middleware<T, U, V>[]): RequestHandler<T, U, V>;
function errors(...middlewares: Middleware<T, U, V>[]): ErrorHandler<T, U, V>;

2. Create a second PR with a commit adding tests that defines how I would like to compose behaves.

3. After reaching an agreement with the tests, I would like to make a second commit implementing the solution.

If you like, you could create a dev or next-major branch for not poluting master after you approve the whole changes.

Cheers!

[blakeembrey]

@xgbuils You're fine to create a PR doing all these things together. I think we're aligned on the API, it makes sense 👍

[blakeembrey]

One interesting thing to state is that a few people get confused by the function arity in Express.js, so it may even be worth considering alternate APIs that don't rely on function arity (this only need to be a consideration of potential solution) if we're doing a breaking change either way.