Proof-of-concept for custom network adapters

[iamlacroix]

As the title states, this just a simple example which adds the ability to inject a network adapter to the milddleware. I'm adding this mostly to open up the discussion of this feature.

With this feature, we could create our own adapter to set headers/auth, or to even use a different request library in place of SuperAgent.

The idea is to update the queryMiddleware's signature so that an adapter is passed as the 3rd argument. The "adapter" in this example is a function that accepts three args:

• url
• method
• body (optional)

The adapter will return an object with two properties, both of which contain functions:

• execute
• abort

The execute function requires a callback, which will receive (at this moment) the following args:

• err
• resOk
• resStatus
• resBody
• resText

The main adapter function is called by the middleware and the returned object is then used to call and cancel requests.

The execute function's return args mirror the original variables that were used in the middleware, but we'll likely want to remove the resOk argument in order to simplify the API. We could just ensure the status code is 2xx instead.

Tests are passing, and the async example project is working with this POC, however there might be more I'm missing since I haven't read through the entire source.

Anyway, let me know your thoughts on this type of feature.

[codecov-io]

Codecov Report

Merging #4 into master will increase coverage by 1.55%.
The diff coverage is 79.31%.

@@            Coverage Diff            @@
##           master      #4      +/-   ##
=========================================
+ Coverage   88.34%   89.9%   +1.55%     
=========================================
  Files           7       8       +1     
  Lines         206     208       +2     
=========================================
+ Hits          182     187       +5     
+ Misses         24      21       -3

Impacted Files	Coverage Δ
src/middleware/query.js	94.44% <100%> (+5.98%)	✅
src/adapters/superagent.js	75% <75%> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 32655ce...57c1868. Read the comment docs.

[ryanashcraft]

@iamlacroix The changes looks good as a proof of concept. I do have some concerns but first I'd like to get a better understanding of your end-goals. Smaller bundle size? Is there something you need that superagent doesn't support? I think it may make sense to have the ability to swap in a different library, but I want to make sure we make progress in the right areas.

As far as supporting headers/auth, @adamsanderson and I are discussing some simple changes to support those here: #3

[iamlacroix]

Hey @ryanashcraft, our main goals are:

• Reducing bundle size.
• Using a different request library. We already have one in use across ~25 apps.

Happy to help where needed.

[ryanashcraft]

@iamlacroix Makes sense. I want to do some thinking about the right API for this.

Some early thoughts:

• Ideally the superagent adapter would be the default adapter for queryMiddleware so you wouldn't have to provide one for basic usages. However, I don't think there's a clean way to do that without module loaders bundling in superagent (without hacky configuration).
• I'm worried about going down a route where you have to install a separate package to get redux-query to work at all (for example, you have to install redux-query and redux-query-superagent). So superagent will still need to be an npm dependency. I think that should be fine as long as we can get the resulting builds to not include superagent. Probably will want need to have the built adapters live at different module paths (i.e. not exported by the main index.js) like 'redux-query/adapters/superagent'.
• Would be nice if we can get different UMD builds for the different adapter options.
• I agree with you about nixing resOk. I'd be for basing it off the status code being a number between 200 and 300 (exclusive). Internally we use a function isOkStatus that pretty much does this.

[iamlacroix]

@ryanashcraft I agree on keeping it together as one package. I don't think you should have to maintain separate adapters within the repo either (in case I was sending that message).

As you mentioned, if we could find a way to have the default build inject the superagent adapter and an alternate build w/o, I think that could work well.

Anyway, great work on this library!

[ryanashcraft]

@iamlacroix 👍

Thinking more about this. My next question: what request library are you using that you'd like to use with redux-query?

[ryanashcraft]

If it's a public library like request it'd be nice to check it into the repo so others can use.

I'm currently thinking a nice way we could do this is a separate main entry point for "advanced usage" for being able to set the request library adapter. Using it would look something like this:

import { queryMiddlewareAdvanced } from "redux-query/advanced";

queryMiddlewareAdvanced(myCustomAdapter)(queriesSelector, entitiesSelector)

Then the existing query-middleware.js would become query-middleware-advanced.js with a new signature: (adapter) => (queriesSelector, entitiesSelector, ...) =>. The normal entry point would continue to export queryMiddleware, which will just be queryMiddlewareAdvanced bound with the superagent adapter.

Great thing is I don't think this is too far from your proof of concept.

This is sort of similar to what react-redux does with connectRequest. And redux-form does a similar thing with "redux-form/immutable".

[iamlacroix]

It's not public unfortunately. Funny thing is it's actually a wrapper around superagent, so we could try to finagle its version so that npm/yarn can resolve to one instance.

We could try this initially to reduce the amount of work around the build process aspect (if that helps), as long there is a way for us to inject it into the middleware. Your latest example above is a great approach.

[ryanashcraft]

Alright cool. I'm feeling pretty good about this. Are you interested in making these changes on top of your PR?

[iamlacroix]

Yea, I'd be happy to make these changes. Thanks for discussing this!

[iamlacroix]

Superseded by #23