Add response handler #171
Add response handler #171
Conversation
src/middleware.js
Outdated
| return next(action); | ||
| } | ||
| function createMiddleware(options = {}) { | ||
| const middlewareOptions = Object.assign({}, defaults, options); |
There was a problem hiding this comment.
Thanks for the PR! I've only skimmed over this but have one immediate concern -- the 'responseOk' config seems much more useful on a per-request basis (ie, a property on callAPI below) than on the middleware as a whole.
Perhaps I'm misunderstanding the implementation here, but what are your thoughts? In either case, we'll need some docs explaining how to use this 😆
|
@nason Yes for response check makes sense per-request basis.
With the new |
|
Hey @iamandrewluca are you still interested in working on this? My opinion is that adding per-RSAA options (vs global middleware config) is the most useful for now, but I could be convinced otherwise 😆 |
Now user can provide a global ok/fetch handler or a local ok/fetch handler Local ok/fetch handler will take priority
|
Does this solve redirect handling? Is there an ETA? |
|
@shulcsm yes, with this PR, you could do your own response status handling, need to write some test, and may be merged. |
There was a problem hiding this comment.
I finally got to spend some time looking at this. Great work @iamandrewluca!
I left a few comments below, and I agree the bulk of work that remains is tests and documentation. I'm happy to jump in with anything thats left, just let me know how I can be helpful.
| @@ -176,7 +174,7 @@ function apiMiddleware({ getState }) { | |||
| } | |||
|
|
|||
| // Process the server response | |||
| if (res.ok) { | |||
| if (ok(res)) { | |||
There was a problem hiding this comment.
Lets handle errors thrown from the ok function in a try/catch here. Should they be handled like those from doFetch above?
| */ | ||
| function apiMiddleware({ getState }) { | ||
| return next => action => { | ||
| function createMiddleware(options = {}) { |
There was a problem hiding this comment.
We should document this and how to use it 📚
Would configuring defaults for your middleware be like:
const apiMiddlewareWithDefaults = createMiddleware({
fetch: myFetch,
ok: myOk
})when configuring your store? What other defaults can you set this way?
And then you can override from individual RSAAs?
{
[RSAA]: {
...
ok: myOtherOk
...
}
}
|
@iamandrewluca I opened #194 from this branch, rebased it onto I'm going to close this PR and merge the updated one into |
Add custom `ok` hander (#171) + cleanup, docs, and tests
|
This has been released in |
|
@nason Thanks! |
|
Any example what is good practice here? |
Added a wrapper for apiMiddleware so user can pass his own check if response is OK
User can import as usual
apiMiddlewareOr can import function
createMiddlewarethat when called with options will return theapiMiddlewareTODO:
global JSON response handlerper-request JSON response handlerCloses #170