Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Should actions like showing/hiding loading screens be handled by reducers to related actions or generated by action creators themselves?

Tags:

redux

Given that you have some global view (showing a loading screen, for example) that you may want to happen in many cases, is it more appropriate to create an action creator/action pair for that behavior or to have the reducer for the related actions handle the transition?

This is hard to describe succinctly, to illustrate what I mean, here's a couple of examples. Which is better, and why?

A

function showLoading () {
    return 'SHOW_LOADING';
}

function hideLoading () {
    return 'HIDE_LOADING';
}

function fetchPostsRequest () {
    return 'FETCH_POSTS_REQUEST';
}

function fetchPostsSuccess () {
    return 'FETCH_POSTS_SUCCESS';
}

function doSomethingAsync () {
    return dispatch => {
        dispatch(showLoading());
        dispatch(fetchPostsRequest());
        // other logic
        dispatch(hideLoading())
        dispatch(fetchPostsSuccess());
    }
}

function rootReducer (state = {}, action) {
    const payload = action.payload;

    switch(action) {
        case 'SHOW_LOADING':
            Object.assign({}, state, {isLoading: true})
            break;
        case 'HIDE_LOADING':
            Object.assign({}, state, {isLoading: false})
            break;
        // other reducers for handling success/request stuff
    }
}

B

function fetchPostsRequest () {
    return 'FETCH_POSTS_REQUEST';
}

function fetchPostsSuccess () {
    return 'FETCH_POSTS_SUCCESS';
}

function fetchPostsFailure () {
    return 'FETCH_POSTS_FAILURE';
}

function doSomethingAsync () {
    return dispatch => {
        dispatch(fetchPostsRequest());
        // good
        dispatch(fetchPostsSuccess());
        // bad
        dispatch(fetchPostsFailure());
    }
}

function rootReducer (state = {}, action) {
    const payload = action.payload;

    switch(action) {
        case 'FETCH_POSTS_REQUEST':
            Object.assign({}, state, {isLoading: true})
            break;
        case 'FETCH_POSTS_SUCCESS':
            Object.assign({}, state, {isLoading: false /* other logic */})
            break;
        case 'FETCH_POSTS_FAILURE':
            Object.assign({}, state, {isLoading: false /* other logic */})
            break;
    }
}

I prefer A because it seems more sensible to me to describe these behaviors in one place rather than duplicate the logic of state management, but I have heard a maxim in the redux community that actions should describe what happened or is happening, rather than be imperative commands. In which case, is this just a semantic issue where a term like "ASYNC_OPERATION_START" is better than "SHOW_LOADING"?

like image 240
Eric Avatar asked Oct 19 '15 17:10

Eric


1 Answers

Think how this particular piece of code will evolve.
Use this to make decisions.

For example, you are likely to have more than one set of items that can be loading. You may eventually have two lists of items side by side, or one below the other. Thus you will want them to have separate isLoading state just like they have separate lists of IDs.

How would the code change in both options? It seems that having less actions is simpler because this lets you keep isLoading state of the particular list close to other information about it, and also not worry that you forgot to reset its state in the action creator. So in this case I'd choose option B.

On the other hand, if we're talking about a use case like showing a UI notification, I'd probably fire that as a separate action. It exists fairly independently of the server response that caused it: the notification needs to be hidden after a while, two notifications may live on the screen at the same time, etc. So, with this use case, option A seems a better fit.

In general, you should ask yourself:

  • How will this piece of code likely evolve?
  • Are these two actions really the same one or are they just related but independent?
like image 51
Dan Abramov Avatar answered Sep 28 '22 10:09

Dan Abramov