Skip to content

Instantly share code, notes, and snippets.

@kof
Last active January 18, 2020 09:06
Show Gist options
  • Star 9 You must be signed in to star a gist
  • Fork 1 You must be signed in to fork a gist
  • Save kof/9ead8b0899e2e1306311 to your computer and use it in GitHub Desktop.
Save kof/9ead8b0899e2e1306311 to your computer and use it in GitHub Desktop.
Action creator vs. reducer

When should you use action creator and when reducer?

Action creator

  • you need to have side effects
  • you need to read from store to decide what to do
  • you need to dispatch more than one action
  • action produced by action creator needs to contain all the data reducer can need to shape the components state

Reducer

  • should not have any side effects
  • should not read global application state
  • should not manipulate data structures passed with the action, it should only pick the right data and merge it into state
@nikgraf
Copy link

nikgraf commented Jan 23, 2016

There was one case where our reducer also managed the Cookie. At login success the cookies with the auth toke was set. At logout it was removed. Would you move this side-effect also to the action creator?

@keit
Copy link

keit commented Jan 24, 2016

I sometimes use a middleware when I need to dispatch more than one action.

@BerkeleyTrue
Copy link

I avoid side-effects pre reducer all together. Action creators are map functions, reducers are reduce functions, then use middlewares for effects.

@kof
Copy link
Author

kof commented Jan 24, 2016

There was one case where our reducer also managed the Cookie. At login success the cookies with the auth toke was set. At logout it was removed. Would you move this side-effect also to the action creator?

Sounds like a clear case for action creator.

@kof
Copy link
Author

kof commented Jan 24, 2016

What bugs me is that I end up having basically reducers like that:

export default function reduce(state = initialState, action) {
  switch (action.type) {
    case types.SET_USER:
    case types.SET_CHANNEL:
    case types.HIDE_CHANNEL_INFO:
    case types.SHOW_CHANNEL_INFO:
    case types.ADD_USER_TO_CURRENT_CHANNEL:
    case types.USER_LEFT_CURRENT_CHANNEL:
      return {...state, ...action.payload}
    default:
      return state
  }
}

This basically means that I don't really need a reducer. It can be one generic reducer + list of action types per component.

This is possible because my action creators build payloads that fit the state ... is it wrong?

@kof
Copy link
Author

kof commented Jan 24, 2016

Also I came up with a conclusion that a reducer should not access anything but action, no other imports, no global objects .... are you with me?

@tget4
Copy link

tget4 commented Jan 24, 2016

@kof - I would avoid building the state tree in your action creators. Actions should be decoupled from the shape of your state. They should strictly tell your reducer what changed and pass the data that changed. It's up to your reducers to determine where that data exists in your state.

From redux docs:

It’s a good idea to pass as little data in each action as possible. For example, it’s better to pass index than the whole todo object.

@kof
Copy link
Author

kof commented Jan 24, 2016

"pass the data " means that I need to have/build the data in some form in creator. In most of my cases the data I passed from creator was just merged into state by reducer. In my code most of the time there is no need for custom cherry picking logic in reducers.

@kof
Copy link
Author

kof commented Jan 25, 2016

Also who defines how much should data passed from creator be normalized to fit the state needs?

@gaearon
Copy link

gaearon commented Feb 5, 2016

Yes, action creators knowing the state shape is generally an anti-pattern. It's fine if you normalize data from the server to some common shape and merge that into the state — but then you don't even need to react to different actions. Check out real-world example and its entities cache to get an idea of how to do that.

It all depends on where the source of truth is. If data from server is source of truth, just admit it by having an entities reducer that merges any new info right from the action. If you have complex local (or optimistic) mutations, this is the point where actions would be thin, and most of the logic would be inside the reducers.

@alfonsodev
Copy link

I stumbled upon this post because I found myself having all reducers responding with,

      return {...state, ...action.payload}

it felt fishy, I'd put it different words to see if I understood well why this is an anti-pattern
and to explain how I ( and I think many other users too) ended falling in the anti-pattern.

After reading this posts I understood that because
one action can affect one or many reducers its payload should be "reducer agnostic",
the reducer should be picking the properties it needs from the payload so that the reducer keeps
the ownership of the store shape.
This is much more maintainable because if you ever have to change the shape of the store,
there is only one place to change it, you go to the particular reducer and is just one change.
But If you do ...action.payload in the reducer, you'll have to check all the actions that you are reacting to,
change those, and then also check if there are other reducers reacting and adjust those too,
definitely much harder to maintain.

So why I ended doing return {...state, ...action.payload} ?
It started with a normal reducer like this ...

case UPDATE_USER_PROFILE:
  return {
    ...state,
    name: action.payload.name,
    address: action.payload.address,
    email: action.payload.email,
  }
break;

Because all keys match and action.payload looks redudant, and because we have es6 then I go ...

case UPDATE_USER_PROFILE:
  const { name, email, address } = action.payload
  return { ...state, name, email, address }
break;  

Which still kind of OK because it respects the patter, I'm cherry picking the properties from the payload,
but then you (or a colleague) come back, see that code, and realize the action just returns those 3 props,
and because we all love ...spreadOperator then I end up with

case UPDATE_USER_PROFILE:
  return { ...state, ...action.payload }
break;  

And now I learnt this ☝️ is an anti-pattern

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment