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

question: side-effects and dispatch

Open tony-kerz opened this issue 7 years ago • 9 comments

in this section of the doc on side-effects, it shows something like:

import { sendAnalytics } from '../analytics';
import { doFoo } from '../api/foo';

export function userDoesFoo() {
  return {
    type: DO_FOO,
    promise: doFoo(),
    meta: {
      onSuccess: (result, getState) => {
        const userId = getState().currentUser.id;
        const fooId = result.id;
        sendAnalytics('USER_DID_FOO', {
          userId,
          fooId,
        });
      }
    }
  }
}

in the above sample, sendAnalytics looks a lot like an action creator (with type and payload as args), but submitting actions to redux requires a reference to dispatch somewhere.

is the implication that sendAnalytics submits a redux action, and if so, what is the implied strategy for obtaining a reference to dispatch?

or maybe, i'm asking the wrong question, but my general question is:

what would be the idiomatic way to manage a side-effect with redux-pack that wants to submit another redux action?

or is that an anti-pattern that redux-pack is specifically attempting to avoid?

apologies for being dense, i admittedly struggle with extracting "best practices" for side-effects from related discussions...

my immediate use-case is: on the failure of a data-fetch action i want to submit a subsequent ui-centric action concerned with managing errors (e.g. notify the user visually of an error)

tony-kerz avatar Sep 14 '17 15:09 tony-kerz

i subsequently stumbled across this comment on topic which shows a way to use redux-thunk to get a reference to dispatch, and i cobbled together a pattern like the following:

      function doSomething() {
        return dispatch => {
          dispatch({
            type: SOMETHING,
            promise: somethingPromise(),
            meta: {
              onSuccess: result => {
                dispatch({
                  type: SOMETHING_SUCCEEDED, 
                  promise: somethingSucceededPromise()
                })
              },
              onFailure: result => {
                dispatch({
                  type: SOMETHING_FAILED, 
                  promise: somethingFailedPromise()
                })
              }
            }
          })
        }
      }

wondering where this might fall on the spectrum of anti-pattern <-> idiomatic...

tony-kerz avatar Sep 14 '17 16:09 tony-kerz

I've got the same general line of thought in mind. Just like the last time I commented on this project, having dispatch available in meta seems like it would give tremendous flexibility to replace redux-thunk with something more organized.

icopp avatar Oct 03 '17 23:10 icopp

I too am curious whether this is an anti-pattern? We have a need to do something like this, and if it is indeed NOT some kind of massive anti-pattern, I agree with @icopp that it should be integrated into the meta callbacks.

duro avatar Oct 06 '17 20:10 duro

@icopp and @tony-kerz:

Based on the following in this projects README, I have a feeling the author considers this an anti-pattern:

Async actions in redux are often done using redux-thunk or other middlewares. The problem with this approach is that it makes it too easy to use dispatch sequentially, and dispatch multiple "actions" as the result of the same interaction/event, where they probably should have just been a single action dispatch.

This can be problematic because we are treating several dispatches as all part of a single transaction, but in reality, each dispatch causes a separate rerender of the entire component tree, where we not only pay a huge performance penalty, but also risk the redux store being in an inconsistent state.

redux-pack helps prevent us from making these mistakes, as it doesn't give us the power of a dispatch function, but allows us to do all of the things we were doing before.

Given this encouragement, I was able to take a step back and look at our case where we thought a dispatch side-effect was needed, and see how it could be combined into a single action.

duro avatar Oct 06 '17 20:10 duro

In my case, I want these dispatches to render separately, because I have multiple asynchronous actions that can trigger each other but that all have different UI side effects.

For example: pushing a button triggers action A with promise A, which alters things in the store initially, on a successful resolution, and on a failure (optimistic update style, re-rendering things accordingly for each case), but once it's successfully resolved (but not if it fails) I have to also trigger action B with promise B, which on a successful resolution alters other things in the store based on side effects of A on the server (and also re-renders things accordingly).

icopp avatar Oct 06 '17 21:10 icopp

not sure if this is what @icopp is alluding to, but in my case, i want to dispatch something conditionally based on the success or failure of the initial action/promise, so i don't think combining actions works for that use-case...?

tony-kerz avatar Oct 07 '17 15:10 tony-kerz

Have you thought about using epics via redux-observable or saga via redux-saga for these side effects ? I've found using epics is way more powerful than using any kind of sequential / meta actions.

stephanebachelier avatar Oct 16 '17 16:10 stephanebachelier

My take on the redux-pack way of doing something is this: your actions should not be split up into various actions based upon the possible lifecycles of an action. So the developer has the space, I think, to decide what an entire action looks like, what looks sensible as the entire lifecycle of an action and go from there. If one action triggers another, I think it makes sense to trigger that action in the meta part of a redux-pact action. An easy way do that is with redux-thunk. Redux-thunk gives you access to the store and dispatch an action from anywhere in your app. So, for instance, you can in your actions.js:

import store from '~/public/store.js';

and then in an action do this:

export const REGISTER = 'REGISTER';
export function register(payload){
    return {
        type: REGISTER,
        promise: system.API.POST('/users',{"payload":payload}),
        meta: {
            onSuccess: (result, getState) => {
                if (result.status===201){
                    store.dispatch(login(payload));
                }
            },
            onFailure: (err, getState) => {
                console.warn(err);
                store.dispatch(clearCredentials());
            }
        }
    }
}

Although the author might consider this an anti-pattern, it feels right to me for its context in my app, and works slick.

EnshaednHiker avatar Mar 03 '18 18:03 EnshaednHiker

Why not move it to middleware? like that

export const analyticsMiddleware = store => next => action => {  
    switch (action.type) {
        case DO_FOO: 
            const { meta } = action;
            const lifecycle = meta ? meta[KEY.LIFECYCLE] : null;

            switch (lifecycle) {
                case LIFECYCLE.SUCCESS: store.dispatch(SOMETHING_SUCCEEDED); break;
                case LIFECYCLE.FAILURE: store.dispatch(SOMETHING_FAILED); break;
            }
        default: ;
    }
    return next(action);
}

:P

YaaMe avatar Mar 31 '18 08:03 YaaMe