feature request: make combineReducers recursive

[jimbolla]

Currently, building a reducer with combineReducers looks something like this:

const reducer = combineReducers({
  form,
  entities: combineReducers({
    customers: combineReducers({
      // ...
    }),
    products: combineReducers({
      // ...
    }),
    orders: combineReducers({
      // ...
    }),
    // ...
  }),
  ui: combineReducers({
    // ...
  })
});

... and it would be nice to be able to do this:

const reducer = combineReducers({
  form,
  entities: {
    customers: {
      // ...
    },
    products: {
      // ...
    ),
    orders: {
      // ...
    ),
    // ...
  }),
  ui: {
    // ...
  }
});

This would reduce some boilerplate.

Implementation should be rather trivial:

 src/combineReducers.js | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/src/combineReducers.js b/src/combineReducers.js
index f0ff7eb..04693a2 100644
--- a/src/combineReducers.js
+++ b/src/combineReducers.js
@@ -111,6 +111,8 @@ export default function combineReducers(reducers) {

     if (typeof reducers[key] === 'function') {
       finalReducers[key] = reducers[key]
+    } else if (typeof reducers[key] === 'object' && reducers[key]) {
+      finalReducers[key] = combineReducers(reducers[key])
     }
   }
   var finalReducerKeys = Object.keys(finalReducers)

I can create test(s) + PR for this if you think it's a good idea.

[markerikson]

Heh. SEARCH THE REPO FOR PREVIOUS ISSUES FIRST! :) :) :)

But yeah, this has been brought up numerous times before. See #1768 for the technically-still-open accepted attempt, discussion, and links to previous issues.

[jimbolla]

That's not the same thing as what I'm asking. I just want to eliminate extra calls to combineReducers to tighten up my syntax.

[markerikson]

Right, but the issue is that there's been many different alterations to combineReducers proposed, and Dan's been very hesitant to move forward with any of them. #1768 was the only one he actually gave any kind of a green light to.

You might want to skim through https://github.com/markerikson/redux-ecosystem-links/blob/master/reducers.md to see if any third-party libs do this already. I think https://github.com/convoyinc/combined-reduction looks fairly close.

[jimbolla]

I don't follow the logic... because one or more unrelated feature requests weren't accepted, that no other features should ever be proposed?

Looking at your list... I realize a new problem that it would actually take longer to go over that list to see if any meet my needs than to just copy+paste combineReducers and change what I need. I'm not sure how I feel about that.

[markerikson]

The biggest issue is that there's so many different proposals for what extra features should be added to combineReducers. Everyone has a different idea of how it should behave for certain situations (which is why I keep emphasizing "just the common use case" in the docs). It should be recursive, it should allow iteration over non--plain-JS-objects, it should allow passing the root state down as a third arg, it should allow passing the next-level-up state as a third arg, ..... . It's not that these are bad ideas in and of themselves, it's that they add additional complexity and probably lock in certain use cases to the exclusion of others.

Like many other aspects of Redux (subscription callbacks, dispatch handling, etc), the overarching theme is "we provide the basic primitives, and you can go build stuff in userland if you want to for your own specific use case". And people have, which is why that list of reducer utils is so long :)

[naw]

Part of the trouble is that combineReducers itself is more suited to user land or a separate library.

It's just one of many feasible abstractions that ultimately create what redux core wants --- a single reducer function. Conceptually, I'd be in favor of moving combineReduers to another library, even though that probably increases the perceived complexity for a beginner, since it's one more library to install.

Forgetting redux core for a moment, and just looking at combineReducers itself and the API that it exposes, I think @jimbolla's suggested API change is very much in line with the spirit of what combineReducers is meant to do. So, while I'd generally be against changes to combineReducers, particularly ones that turn it into some kind of all-encompassing Swiss army knife, I think this particular change is pretty reasonable. I suppose I could be convinced otherwise, as I might be missing some good counter arguments.

There are competing ideas for what parameters combineReducers could or should pass to the reducing functions, for example, but I can't think of a more intuitive view of what combineReducers could or should do when it encounters an object as a key's value instead of a function as a key's value. Are there any alternative views?

All of that said, rather than copy-pasting combineReducers or modifying it, it would be just as easy to build a wrapper that transforms the terser syntax into the object that combineReducers expects, and this would be easy to supply in user land or a 3rd party library while still piggybacking on the "official" code. So, rather than calling combineReducers(yourObject), you'd call combineReducers(transform(yourObject));

Lots of respect for both of you @jimbolla and @markerikson , just sharing my 2 cents.

[markerikson]

Yeah - strictly speaking, applyMiddleware and combineReducers both aren't "required" to be in Redux's core library. There'd be a semi-valid argument for moving both to separate packages, similar to how redux-thunk is separated. However, they're so useful and idiomatic that it makes sense to include them in core. (For combineReducers, particularly, it exactly maps to Dan's preferred approach to structuring reducers.)

The big blocker to any real changes to combineReducers at this point is that it would really require Dan's buy-off. Getting his attention is gonna be tough atm, given his focus on CRA and the React docs.

[davidonlaptop]

Not sure if I'm using an unsupported feature, but this works:

const rootReducer = combineReducers({
    taxPage : taxPageReducer
});

export default function taxPageReducer(state = INITIAL_STATE, action) {
    return {
        ...state,
        taxSection: taxSectionReducer(state.taxSection, action)
    };
}

export default function taxSectionReducer(state = INITIAL_STATE, action) {
    switch (action.type) {
        case c.FETCH_TAXES_FULFILLED:
            state = {...state, records: action.payload};
            break;
    }
    return state;
}

It results with the following state:

const state = {
    taxPage: {
        taxSection: {
            collapsed: false,
            loading: false,
            error: false,
            records: []
        }
    }
}

[naw]

@davidonlaptop It's perfectly fine for one reducer to delegate work to other functions/reducers as you've done, but I don't see how your code is relevant to the original issue proposed. Perhaps you could clarify.

[davidonlaptop]

@naw I was looking for a solution on nested reducers which could have been solved with recursive reducers. So perhaps it could be of some help to the original author.

But you got me there, it is not precisely what is described in the issue.

[naw]

@davidonlaptop Thanks for clarifying.

Are you under the impression that you can't nest reducers with combineReducers?

If so, please be aware that nesting works fine with combineReducers:

const rootReducer = combineReducers({
    taxPage : taxPageReducer
});

const taxPageReducer = combineReducers({
  taxSection: taxSectionReducer
});

Unless you have special circumstances, you shouldn't need the explicit code you wrote above just to achieve nesting.

[davidonlaptop]

Much simpler this way! Thanks @naw!