-
Notifications
You must be signed in to change notification settings - Fork 10
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Improve API #1
Comments
@gaearon I would love to get your thoughts on this if you have a minute 🙇 |
@idolize I think locking people into a specific http/XHR implementation isn't a good thing. Some people use |
@johanneslumpe Agreed! I doubt I'll go the route of creating my own "redux http library", but if I do rest assured it will be an entirely separate library from this one, and I'll do my best to avoid requiring a specific fetch/XHR library. |
@idolize Ok good to hear :) But I do agree with the need to get rid of the first |
Helper |
Right now the library is very un-opinionated, in that the developer can use whatever kind of request library they want (it's not even specific to HTTP) as long as they add the
meta.httpRequest
field and use uniquemeta.httpRequest.url
parameters.This is a nice feature, however, there is more boilerplate and required convention than I'd like.
Specifically, requiring the developer to surround the first Action dispatch with an
if (!dispatch(myAction)) { return; }
in order to bail out of the remaining sequence in the Action Creator is easy to forget.I'm looking for feedback on how we can improve this API.*
Right now, the best solution I've thought of is to export an additional (optional) helper function that takes the initial (
{ done: false }
) action as a parameter, as well as afunction
to run if it succeeds (is not cancelled). This would just be a very simple wrapper over theif (!dispatch(myAction)) { return; }
we have now, but I think it would be cleaner for the developer using the library.Thoughts? @acdlite
The text was updated successfully, but these errors were encountered: