add Middleware support via server.middleware = [/*...*/]

[osi-jehrlich]

Relating to Add Middleware support to Mirage #41 .

Middleware

This PR adds middleware to miragejs via a new server.midleware = [...]

Example middleware which randomly returns a 500 response:

  function random500() {
    return (schema, req, next) => {
      return (Math.random() > 0.7)
        ? new Response(500, {}, 'no')
        : next();
    }
  }

Routes which use the middleware defined above:
(Edited to match latest update)

  routes() {
    this.middleware = [
      random500(),
      addABunchOfResponseHeaders(),
      // more middleware can be added here...
    ]

    // Subsequent routes will have that middleware
    this.get('/users', (schema, req) => {
      return new Response(204, {}, null);
    });

    this.middleware = [
      // differentMiddleware
    ]
     
    this.get('/frogs', (schema, req) => {
      return new Response(204, {}, null);
    });
  }

Not included

1. There's a bit of funkyness when middleware wants to modify the response from a route handler because the route handler might return a Response object or it might return something else altogether. I imagine this could be addressed separately.

2. async middleware - again, could be added after (if this is the approach this repo heads in).

[osi-jehrlich]

Just realized an even simpler API was staring me right in the face, similar to the API to set a namespace:

  routes() {
    this.middleware = [
      random500(),
      addABunchOfResponseHeaders(),
      // more middleware can be added here...
    ]

    // Subsequent routes will have that middleware
    this.get('/users', (schema, req) => {
      return new Response(204, {}, null);
    });

    this.middleware = [
      // differentMiddleware
    ]
     
    this.get('/frogs', (schema, req) => {
      return new Response(204, {}, null);
    });
  }

I've update this PR to have this API. It still has withMiddleware, too, but that's now optional. Thoughts?

[IanVS]

Hi @osi-jehrlich, thanks for the contribution! I have to admit that after the original maintainer of this project left, we're just a skeleton crew keeping the lights on, so I'm a bit reluctant to add new features. That said, this does seem like it could be pretty useful, and you've added both tests and types so I feel a bit more comfortable with it. I'd argue against adding two different APIs that accomplish the same thing, and your new API feels a bit more similar to how other parts of mirage work, so I'd suggest removing withMiddleware.

I'd also like to hear what @cah-brian-gantzler thinks of this approach.

[IanVS]

With this approach, if middleware wants to change the request, it needs to mutate the request parameter directly before calling next(), right? What would you think about also allowing middleware to call next(request), to be able to pass a cloned or even completely new request object into the subsequent handler? I think something like this could work, while still allowing next() to be called without arguments, right?

Suggested change

	() => {
	return this.handler.handle(request);
	(req = request) => {
	return this.handler.handle(req);

[osi-jehrlich]

I like the idea quite a bit; it'll need a couple more lines of code and type definitions, too - I'll get on that. I haven't made new Requests from old Requests, but as long as all the data is available to copy over it sounds sound.

Removing withMiddleware seems like a good idea, too. I think it's easy enough for library consumers to write a standalone version of that if they need to once the simpler API is available. We can always add it at some point if there's demand.

[osi-jehrlich]

Updated :-)

[osi-jehrlich]

we're just a skeleton crew keeping the lights on

Is there anything else I can do to help? I haven't yet updated the documentation (api docs, guides, etc.), and I'm happy to do exactly that if we think this PR is on its way in.

[IanVS]

Thanks, I'd like to hear what @bgantzler thinks of this approach, but he's been busy lately.

[cah-brian-gantzler]

Given what I see Im fine with it.

Future changes could be that the default handler function is supplied as the last element. While I have already extracted the default handlers so that we could supply a Pretender or the MSW variation. With this change Im wondering if there isnt a way to further extract that to make the Pretender and MWS version BE just another middleware (I realize they are since the handler is in the last spot, but I bet theres room for improvement and simplification of the abstract handler). Just a thought, could be wrong. Would have to explore to see if that could be done or not.

I am fine with this PR. I understand it, and its a pretty simple change considering what its doing.

[osi-jehrlich]

FYI, I've created a PR with documentation to the repo for the main website. If you get a chance to take a look I'd appreciate any and all feedback.

Is there anything else I can do (or need to do) to help move this PR toward merge/release?

[dodalovicgran]

Hi, how to call async calls in middlewares? In my case i want to check jwt using

await jose.jwtVerify(getJwt(request), jwtSecret)

which seems not to work?

[osi-jehrlich]

1. We didn't add explicit async support into middleware yet.
2. That said, I believe some asynchronous middleware should already work, see these tests (in a branch where the only change is adding those tests).

Perhaps we can move this discussion to the miragejs/discuss repo?

[Update]: I can't get async middleware to NOT work as long as it's aware that downstream middleware might return a promise. Perhaps the Typescript type definition needs to be updated to specifically state a Promise<...> is a possible return value.
[Update2]: The Typescript type definition looks like it already uses MaybePromise.

[dodalovicgran]

Thank you @osi-jehrlich This worked for me!