feat: Allow async requestMiddleware

[shkreios]

Currently, it's not possible to pass any async data to request middlewares. For example, fetching and auth token. I think using Promise.resolve is the cleanest solution and shouldn't add any noticeable overhead to non async middlewares.

[shkreios]

Closes #369

[jasonkuhrt]

LGTM, but please add tests.

[blaenk]

Nice, I was just now reaching for this, in particular the ability to use the "set headers to be a function" albeit an async one, which it currently doesn't support. Not sure if you'd like to take that up for this PR as well, it seems middleware will work as well for this purpose.

One thing I'm curious about is, what version of graphql-request actually supports middleware? The readme confusingly talks about it but the latest stable version 4.3.0—the version mentioned in the readme badge—doesn't seem to have middleware implemented.

It seems that one should be using the next version 5.0.0-next.5 instead?

[jasonkuhrt]

Will cut a release soon

[shkreios]

LGTM, but please add tests.

Sure will try to orientate myself along the existing ones. Or do you have any specific guidelines for test that should be followed?

[shkreios]

Nice, I was just now reaching for this, in particular the ability to use the "set headers to be a function" albeit an async one, which it currently doesn't support. Not sure if you'd like to take that up for this PR as well, it seems middleware will work as well for this purpose.

As the header function is resolved in a sync function, adding the option to return a promise would require changing the request method substantially. As I don't know anything about the plans for graphql-request or any architecture or design plan it might have, I would suggest pushing these changes into another PR.

[blaenk]

Sure will try to orientate myself along the existing ones. Or do you have any specific guidelines for test that should be followed?

My unsolicited/uninformed opinion: like the existing ones but wrap them in promises instead 🤷🏼‍♂️

As the header function is resolved in a sync function, adding the option to return a promise would require changing the request method substantially. As I don't know anything about the plans for graphql-request or any architecture or design plan it might have, I would suggest pushing these changes into another PR.

That's totally fair! This middleware approach appears to enable what I want to do anyway: asynchronously fetch an authorization token to attach to the request headers.

[shkreios]

My unsolicited/uninformed opinion: like the existing ones but wrap them in promises instead 🤷🏼‍♂️

Currently, the test setup for middlewares tests both request and response middlewares. I just copied the request part of the other tests and added the async mock. I don't know if that is enough. Maybe after releasing the middleware feature and people find issues, it becomes more clear what needs to be covered by unit tests.