RFC: Make combineReducers() shape-agnostic

[gaearon]

I propose we should make combineReducers() work with ImmutableJS and any other libraries. This way all the checks it makes would work out of the box, but the actual state transformation for associating the values would be swappable.

I’m thinking something like combineReducers(reducers, [options]).
By default options is something like

{
  create: (obj) => obj,
  get: (state, key) => state[key]
}

but you could supply something like

{
  create: (obj) => (obj instanceof Immutable.Map) ? obj : new Immutable.Map(obj),
  get: (state, key) => state.get(key)
}

Does this make sense to anyone?
If you’d like to work on a PR to make a proof of concept, please comment here and submit it!

If you think it’s a bad idea, please write here as well.

[bvaughn]

I know it's just an example listed, but I'd suggest changing to use fromJS (since it does a deep conversion). Edit: Removed after below discussion.

[markerikson]

Ooo. I _like_ this idea. 👍 💯

[gaearon]

Deep conversion is not necessarily what you want (you might have nested plain object reducers). Also it would be much slower in the common case.

[bvaughn]

Fair enough. In my experience, shallow conversion commonly leads to errors (eg methods like getIn and setIn don't work as expected, you can accidentally mutate nodes in your immutable tree, etc). But I'm sure you've thought about this particular use case much more than I have. :)

[markerikson]

To expand on my comment: this seems useful, simplifies interop, and is consistent with the "sensible default with optional customization" approach used by mapState and mapDispatch. Seems like all-upsides on this one.

[gaearon]

@bvaughn

Since it's pluggable you would be able to customize it. But it would be super confusing if applying it at the root level broke third-party reducers from another packages, for example.

Usually combineReducers() has no effect on the state of the tree below. It just manages one level deep. It would be strange if suddenly it took over the state returned by nested reducers and transformed it. So while the approach you suggest is possible, it isn't something we'd encourage.

[markerikson]

Quick question: How well does this tie in with the other open PRs to combineReducers, #1768 and #1789 ?

[kennetpostigo]

Don't use fromJS it's really expensive operation in immutable lib.

[bvaughn]

There are lots of good reasons to use it, just not in this context as Dan explained. I've struck-through my suggestion so others won't read it and not notice the below discussion. :)

[rt2zz]

Big 👍 . Having this in redux will provide guidance to other projects for how to support this scenario, e.g. this has been a long standing question in redux-persist.

I believe the configuration would also need an iterator method.

[benlesh]

I've personally never been a fan of these sorts of pre and post processing configs. js-csp has something like this I think. I prefer something more functional and composable I think

[deanrad]

It's fine that combineReducers has its utility for delegating responsibility for keys of objects, I just wish the Redux docs showed a simple reducer to be delegation driven as well, ala this gist or simply:

createReducer({
  ADD: (s, a) => s + a,
  MULT: (s, a) => s * a
}, 10)