Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Proposal] Make combineReducers pass extra arguments to child reducers #1904

Closed
wants to merge 1 commit into from
Closed

[Proposal] Make combineReducers pass extra arguments to child reducers #1904

wants to merge 1 commit into from

Conversation

aikoven
Copy link
Collaborator

@aikoven aikoven commented Aug 21, 2016

This can be useful a couple cases:

1. Reducers with dependencies

In my opinion the most explicit and convenient way to have one branch of reducers dependent on another branch is to make them have common parent that calculates dependency branch first and then passes its to the dependent reducer:

const dependencyReducer = (state = ..., action) => {...};

const dependentReducer = (state = ..., action, dependency) => {...};

const parentReducer = (state = {}, action) => {
  const {dependency, dependent} = state;

  const nextDependency = dependencyReducer(state, action);
  const nextDependent = dependentReducer(state, action, nextDependency);

  if (nextDependency !== dependency || nextDependent !== dependent)
    return {dependency: nextDependency, dependent: nextDependent};

  return state;
};

The change in this PR allows dependentReducer to be built from smaller parts, each one accepting dependency.

2. Multiple branches of same shape

Suppose we are developing an instant messenger. We can switch between chat views for different contacts and we want to store some data for each view, e.g. entered text, scroll position etc.

Let's structure state like this:

{
  alice: {
    enteredText: '...',
    scrollPosition: ...,
  },
  bob: {
    enteredText: '...',
    scrollPosition: ...,
  },
  ...
}

Actions:

const chatOpened = (contact) => ({
  type: 'CHAT_OPENED',
  contact,
});

const textEntered = (contact, text) => ({
  type: 'TEXT_ENTERED',
  contact,
  text,
});

My first approach to this was to have parent reducer look into action's contact prop and pass action to chatViewReducer for particular branch. But that couples parent to child, parent has to know of all actions that child accepts etc.

So my current approach is to pass all actions to chatViewReducer for all child branches with contact passed as third argument:

const chatsReducer = (state = {}, action) => {
  if (action.type === 'CHAT_OPENED') {
    return {
      ...state, 
      [action.contact]: chatViewReducer(undefined, action, action.contact),
    };
  }

  const nextState = {};
  let hasChanged = false;

  for (let contact of Object.keys(state)) {
    nextState[contact] = chatViewReducer(state[contact], action, contact);
    hasChanged = hasChanged || nextState[contact]  !== state[contact];
  }

  return hasChanged ? nextState : state;
};

Once again, this change allows chatViewReducers to be built from smaller parts each accepting contact as third argument, allowing them to filter incoming actions by contact to see if they correspond to that particular chat view.

Right now I have a custom combineReducers version that is almost the same as original.

Also, the above pattern can be generalised as reducer factory:

/**
 * @param childReducer reducer for state branches
 * @param shouldAddKey function that accepts action and returns undefined or 
 *   string key to be added to state
 * @param shouldRemoveKey function that accepts action and returns undefined or
 *   string key to be removed from state
 */
function createMappingReducer(childReducer, shouldAddKey, shouldRemoveKey) {
  ...
}

const chatsReducer = createMappingReducer(chatViewReducer, (action) => {
  if (action.type === 'CHAT_OPENED')
    return action.contact;
});

@markerikson
Copy link
Contributor

markerikson commented Aug 21, 2016

This has been proposed numerous times, and was generally rejected. The last time this was proposed was #1768 , which appears to have stalled due to questions about what exactly the new behavior should involve.

Remember that combineReducers is simply a utility function for the most common use case, and is not intended to handle all situations. If you need more specific behavior, you can always write your own variation on the concept.

You may also be interested in my WIP addition to the Redux docs on the topic of Structuring Reducers (#1784), which gives some further suggestions for ways to handle things. There's also a wide variety of Redux reducer-related libraries and utilities in the Redux Addons Catalog - you could always publish your own version separately.

Given that #1768 is already around, I'm going to close this for now. If @gaearon happens to think that further changes to combineReducers are warranted, this can be reopened.

@aikoven
Copy link
Collaborator Author

aikoven commented Aug 22, 2016

OK, no probs!

This is a bit different from #1768, because it doesn't add any semantics or implicit logic. Whatever you pass as third argument is up to you, it just allows to implement many useful patterns without requiring users to use custom combineReducers function.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants