Allow a fetch function to be passed into apiMiddleware

[nwwells]

This was done in the most API-preserving way possible. Another (breaking) approach would be to force the library user to initialize the apiMiddleware with the given fetch. I'd actually prefer that, but thought that preserving the existing API would be the most important priority right now.

Note also that this way of doing it is slightly leaky as passing in fetch essentially modifies a global-ish variable. The test is actually quite leaky, since any test after it will be executed with the mocked fetch.

This was the result of some rapid prototyping. I just want to see if the idea has merit, and get some guidance on the right direction before getting too far into either approach.

[coveralls]

Coverage remained the same at 100.0% when pulling 5d8390a on symbiont-io:master into 1543910 on agraboso:master.

[Darmody]

This approach in the most API-preserving way, which I really forgot considering about.

Replace the fetch function can resolve my problem, but it looks heavily for just add some hooks for the middleware, I mean, for a package user.

[cevou]

This is not needed anymore if #77 is merged. You can then just override the global fetch.

[nwwells]

@cevou but what if I want to wrap the fetch function for just this module and not globally (which is my use case)?

[cevou]

You can easily override global fetch before your code is executed and reset it afterwards. This however only works if no other module calls the fetch function in between. The question is why you are wrapping a standard API function in your module?

[mlippens]

@cevou then you are patching a global twice just for one use case? Let's assume that we follow @nwwells approach here, does it really have a downside but not following the fetch spec and providing a hook in the middleware? If I turn it around, to me the only reason to choose #77 is that it is conform with the fetch spec.

[nwwells]

I wrap the fetch function because I want to do some general handling before redux-api-middleware sees the result. I don't modify the response at all, and my wrapping function maintains the fetch interface.

why you are wrapping a standard API function in your module?

Because wrapping is better than patching. Because modifying the global object is dangerous. Because assuming that a given function executes synchronously is generally a bad idea, and because it just doesn't work in this case specifically (the only obvious way to do it would be to override the headers CALL_API action function, but then I don't have a way of reseting it).

[cevou]

@froginvasion I think you should not patch the global function at all. Especially, if you don't want to change the actual functionality but rather do some preprocessing of the result.
Wouldn't it be better to have some preprocessing hook/callback that you could just pass in the CALL_API action that is then executed before the result is passed on?

[alexanderchr]

Another way to do this in a non-breaking way is to add a function to the exported middleware function.

apiMiddleware.withFetch(fetchImplementation)

See reduxjs/redux-thunk#70

[migueloller]

The correct way for an external library to use fetch is to either allow the developer to provide their own polyfill in the global (This is the best option since this is the behavior that will happen once the standard is implemented. And it's already implemented in Chrome and React Native) or let the developer pass it as an initialization parameter (This is less desirable but still compliant).

The current way causes a whole slew of problems as we can see being discussed here, and the mocking and React Native issues discussed in #77.

[teofilomonteiro]

So when is this going to be release? I think it will be a great improvement since we will be able to mock the fetch request.

[MrOggy85]

LGTM

[nwwells]

@MrOggy85, can you then merge it? I don't have write access to the repo.

[coveralls]

Coverage remained the same at 100.0% when pulling 6d5cc7a on symbiont-io:master into 1543910 on agraboso:master.

[nwwells]

Closing, as we are deprecating our use of this middleware.