Skip to content
This repository has been archived by the owner on Jan 10, 2018. It is now read-only.

Added support for nesting in the combineReducers utility method. #214

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

KwintenP
Copy link

Current situation

During design of your state tree, you typically divide it up into different sections. F.e.

{
  ui: uiReducer,
  data: dataReducer
}

NGRX/Store provides the combineReducers method to easily work with such structures.

Problem description

If you want to work with multiple levels of nesting in your state tree, you need to do something else F.e.

{
  ui: {
      login:...,
      main: ... 
  } ,
  data: ...
}

In that case, you'd could:

  1. Write a uiReducer yourself which delegates every action related to login to a loginReducer and every 'main' related action to the mainReducer.
  2. Use a utility such as: https://github.com/brechtbilliet/create-reducer-tree which handles this for you
  3. Nest the combineReducers method like this:
const rootReducer = 
{
   ui: combineReducers({
         login: loginReducer,
         main: mainReducer
  }),
  data: dataReducer
}

Improvement description

Option 1 provides you with extra work and option 2 forces you to work with a third party library. I personally prefer option 3 where you nest the combineReducers method inside your tree.
This could actually be easily integrated into the current combineReducers method and make the following possible:

rootReducer = {
   ui: {
      login: loginReducer,
      main: mainReducer
   },
   data: dataReducer
}

new Store(rootReducer)

It's a lot cleaner than approach where you nest the combineReducers method yourself.
This implemented by making the combineReducers function a recursive one.

Happy to hear any feedback on the feature request/implementation :).

I also created test for this functionality. If you feel these are insufficient or not in the correct place/file, I'm happy to change this as per your instructions.

@SebastianStehle
Copy link

I like it, I also would like to see multiple reducers for one slice. My scenario is a diagramming application, where you have a lot of actions, e.g.

rootReducers = {
    diagram: [
        groupingReducers,
        selectionReducer,
        arrangingReducer,
        itemsReducer,
        dragDropReducer
    ],
    ...
}

this helps keeping the reducers small.

@robwormald
Copy link
Contributor

This seems like a reasonable feature, but I'm increasingly against adding any more complexity to the core of Store, especially as we look into adding lazy-loaded reducers in #211. Is this something we could add to Store or Core as a utility?

@colthreepv
Copy link

Any news on that? has it been released stand-alone?

@moravcik
Copy link

moravcik commented Mar 9, 2017

in my opinion this is rather a syntactic sugar than adding more complexity, which would be helpful if merged

@KwintenP
Copy link
Author

Created a separate npm package for this. Am using it in several project and this way I don't have to copy it over all the time. Would love to see this actually getting into the actual project.

You can find it here: https://github.com/KwintenP/combine-reducers-enhanced
Npm package name is: combine-reducers-enhanced.

@dszmaj
Copy link

dszmaj commented Sep 30, 2017

I think this is like a "must" feature for a bigger, modular projects which are composed from modules with own store implementations in each that create a tree of different states and reducers for parts of those states. With this all we must do to "enable" such module is importing its reducers object in root app reducers object.

Despite limitations it seems to "work", but doesn't initialize states from modules which could effect in code not executing if it relies on store dispatching state with those properties. Solution is dirty and involves creating "Effect" for dispatching initial action to store from this module.

Please consider adding it at least as an option to choose between standard "combineReducers".

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

Successfully merging this pull request may close these issues.

6 participants