jamesplease
commented
6 years ago I spent more time thinking about this. Rather than using the request key stuff, I think that updateState should be called with a custom Error if a new request is configured while a previous one is still in flight.
This effectively takes the place of "aborting" the previous request until the abort API is more broadly supported.
⚠️ Until this lands, it is dangerous to use Holen in a production app
Holen is susceptible to race condition bugs. One situation where it is more noticeable is when network conditions aren't great, or when you have one slow endpoint and one fast endpoint.
Consider a situation where every HTTP request takes 30 seconds, because a user is on a mobile device and the connection is spotty.
The application is using Holen for requests to a search. Each time that the user types, a debounced request is sent off.
The user types "Lo", and after a moment a search request is sent off.
10 seconds pass, and the request has not ended. The user types an r, so a second request is kicked off for "Lor".
Holen won't behave as expected in this situation. Given another 20 seconds, the first request's response will be received, and given the way most people author React applications, the user will see the results for "Lo". Because
loadingis no longer passed to therenderprop callback, the spinner has disappeared, too. The user has good reason to believe that they are looking at the results for "Lor," when they are in fact looking at the results for "Lo".The user is reviewing the results for 10 seconds, when the response for "Lor" comes back, and it is slightly different. Suddenly, the UI updates out of nowhere.
There are other versions of this problem, too. i.e.; a request that resolves in 3 secs being sent before one that resolves in 500ms. The consequences of this one are even worse, where the UI can permanently display the incorrect result (until a new request is fired off)
This can be avoided with another check around here.
If this idea were abstracted into a notion of a "request key," (it could just be a string made up of the four values that are checked there), then the key in the callback could be compared against the key in state. If the keys don't match, then the subsequent response could be considered as "dropped," which just means that the receiving of the response goes ignored; it doesn't trigger a state update.
This would require computing the key and placing it onto the state so that it can be referenced at a later time.
When the abort API ( #17 ) is supported, in-flight requests should probably also be aborted to save on bandwidth.