rjz / typescript-react-redux

A TypeScript-enabled react/redux application
https://rjzaworski.com/c/typescript
Other
124 stars 24 forks source link

request in success? Why? #29

Open meandmax opened 6 years ago

meandmax commented 6 years ago

Im curious why you dispatch the request as a parameter in the success action group like here: https://github.com/rjz/typescript-react-redux/blob/master/src/actions/index.ts#L66

You do not use this property at all in the reducer, so is there another reason why you dispatch the request?

Nice work btw. Thanks Max

rjz commented 6 years ago

Thanks, @meandmax, and great question! I think this is a case of broad pattern, narrow example, and poor ("no") explanation from the author.

The pattern: even though request isn't used here, I often find it useful when applying a response to application state to reference variables from the original request that may not be echoed in the response (think of keying search results by query or page number, for instance). There are lots of ways to accomplish that depending on the structure of the application, of course, but the { request, response?, error? } pattern seems to hold up well across a variety of asynchronous actions.

Definitely worth notes both here (#22) and in the supporting article, though. I'll add.

meandmax commented 6 years ago

Ah ok, got it ... so you would say its generally a good pattern to dispatch always both objects when doing an async api action call?

Another question that I have in mind is what happens if you have several actions which needed to be called following a successful api call? Lets say you need to reset or initialise redux-form ... At the moment I handle this throught Thunks and dispatch these actions in the Promise callback. I want to rewrite this in the generic style that you showed in the example but your strict pattern does not allow dispatching actions besides the success action as I saw here:

https://github.com/rjz/typescript-react-redux/blob/master/src/actions/index.ts#L66

Thought about redux-loop to isolate side-effects, but I have the feeling with you approach I don`t need an extra library, I just need to change my way of writing thunks.

Thanks for your help.

meandmax commented 6 years ago

For now I added a third parameter to apiActionGroupFactory which would be called when the api call was successful like this:

function apiActionGroupFactory<_Q, _S>(
  x: ApiActionGroup<_Q, _S>,
  go: ApiFunc<_Q, _S>,
  cb?: (dispatch: Dispatch) => void
) {
  return (request: _Q) => (dispatch: Dispatch) => {
    dispatch(x.request(request));
    go(request)
      .then(response => {
        dispatch(x.success(request, response));
        if (cb) {
          cb(dispatch);
        }
      })
      .catch((e: Error) => dispatch(x.error(e, request)));
  };
}

Whats your thoughts on that?

rjz commented 6 years ago

The best solution is the one that works! The original ApiActionGroup was intended to model a single operation (here an API request, but really anything that could be represented as a Promise<T>) mostly to avoid making assumptions about how a chain of actions could be structured/canceled/etc. But cb(d: Dispatch): void seems like a tidy way to model continuation-passing: if it's solving a common issue in the project, cheers!