Is passing top-level state object to branch reducer an anti-pattern?

[pitaj]

I have a redux state where multiple branches' state demand on state of other branches. This means that I must last three entire state tree to each reducer, while the reducer only returns a single branch.

Is this a bad thing to do? I don't see how it could lead to problems as long as a branch is not modifying data of other branches (which they aren't).

[markerikson]

It's not necessarily wrong. Reducers are just functions, and how you structure them is up to you as long as they all ultimately consistently implement (state, action) -> newState and handle data immutably. Separating responsibilities by top-level key, as combineReducers does, is certainly a common pattern, but you are not required to restrict yourself to that.

[gaearon]

I think @markerikson is right. If you feel that this makes your codebase simpler rather than more complicated, go for it. I would only add that “state demanding on state of other branches” is often an antipattern and can be more easily implemented as derived data over some minimal state with Reselect.

Alternatively you can look at rereduce although I’m not aware if anybody uses it in production. See also discussion in #1315.

[naw]

Suppose you had two slices of state that depend on each other, and you have a "branch reducer" for each slice.

You could easily refactor those "branch reducers" into a single reducer by combining the code into a single function that operates on multiple subkeys. Likewise, if you had complicated single reducer that was operating on multiple subkeys, you could easily break it out into helper functions for each subkey, each of which takes the whole branch's state, but only calculates a single portion of the new state.

As an example:

  petsReducer = function(pets, action) {
    return { cats: catHelper(pets, action), dogs: dogHelper(pets, action) };
  }

Therefore, I think this question really has more to do with how you design your state-tree than how you actually implement your reducers and helper functions.

For a given action, if you find it impossible to update separate pieces of your state-tree independently, then those pieces of your state-tree aren't actually separate --- you should combine them. If you combine them, and you end up breaking other heuristics (like flatness vs nesting), then something is probably wrong with your overall state-tree design.

I believe there are cases where a helper function with a signature of (slice, action) => subslice is great. However, in your case, it sounds like you might be using it to mask pain that would be better alleviated by changing your state-tree design.

There may be legitimate exceptions, but generally I'd say this is an anti-pattern.

Of course the specifics of each case matter, so it's difficult to pass judgement on your particular application.

Finally, if a "branch reducer" reads all state and returns only a slice of state, it's not actually a reducer because it has the wrong method signature. You can construct a reducer with helper functions (like I did above with petsReducer), but those helper functions are not reducers themselves unless they expose a (slice, action) => (slice) signature like the sub-reducers used with combineReducers do.

[gaearon]

@naw This is a great comment, thanks for your perspective.

[markerikson]

@gaearon : suppose this could also go over in some of the docs discussion we're having, but I'll put it here for now. Was reviewing some older bookmarks and saw http://www.code-experience.com/problems-with-flux/ again. That author makes the argument that reducers should be more oriented towards individual write operations rather than dealing with a specific sub-branch. Any thoughts?

[gaearon]

Personally I disagree with the approach in that article and its conclusion. I know some people like this approach better, and they might be right—I just didn’t have a use case that truly makes this technique compelling. On the opposite, I find it much more compelling to immediately know which function changed the state just based on the reducer structure. In my opinion it helps debugging a lot.

[gaearon]

As a developer, I wish to have an overview of all the state updates an action triggers. In one place in my code. Line by line.

I don’t understand this part. Why is this important? In my view, what gets updated is up to the reducers which might be written by different people. You only need to care that the component you wrote dispatch the actions you intended, and that the reducers you wrote respond to them correctly.

If it is important for debugging a problem, tools that show differences between states can help. For example, something like redux-diff-logger can help.

Having a comprehensive summary somewhere in the code maximizes the possibility of getting the state update correct, especially if the order of the write operations is relevant!

The order of write operations cannot be relevant in Redux because there is no order (unless you go out of your way to make them go in a specific order, at which point you have full control over it).

Reducers are supposed to compute independent state. You shouldn’t care about the order here. If you care about the order it probably means that the state you are computing shouldn’t have been in the store at all, and should instead be computed as derived data.

[naw]

There are two way of looking at your application:

1. Given an action, what slices of state does it affect, and how?
2. Given a slice of state, what actions affect it, and how?

Both are useful, and which is more useful is really subjective. You have to try both and decide for yourself what works well for you and your team, or blindly choose to take someone's opinion.

The redux code itself works with either approach, although the redux docs (and flux in general) heavily encourage approach (2).

Let's consider a concrete example:

Suppose you have three domain objects: friendRequests, chatMessages, and notifications, and suppose your state-tree looks like this:

{
  friendRequests: [],
  chatMessages: [],
  notifications: []
}

When you receive a new friendRequest, you want to append it to the friendRequests array, and also create a notification. Similarly, when you receive a new chatMessage, you want to append it to the chatMessages array, and also create a notification.

You can model this with either the action-centric approach suggested in the article, or with a slice-centric approach as recommended in the redux docs.

The first action-specific way looks like this:

const mainReducer = function(state, action) {
  switch(action.type) {
    case NEW_FRIEND_REQUEST:
      return {
        ...state,
        friendRequests: state.friendRequests.concat(action.friendRequest),
        notifications: state.notifications.concat({ notification_type: 'friendRequest', notification_id: action.friendRequest.id })
      };
    case NEW_CHAT_MESSAGE:
      return {
        ...state,
        chatMessages: state.chatMessages.concat(action.chatMessage),
        notifications: state.notifications.concat({ notification_type: 'chatMessage', notification_id: action.chatMessage.id })
      };
  }
}

You can at a glance see that the NEW FRIEND_REQUEST action does 2 things to the state --- adds a friend request and adds a notification. What you can't see at a glance, is which actions affect notifications --- you have to look through every action to determine that.

The second slice-centric way looks like this:

const mainReducer = function(state, action) {
  const friendRequests =  function(action_type) {
    switch(action_type) {
      case NEW_FRIEND_REQUEST:
        return state.friendRequests.concat(action.friendRequest);
      default:
        return friendRequests;
    }
  }(action.type)

  const chatMessages = function(action_type) {
    switch(action_type) {
    case NEW_CHAT_MESSAGE:
      return state.chatMessages.concat(action.chatMessage);
    default:
      return state.chatMessages;
    }
  }

  const notifications = function(action_type) {
    switch(action_type) {
      case NEW_CHAT_MESSAGE:
        return state.notifications.concat({ notification_type: 'chatMessage', notification_id: action.chatMessage.id });
      case NEW_FRIEND_REQUEST:
        return state.notifications.concat({ notification_type: 'friendRequest', notification_id: action.friendRequst.id});
      default:
        return state.notifications;
      break;
    }
  }
  return { friendRequests: friendRequests, chatMessages: chatMessages, notifications: notifications };
}

You can see at a glance that both NEW_CHAT_MESSAGE and NEW_FRIEND_REQUEST affect notifications, but you can't see at a glance that NEW_FRIEND_REQUEST affects more than one slice of state.

These are completely isomorphic. You can easily (albeit painstakingly) switch back and forth between the two implementations. In fact, you could probably build a tool that could convert one to the other, or even visualize one as if it were the other.

I do find it interesting that redux code itself is fully compatible with both approaches, yet heavily encourages the 2nd. In other words, there is nothing about redux in and of itself that would push you one direction or the other.

[rossipedia]

 notifications = switch(action.type) {
    when NEW_CHAT_MESSAGE:

@naw I thought this was ES6, but the above construct tripped me up. what lang are you using in your sample?

[naw]

@rossipedia My apologies --- I was trying to write regular javascript switch statements, but clearly I had some errors on my memory on how to construct them in more than one way. Of course the object spread stuff is ES6. I have updated the code in my original comment.

I hope this didn't distract from my point, which was just to provide a simple concrete example of how the logic would be separated differently with the action-centric and slice-centric approaches.

[pitaj]

@naw let's say I have your example as the state I want to hold. You mentioned earlier that I might be better to reorganize my state tree. How would that be done with the state tree in your example while staying with the branch-centric approach?

[naw]

@pitaj Do you mean the pets example or the chatMessages, friendRequests example?

[pitaj]

@naw I was referring to the second one with chat messages, friendRequests, notifications.

[naw]

@pitaj

There are three distinct slices of state, and they can each be updated by a separate sub-reducer. None of the slices depend on the other slices, so none of them needs to read more state than they're responsible for writing.

Here is the same example refactored slightly to use combineReducers with three sub-reducers:

const friendRequestsReducer =  function(friendRequests = [], action) {
  switch(action.type) {
    case NEW_FRIEND_REQUEST:
      return friendRequests.concat(action.friendRequest);
    default:
      return friendRequests;
  }
}

const chatMessagesReducer = function(chatMessages = [], action) {
  switch(action.type) {
  case NEW_CHAT_MESSAGE:
    return chatMessages.concat(action.chatMessage);
  default:
    return chatMessages;
  }
}

const notificationsReducer = function(notifications = [], action) {
  switch(action.type) {
    case NEW_CHAT_MESSAGE:
      return notifications.concat({ notification_type: 'chatMessage', notification_id: action.chatMessage.id });
    case NEW_FRIEND_REQUEST:
      return notifications.concat({ notification_type: 'friendRequest', notification_id: action.friendRequest.id});
    default:
      return notifications;
  }
}

const mainReducer = combineReducers({
  friendRequests: friendRequestsReducer,
  chatMessages: chatMessagesReducer,
  notifications: notificationsReducer
})

[pitaj]

Okay, but what if, for instance, you assigned each friend request a unique incremental id. You have to store the last id used somewhere so when you create a new one, you know what the last id is. This could be stored as the last index in an array of friend requests, for instance. In order to create the notification, you have to point to this id (your URL routing requires the id), so that means it must depend on the state of the friendRequests branch. Is there a way to reconcile this situation?

[naw]

Is there a way to reconcile this situation?

Yes. Great question. The actionCreator is responsible for generating the unique id, not the reducer. This is more obvious if you're using a random id, because you can't call a random id generator in a reducer without making it unpure (which is a definite anti-pattern in redux). You'll notice in my example that the id is already held within the action payload.

If you're using a sequential id, the assignment could go in the reducer, but it's better for it to go in the actionCreator for exactly this issue of multiple reducers needing the id.

You probably don't want to use the actual array index as an id (inevitably this leads to issues in all but the simplest of cases, such as when you want to delete an entry, for example), but if you wanted a sequential id, the store could hold the lastId, and the actionCreator would fetch that lastId and increment it by 1 to get the next sequential id. You might wonder if this could lead to race conditions --- and generally I think it shouldn't unless you introduce asynchronous code in the wrong place. Nevertheless, for that reason, it's perhaps safer to stick with random id's.

Also, if you're building an application backed by a server API, then the real id for a record actually comes from an async server call with a database generating the unique id and making it available to the action creator before the data hits the reducer (unless you're doing optimistic updates). With chatMesssages and friendRequests, for example, those objects probably came from some websocket push and already have id's associated with them before redux even sees them.