abc123s / redux-batch-enhancer

Batch subscriber notification for an array of actions (including complex actions, e.g. thunks).
25 stars 8 forks source link

nested promise based thunk are not supported #2

Open jonashartwig opened 8 years ago

jonashartwig commented 8 years ago
function receiveData(data) {
    return {
        type: APPLICATION_ACTIONS.RECEIVE_DATA,
        data
    };
}

export function getData() {
    return dispatch => {
        return fetch("url")
            .then(data => dispatch(receiveData(data)));
    };
}

export function login(credentials) {
    return (dispatch, getState) => {
        return UserService.login(credentials)
            .then(() => {
                const action = batchActions([
                    onLogin(),
                    getData().then(() => {
                            dispatch(navigate("/"));
                    })
                ]);

                dispatch(action);
            });
    };
}

the code above works without batching with plane thunk. Using batching it does not work. I do not get any error either. If I remove the then, it works. But the application flow is broken. Any ideas?

Regards

wsun commented 8 years ago

@jonashartwig just to clarify, when you say it doesn't work, do the actions and state updates happen? (they just don't update the components?)

jonashartwig commented 8 years ago

Ok, so WITHOUT then. everything seems to work as expected. If I have the then the code calls getData() and onLogin() but it does not even reach inside of getData() the statement return fetch.

I debugged the code in detail. If the then is present then it never goes into the batchMiddleware function at all.

I think I understand why: before it was like this: dispatch(getData()) .then(() => { dispatch(navigate("/")); })

and dispatch from thunk would return whatever is returned from within function (dispatch) { return ... }. But this does not happen now.

So I changed the code like this:

export function login(credentials) {
    return (dispatch, getState) => {
        return UserService.login(credentials)
            .then(() => {
                const action = batchActions([
                    onLogin(),
                    dispatch(getData()).then(() => {
                            dispatch(navigate("/"));
                    })
                ]);

                dispatch(action);
            });
    };
}

With that code all actions and dispatches are handled as i wish them to be. However 1. it feels very strange to do it like this. On the other hand the navigate() action which should change the react router state and navigate to a new page does not trigger a component update somehow. I see the route change in the browser however. In the baseline the router onEnter function is not called either, so getComponents is not called from the router API.

In the end I refactored the code to look like this. It is also way cleaner:

export function login(credentials) {
    return (dispatch, getState) => {
        return UserService.login(credentials)
            .then(() => getData())
            .then((data) => {
                const action = batchActions([
                    onLogin(),
                    doSomeDataStuff(data),
                    navigate("/")
                ]);

                dispatch(action);
            });
    };
}
jonashartwig commented 8 years ago

As it turns out this seems to be an issue only together with react router or react redux router.