-
Notifications
You must be signed in to change notification settings - Fork 36
Open
Description
Currently combineReducers({sub1, sub2}) returns an Immutable.Map. However, Immutable.js provides a native Record type which is more natural fit for holding data with specific, unchanging set of keys.
Here's the breakdown:
| Option | Immutable.Map | Immutable.Record |
|---|---|---|
| Accessing sub-states | state.get('sub1') |
state.sub1 - much cleaner (but state.get('sub1') also supported) |
| Creating inexistent sub-states | Possible by mistake: state.set('foo', 5) |
Not allowed |
I believe redux-immutablejs must be improved to return a Record instead of a Map by default. This will be fully backwards-compatible in all cases besides creating inexistent sub-states (which is a clear misuse of the combined reducers pattern).
By the way, it is already possible to pass a Record instance to combineReducers, but it requires non-trivial boilerplate code (which is most probably suboptimal as well):
import Immutable from 'immutable'
import {combineReducers} from 'redux-immutablejs'
import sub1 from './sub1'
import sub2 from './sub2'
class State extends Immutable.Record({
sub1: null,
sub2: null
}) {
// The two methods below allow to trick combineReducers to accept this class
// instead of Immutable.Map or plain object
filter() {
return this;
}
map(k, v) {
return new State(Immutable.Map(this).map(k, v));
}
}
export default combineReducers(new State({sub1, sub2}));Metadata
Metadata
Assignees
Labels
No labels