redux-api-middleware
redux-api-middleware copied to clipboard
Clone the response object before passing to payload/meta functions
When implementing meta functions, one must currently create a clone of the response object in order to read the response body. To do that, they must also implement the payload function, clone the response, and return the json. This behavior has been documented a few times that I've found so far:
- https://github.com/agraboso/redux-api-middleware/issues/134
- https://github.com/agraboso/redux-api-middleware/issues/92
By calling those functions using the clone, we avoid this issue.
Notes:
- I had to update the test snapshots because the
ReadableStreamare no longer marked as disturbed after cloning.
Coverage remained the same at 100.0% when pulling 6e56b243e41f55c305b0839375fe87171ac77165 on NickWer:master into 6e782cdea6d5451e6e96406de631db7c11df1a2d on agraboso:master.
I realized I have made a mistake. Will push a fix shortly and unmark as WIP when it's ready. My apologies.
Should be OK now. I added a little unit test, too.
I agree with this PR, but I think user should be responsible for reading the response in a correct way. As a user I would just made my own getJSON function.
Cloning a big response 2 times, can be a little too much :)
import { getJSON as noCloneGetJSON } from 'redux-api-middleware'
export function getJSON(res) {
return noCloneGetJSON(res.clone())
}
Thank you @NickWer, and sorry for the delay 😿This PR looks good 👍
This does seem to be a common pain point, which this approach would help avoid.
user should be responsible for reading the response in a correct way Cloning a big response 2 times, can be a little too much :)
@iamandrewluca great feedback, thank you. Are there downsides to cloning the response internally? Is your concern mostly about performance or something else?
@nason Yeap, mostly about performance. But I don't think there is such big performance lose.