-
Notifications
You must be signed in to change notification settings - Fork 195
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
RFC: configureCallRSAA
(Callable-RSAA utilities)
#216
Comments
thought a little more on a few of the above points:
|
Lately I've also been considering removing the This would simplify things quite a bit actually, and slim down the code... but wouldn't offer the "Promise" based utility if anyone thinks that would still be helpful (but we could later add a |
Background:
The first util I'm thinking about porting over is a
configureCallRSAA
util, that's used to createfetchRSAA
andfromRSAA
methods that will immediately invoke thefetch
for any valid RSAA (using apiMiddleware implementation internally) and either return a[Promise, FSA]
array tuple (fetchRSAA) or an Observable that emits FSAs (fromRSAA). See JSDoc below fold for more info:I originally had a use-case for this when using
redux-api-middleware
alongsideredux-observable
. Most often when usingredux-api-middleware
, I'll dispatch RSAAs normally to the redux store, allow apiMiddleware to process it, and respond to the REQUEST, SUCCESS, and FAILURE actions in reducers or epics.However, because of redux-observable issue #314, We can't really "dispatch" sequenced RSAAs from a single Epic anymore, only emit them in the result, which meant I usually needed multiple epics or observables to dispatch an API request, forward it to the store to be processed by apiMiddleware, and then respond to the result FSA in the epic... which felt like a bit too much indirection for something that I really just needed
fetch
andfrom
for if I avoided my preexisting RSAA action creators we've made for our API.I didn't really want to abandon our RSAAs in these situations if I could help it... they are a great way to organize RPC/REST/Legacy api methods in the client, and we still often use them normally by dispatching them directly to the redux store. Doing so would also mean that I would need to rework any middleware we use that operate on RSAAs or response FSAs (like our "authMiddleware" that applies auth token to headers by reading token from store).
After I tried out various iterations of
configureCallRSAA
to solve the problem, and it became more useful, I found this also allowed me to clean up some "noise" in the action log and reducer code for a few API-request-heavy features and avoid store updates until the entire sequence (or parts of the sequence) was complete. As I tried to generalize the solution, I also realized this would allow you to useredux-api-middleware
outside of a redux-store context entirely, if desired, and still offer the pre/post-fetch hooks w/ middleware for interceptor-like functionality (similar to angular, axios, etc).This util might be a good fit for
redux-api-middleware
lib, since it would offer users a stable, built-in utility that allows projects usingredux
&redux-api-middleware
to reuse the existing RSAA spec (and existing action creators or middlewares that leverage it) for situations where direct fetch may be desired.Proposal
Here is the current JSDoc I have for this util that describes this in more detail:
Caveats
There were a couple of design decisions made to simplify the implementation and focus it's utility on most common/likely use case:
configureCallRSAA
can acceptrsaaMiddleware
andfsaMiddleware
configurations, these middlewares are not treated identically as a redux middleware configured with the redux Store. Since we're operating outside of the store's middleware context, and only interested in the request/response behavior of the individualfetch
request handled byapiMiddleware
, I limit the # of possible middlewares to those that are synchronous, and only emit a single action to the next middleware (i.e.next
called only once). Trying to mimic a redux store's middleware chain point-for-point would be a bit more involved since any potential side-effect or async processing would be allowed, meaning this util would no longer be thefetch
wrapper I needed. And if you really needed the identical behavior of a redux store here, I'd recommend using the actual redux store and not using configureCallRSAA :). That said, I probably still need to improve documentation and error handling around this. May also be worth more discussion, or potentially expanding the # of allowed middlewares with future updates (e.g. allownext
to be called once async as well?).redux-api-middleware
) that it might be worth a disclaimer that this isn't intended be an app-wide replacement for redux-api-middleware apps that use something likeredux-observable
... using a redux store's configured apiMiddleware and dispatching the RSAAs individually is still my preferred way to do many things with a REST api and client cache :). So probably some documentation around intended use might be good, and a point about other techniques you could use before reaching forfetchRSAA
orfromRSAA
.configureCallRSAA
, I usually provide astore
value that only provides an interface to the redux store'sgetState
method, and leavedispatch
undefined (generally making any rsaaMiddleware and fsaMiddleware read-only by avoiding property mutations). However, I couldn't really decide if there might be situations where you want afetchRSAA
orfromRSAA
method to invoke adispatch
to the real redux store from a configured middleware... 🤔 . So this is allowed, but in practice I usually avoid it unless I'm refactoring an older middleware function that may have useddispatch
directly.redux-api-middleware
lib, which is not a huge concern for many apps built using unused code removal and tree shaking, but could be an issue for some.bailout
istrue
w/ the callRSAA methods 🤔. In practice, I don't ever use it with RSAAs invoked byfromRSAA
orfetchRSAA
but might be worth discussion?The text was updated successfully, but these errors were encountered: