redux-logic icon indicating copy to clipboard operation
redux-logic copied to clipboard

dispatch() behavior may be wrong while processOptions.successType is defined

Open michael-schaedel opened this issue 7 years ago • 3 comments

I love the feature of processOptions as it removes quite a bit of boilerplate in our repos. However, we found that when successType is defined, we cannot call dispatch as it is bound to the successType. Consider the following example:

createLogic({
  type: 'SOME/TYPE',
  latest: true,
  processOptions: {
    dispatchReturn: true,
    dispatchMultiple: true,
    successType:  'SOME/TYPE_SUCCESS',
    failType: 'SOME/TYPE_FAILURE',
  },
  async process({ api, action }, dispatch, done) {
    await api.post(...);
    await api.post(...);
    dispatch({ type: 'SOME/OTHER_TYPE' });
    done();
  }
});

Due to the following method: https://github.com/jeffbski/redux-logic/blob/v0.12.3/src/createLogicAction%24.js#L105, the dispatch() call wraps the passed action in the payload of the successType. Meaning, the dispatch() call in the process method in the above example results in:

dispatch({ type: `SOME/OTHER_TYPE' }) ->
  dispatch({ type: 'SOME/TYPE_SUCCESS', payload: { type: 'SOME/OTHER_TYPE' } })

In order for us to get around this, we omit successType in the processOptions and call it manually. Any way we can get a quick fix, or are we using redux-logic incorrectly here?

michael-schaedel avatar Jan 24 '18 05:01 michael-schaedel

Yes, currently successType limits you to a single type of action and it will decorate either what is called via dispatch or what is returned (when dispatchReturn is true, or dispatch and done are omitted from the signature).

If you want to have dispatch multiple things of different action types then leave it undefined and just wrap them each with the proper action creator.

There are a lot of use cases where they user only needs to dispatch one action either success or failure and thus the processOptions dispatchReturn and successType are mainly to help with that. It is pretty nice to just return a promise that returns data and having set the successType have it be decorated.

I think if one is making multiple dispatches then it is best to leave successType undefined and just wrap for each dispatch or returned value with the appropriate action creator. I tried to think of other ways this might work but I think they each have drawbacks or can cause confusion.

I'll figure out how I can improve the docs regarding this feature.

jeffbski avatar Jan 24 '18 13:01 jeffbski

I feel it's a bit opinionated to enforce dispatch(successType). I think it would be more intuitive if the process flow didn't enforce dispatch(successType), but only applied this behavior on the result of the promise and when processOption.dispatchReturn === true. To handle our design, we just wrapped createLogic in a wrapper that handled promises in this manner.

Appreciate the quick response.

michael-schaedel avatar Jan 24 '18 16:01 michael-schaedel

Running into the same problem here. In my case, it concerns logic where there's ultimately one success/failure result (represented by a Promise that either resolves or rejects), but there are one or more progress events being dispatched in the process.

I can't represent those with this limitation, unless I totally give up on dispatchReturn. After all, most real-world UI actions are going to require some sort of visual feedback to the user, so the category of "things that result in exactly one dispatch" is going to be negligibly small.

joepie91 avatar Feb 10 '19 22:02 joepie91