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

2nd argument for sliced states added to combineReducers #2072

Closed
wants to merge 1 commit into from

Conversation

faceyspacey
Copy link

@faceyspacey faceyspacey commented Nov 2, 2016

Hey Dan I know you run a tight shift, but I also noticed you recently added an extraArgument parameter to redux-thunk. My thinking here is that this makes for a great introduction to "slicing" state as described here: http://redux.js.org/docs/recipes/reducers/BeyondCombineReducers.html

I think a lot of developers end up using combineReducers for a bit too long and end up engaging in various anti-patterns to get access to other states in their reducers, or achieve access in components via other less-than-optimum ways. Slicing state at first can be intimidating and you're almost scared to customize your root reducer. I think a lot of developers--myself included--go through that stage. This is a great bridge for developers toward understanding that their reducer function is really "just a function," while also helping them avoid those anti-patterns.

It's really quite limiting at first to think of your reducers as these independent atoms that have no knowledge of state elsewhere. I know I worked around it way longer than I should. That said, that limitation is one thing that makes Redux really easy to understand at first. It just becomes complicated when you want to do more. This could serve as a taste to inspire them to go further down the path of making custom reducers, while also solving one of the biggest problems developers new to redux will have. For many apps, the missing piece is just one reducer's state that they wish other reducers had access to. Shifting to a very custom reducer from combineReducers just to achieve that can be a big "ask" for developers new to redux. With this, they just need to treat a single reducer separately in a simple rootReducer function, and then pass it to combineReducers and then return both in the final returned state. Basically here's the intended use case:

const appReducer = combineReducers(reducers);

return function rootReducer(state={}, action) {
   let location = locationReducer(state.location, action);
   delete state.location; //silence combineReducers warning for passing keys it's unaware of

   return {
     ...appReducer(state, action, location),
     location,
   };
}

Currently in one of my apps, I'm unnecessarily shipping a second combineReducers function that does just this. To feed my obsession with keeping bundle size as small as possible, I personally would love to just import the one from redux. Either way, great job with Redux! It really has taken Reactlandia to the place it was supposed to go.

Hey Dan I know you run a tight shift, but I also noticed you recently added an `extraArgument` parameter to `redux-thunk`.  My thinking here is that this makes for a great introduction to "slicing" state as described here: http://redux.js.org/docs/recipes/reducers/BeyondCombineReducers.html 

I think a lot of developers end up using `combineReducers` for a bit too long and end up engaging in various anti-patterns to get access to other states in their reducers, or achieve access in components via other less-than-optimum ways. Slicing state at first can be intimidating and you're almost scared to customize your root reducer. I think a lot of developers--myself included--go through that stage. This is a great bridge for developers toward understanding that their reducer function is really "just a function," while also helping them avoid those anti-patterns. 

It's really quite limiting at first to think of your reducers as these independent atoms that have no knowledge of anything else. I know I worked around it way longer than I should. That said, that limitation is one thing that makes Redux really easy to understand at first. It just becomes complicated when you want to do more. This could serve as a taste to inspire them to go further down the path of making custom reducers, while also solving one of the biggest problems developers new to redux will have. 

Currently in one of my apps, I'm unnecessarily shipping a second `combineReducers` function that does just this. To feed my obsession with keeping bundle size as small as possible, I personally would love to just import the one from redux since. Either way, great job with Redux! It really has taken *Reactlandia* to the place it was supposed to go.
@markerikson
Copy link
Contributor

Yeah, modifications to combineReducers are probably the most frequently requested change to Redux (along with passing some kind of additional info in the subscription callbacks). Unfortunately, there's so many different requests, and they tend to conflict with each other. Should additional arguments be the root state, the next-level-up state, or some kind of callback? Should combineReducers warn on extra keys because you might not be handling things, or ignore those because it should be letting things pass through? Should it provide some generic way to iterate over something other than plain JS objects, like Immutable.js Maps?

Dan has historically been against making further changes to combineReducers. He did agree to let #1768 proceed to see how it would turn out, but that seems to have stalled.

The PR is appreciated, but I think for now it's best to close it. We probably ought to try to resume discussion in #1768 to figure out if we actually do want to make changes to combineReducers, and if so, what.

All that said, one other argument for not modifying combineReducers is that it does force the user to look at writing their own approach. I get that that can be a hurdle for newer devs, but I also think it's an important step in truly grasping Redux. That's a major reason why I did write that whole "Structuring Reducers" section - to try to help people understand that they're not just limited to using combineReducers as-is. (Also, for that matter, the FAQ question at http://redux.js.org/docs/faq/Reducers.html#reducers-share-state ).

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.

2 participants