jeffbski / redux-logic

Redux middleware for organizing all your business logic. Intercept actions and perform async processing.
MIT License
1.81k stars 107 forks source link

wait for dispatch to complete #88

Open naorye opened 6 years ago

naorye commented 6 years ago

I have the following logic:

export const updateSomething = createLogic({
    type: actions.updateSomething,

    async process({ getState, action }, dispatch, done) {

        const { data } = await axios.post('url');

        done(data);
    }
});

export const doSomething = createLogic({
    type: actions.doSomething,

    async process({ getState, action }, dispatch, done) {
        const data = await dispatch(actions.updateSomething(data));

        await dispatch(actions.doAnotherThingUponCompleteOnlyInThisFlow());
        done();
    }
});

Here I am trying to achieve two things:

  1. Get a result of an action (inside logic). This can be solved by reading the state AFTER the action is done, but I think done(data) should somehow return that data.
  2. Wait for dispatch to complete and then dispatch another action. The code I've written doesn't wait for 'updateSomething' to complete. Therefore 'doAnotherThingUponCompleteOnlyInThisFlow' happens before 'updateSomething' completes which is incorrect flow in my app.

What am I doing wrong? Is there any way to fix it?

Thanks, NaorYe

jeffbski commented 6 years ago

@naorye Thanks for writing, sorry for the slow response. Things were a little busy recently. Anyway, there are a variety of ways to solve this type of thing.

If you do need to know when something is done before continuing with other things, then I suggest that you have your API calls that manipulate data return a promise or observable so you can know when they have completed.

Then from the result of the API in your logic then you can dispatch (and that will be able to update synchronously). So when you will know when you are done. So your API calls could be async and then you have a bunch of synchronous actions and just one complex flow which brings it altogether for that group of things. Keeping API's free of dispatching is nice for testing and also for using the API's in other interesting ways.

export const doSomething = createLogic({
    type: actions.doSomething,

    async process({ getState, action, api }, dispatch, done) {
        const {data} = await api.getWidgets();
        dispatch(widgetsSuccess(data));
        const {otherData} = await api.getSomethingElse(data.foo);
        dispatch(anotherSuccess(otherData);
        // ...
        // at this point all async stuff is done
        done(); 
    }
});

If for some reason you can't take this approach then some people simply fire events and then check that status in the other events to know when everything is done. For instance they might dispatch some parallel actions and then simply listen for an array of actions and check the state to know when you are done with all.

export const doSomething = createLogic({
    type: actions.doSomething,

    async process({ getState, action, api }, dispatch, done) {
        // load widgets and categories in parallel, having them each dispatch when done
       dispatch({ type: LOAD_WIDGETS });
       dispatch({ type: LOAD_CATEGORIES });
       // other logic will wait for them to be completed
       done(); 
    }
});

export const loadWidgets = createLogic({
    type: LOAD_WIDGETS,

    async process({ getState, action, api }, dispatch, done) {
        const {data} = await api.getWidgets();
        dispatch({ type: WIDGETS_LOAD_SUCCESS, payload: data });
       done(); 
    }
});

export const loadCategories = createLogic({
    type: LOAD_CATEGORIES

    async process({ getState, action, api }, dispatch, done) {
        const {data} = await api.getCategories();
        dispatch({ type: CATEGORIES_LOAD_SUCCESS, payload: data });
       done(); 
    }
});

export const loadedCategoriesAndWidgets = createLogic({
  // in this case either one might be last so listen for both
  type: [WIDGETS_LOAD_SUCCESS, CATEGORIES_LOAD_SUCCESS],

  process({action, getState}, dispatch, done) {
    // we can check the state that we have both widgets and categories
    const state = getState();
    if (state.widgets && state.categories) { // we have both
      dispatch({type: WIDGETS_CATEGORIES_LOADED});
    }
   done();
  }
};

So it really depends on the individual use case which works better but I like to keep my API's clean of dispatching and then tie things together in a few logics.

I am thinking about having redux-logic return a promise or observable from it's dispatch, but in some multi-dispatching use cases it is hard for users to get right, so I am still evaluating the idea.

Hopefully my above solutions will help, but if not just let me know if you have additional questions.

cyrilchapon commented 6 years ago

I'm trying to use redux-logic along with redux-thunk (this is a transition, not to recode my entire application)

Inside a logic, I'm trying to use the dispatch returned promise of a redux-thunk action, to take advantage of redux-thunk's "Any return value from the inner function will be available as the return value of dispatch itself.", but this doesn't work as dispatch is overriden with a custom implementation inside process hook

import { asyncThunk } from './myAsyncThunk'

export const myLogic = createLogic({
  process({action, getState}, dispatch, done) {
    dispatch(myAsyncThunk('foo'))
    .then(() => dispatch(anyThingElseAsync('bar')))
    .then(() => done())
  }
};

(Is not working. dispatch here doesn't have any .then method here)

Appart from migrating my whole logic (from thunks) inside redux-logic, is there any known strategy ?

I though of passing dispatch through deps (I actualy already do this in validate phase); but I'm afraid of breaking something if I call it from the process hook (I thought that, if dispatch is reimplemented or kinda proxyed, there must be a good reason for that)

jeffbski commented 6 years ago

@cyrilchapon I think the easiest thing to do would be to use the storeDispatch for this purpose (the one you are passing in deps) at least until you can get rid of thunks completely.

The only difference in using store.dispatch versus process's dispatch is that process's dispatch can prevent dispatching if the logic was cancelled. So if you are not worried about that occurring you should be good to go with using the store.dispatch you passed through in deps.

Alternatively you could simply invoke the thunk inline since you have what you need

export const myLogic = createLogic({
  process({action, getState}, dispatch, done) {
    myAsyncThunk('foo')(dispatch, getState)
    .then(result => dispatch(result))
    .then(() => dispatch(anyThingElseAsync('bar')))
    .then(() => done())
  }
};

however if the thunk calls other thunks then you are back to the same problem needing the real store.dispatch.

So assuming you have that situation it is problem safest to just use the store.dispatch you are passing into deps.

I'll ponder it some more, but that seems like a reasonable approach.

cyrilchapon commented 6 years ago

@jeffbski, thanks about that 😄

I went like this, and it worked indeed. But thanks for clarifying the cancellation stuff, that's the kind of specificity I was talking about, I couldn't guess myself.

naorye commented 6 years ago

@jeffbski Hi, I had another case where I needed to wait for an action to complete. I have the following logic:

createLogic({
    type: actions.addCardAndBuyItems,

    async process({ action }, dispatch, done) {
        const { userId, accountId, billingToken } = action.payload;

        try {
            dispatch(addBillingCard(userId, accountId, billingToken));
            dispatch(buyItems(accountId, true));
        } catch (error) {
             // ...handle errors
        }

        done();
    },
});

I need to wait for addBillingCard() to complete before calling buyItems(). I wrote a middleware that should help me solve this issue and I'd like to know what do you think:

const typeResolvers = {};
let currentStore;

export const dispatchProcessMiddleware = (store) => {
    currentStore = store;
    return next => (action) => {
        const resolvers = typeResolvers[action.type];
        if (resolvers && resolvers.length > 0) {
            resolvers.forEach(resolve => resolve());
        }
        next(action);
    };
};

export function dispatchProcess(requestAction, successActionType, failureActionType = undefined) {
    if (!currentStore) {
        throw new Error('dispatchProcess middleware must be registered');
    }

    if (!successActionType) {
        throw new Error('At least one action to resolve process is required');
    }

    const promise = new Promise((resolve, reject) => {
        typeResolvers[successActionType] = typeResolvers[successActionType] || [];
        typeResolvers[successActionType].push(resolve);
        if (failureActionType) {
            typeResolvers[failureActionType] = typeResolvers[failureActionType] || [];
            typeResolvers[failureActionType].push(reject);
        }
    });

    currentStore.dispatch(requestAction);

    return promise;
}

and after applying the middleware, this is how the logic will be:

createLogic({
    type: actions.addNewClusterBillingCard,

    async process({ action }, dispatch, done) {
        const { userId, accountId, billingToken } = action.payload;

        try {
            await dispatchProcess(
                addBillingCard(userId, accountId, billingToken), // Action object
                addBillingCardSuccess.type, // Action type
                addBillingCardFailure.type, // Action type
            );
            dispatch(buyItems(accountId, true));
        } catch (error) {
            console.log(error);
        }

        done();
    },
});

My middleware is waiting for success type and failure type and resolves when they arrive. I know I can write it better (use meta instead of dedicated dispatchProcess() function) but for now it works fine.

What do you think?

NaorYe

jeffbski commented 6 years ago

@naorye That's pretty ingenious. I was pondering some ways we might build that in to redux-logic, but haven't settled on a definitive approach yet.

Seems like your solution will meet the need. Nice job.

Johncy1997 commented 6 years ago

actions.js:


 export function  login  (credentials) {
console.log("actions/login called");debugger
   return async dispatch=>{
   await fetch("url",{
        method:'POST',
        body:JSON.stringify(credentials),
        headers:{
          "Content-Type":"application/json"
        }
      }).then( res => { 
        console.log("res.json:"+res.json());debugger  
        res.json()})

      .then(async(response) =>  {
          console.log('Success:', response);debugger
           if(!response.hasOwnProperty("error")){
            await dispatch({
                type:LOGIN,
                response
            })
          }
          else{
            await dispatch(requestFail(response.error))
          }

        }
    )
      .catch((error) => {
        //   console.error('Error:', error);
          dispatch(requestFail(error))
        });
    }
   }

reducers.js:


   case REQUEST_FAIL:
        console.log("reducers/Authentication/REQUEST_FAIL called");
        return{
            ...state,
            error: action.error,
        }
   case LOGIN:
        console.log("reducers/Authentication/LOGIN called"+state);debugger
        return{
            ...state,
            token: action.response.success.token,
            name: action.response.success.name,
            signedIn: true,
            error:null
        }

Login.js:


        const mapDispatchToProps = (dispatch) => {
      return{
      onSubmit : (credentials) => {
       dispatch(login(credentials));
           }
       }
           }
    submit(){
      let credentials={};
      credentials.email=this.state.email,
       credentials.password=this.state.password,
          this.props.onSubmit(credentials);
    }
     <Button  block onPress={ ()=>{
                this.submit();debugger
               if(this.props.authentication.error == null){
                  this.props.navigation.navigate('Home')
                }
                else{
                  Toast.show('Incorrect username or password');
                }
              }} 
                }}>

The flow of the login i want:

  1. this.submit();(under userdefined fn)
   2.onSubmit(credentials);(under mapDispatchToProps )
   3.dispatch(login(credentials));(within actions.js)
   4.dispatch({
                type:LOGIN,
                response
            }) or dispatch(requestFail(response.error))
  5. Move to "Home" if error is null within the store.

The flow i currently getting:((

   1. this.submit();(under userdefined fn)
    2.onSubmit(credentials);(under mapDispatchToProps )
    3.dispatch(login(credentials));(within actions.js)
     4. Moved to "Home".(because initial state contains error as null)
     5. dispatch({
                type:LOGIN,
                response
            }) or dispatch(requestFail(response.error)) 

Last two steps (4 & 5) swapped...I still dont figure out my problem as i am new to RN..Please help me

Thanks in advance.

jeffbski commented 6 years ago

@Johncy1997 I'll take a look and see if I can figure out what might be going on.

naorye commented 5 years ago

It has been pass more than a year :) any news regarding waiting for an action to complete? I have another use case, I'd like to fetch data (using a different action) and only then make it visible. Since I cant wait for dispatch(fetch(params)) to complete, I don't have a simple way to solve this case.

jeffbski commented 5 years ago

@naorye with v2 it is pretty easy to do things like this by watching for the success actions in the action$ after you do the fetching. You can fire off the fetch actions and then watch the action$ (stream) for the success actions you before continuing.

This would be similar to how my drag and drop example ( https://codesandbox.io/s/n34x5j3jv4 ) works, but you could easily make it wait for multiple using an operator like zip. I'll work up an example.

Blargh1 commented 5 years ago

@jeffbski BTW, mouse up offscreen doesn't work in the example. Just needs a mouse up event listener and a value check: https://codesandbox.io/s/lr7176yyw7

jeffbski commented 5 years ago

@naorye Here is an example of doing something like you are suggesting, this example uses separate actions for dispatching and then has a central one that kicks things off and waits for both to complete before firing another action. using action$ (the stream of all actions) we can create two observables, one waiting for people fetch to complete and one waiting for films fetch to complete.

The zip waits for both observables and then emits. Since we only want to wait for one set then take(1) causes the observable to complete after receiving one pair.

https://codesandbox.io/s/8z4npw9xw9

const beginFetchesLogic = createLogic({
  type: BEGIN_FETCHES,
  debounce: 500, // ms
  latest: true, // take latest only

  // use rxjs ajax injected as httpClient from configureStore logic deps
  process({ action$ }, dispatch, done) { 
    const peopleCompleted$ = action$.pipe(
      filter(x => x.type === FETCH_PEOPLE_FULFILLED),
    );
    const filmsCompleted$ = action$.pipe(
      filter(x => x.type === FETCH_FILMS_FULFILLED),
    );

    dispatch({type: FETCH_FILMS});
    dispatch({type: FETCH_PEOPLE});

    zip(peopleCompleted$, filmsCompleted$)
      .pipe(
        take(1)
      )
      .subscribe({
        complete: () => {
          dispatch({type: FETCHES_COMPLETE});
          done();
        }
      })
  }
});
jeffbski commented 5 years ago

@Blargh1 I am not seeing an issue with offscreen mouse up in the https://codesandbox.io/s/lr7176yyw7 example. Maybe I am not doing something you were doing. It's probably not a big deal for this demo, but if you have code that would fix the issue, feel free to fork and show me what I am missing. Thanks!

naorye commented 5 years ago

Cool! Thanks! Is there any place where I can read the full documentations regarding this feature?

Blargh1 commented 5 years ago

@jeffbski That is the fork, it has a few // NOTE: ... comments with the adjustments

jeffbski commented 5 years ago

@naorye well I guess I'm behind on documenting this. I thought I had written more up on it. I have a short mention of it in the full api docs, but there should be more. As of right now, I guess a couple of the live examples (from the home page of the repo).

  1. Drag and Drop - see the latest fork of that by @Blargh1 https://codesandbox.io/s/lr7176yyw7
  2. Websockets example - https://codesandbox.io/s/m38l7n745y
jeffbski commented 5 years ago

@Blargh1 thanks. I didn't realize that. I updated my version with your changes. I appreciate you taking the time to figure it out and share it.