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

#1792 candidate: Make combineReducers shape-agnostic #1817

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,7 @@
"expect": "^1.8.0",
"gitbook-cli": "^0.3.4",
"glob": "^6.0.4",
"immutable": "3.8.1",
"isparta": "^4.0.0",
"mocha": "^2.2.5",
"rimraf": "^2.3.4",
Expand Down
55 changes: 42 additions & 13 deletions src/combineReducers.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import { ActionTypes } from './createStore'
import isPlainObject from 'lodash/isPlainObject'
import warning from './utils/warning'

function getUndefinedStateErrorMessage(key, action) {
Expand All @@ -12,8 +11,7 @@ function getUndefinedStateErrorMessage(key, action) {
)
}

function getUnexpectedStateShapeWarningMessage(inputState, reducers, action) {
var reducerKeys = Object.keys(reducers)
function getUnexpectedStateShapeWarningMessage(inputState, keysMethod, reducers, reducerKeys, action) {
var argumentName = action && action.type === ActionTypes.INIT ?
'preloadedState argument passed to createStore' :
'previous state received by the reducer'
Expand All @@ -25,7 +23,7 @@ function getUnexpectedStateShapeWarningMessage(inputState, reducers, action) {
)
}

if (!isPlainObject(inputState)) {
if (typeof inputState !== 'object') {
return (
`The ${argumentName} has unexpected type of "` +
({}).toString.call(inputState).match(/\s([a-z|A-Z]+)/)[1] +
Expand All @@ -34,7 +32,14 @@ function getUnexpectedStateShapeWarningMessage(inputState, reducers, action) {
)
}

var unexpectedKeys = Object.keys(inputState).filter(key => !reducers.hasOwnProperty(key))
try {
var unexpectedKeys = keysMethod(inputState).filter(key => !reducers.hasOwnProperty(key))
} catch (e) {
return (
`The provided options.keys failed on ${argumentName}. ` +
'Could not check for unexpectedKeys.'
)
}

if (unexpectedKeys.length > 0) {
return (
Expand Down Expand Up @@ -87,10 +92,23 @@ function assertReducerSanity(reducers) {
* if the state passed to them was undefined, and the current state for any
* unrecognized action.
*
* @param {Object} [options={
* create: (obj) => obj,
* get: (obj, key) => obj[key],
* keys: (obj) => Object.keys(obj)
* }] An optional object that defines how to create a new state, how to
* get a property from the state object, and how to iterate over the keys
* of a state object.
*
* @returns {Function} A reducer function that invokes every reducer inside the
* passed object, and builds a state object with the same shape.
*/
export default function combineReducers(reducers) {
export default function combineReducers(reducers, options) {
options = Object.assign({
create: (obj) => obj,
get: (obj, key) => obj[key],
keys: (obj) => Object.keys(obj)
}, options)
var reducerKeys = Object.keys(reducers)
var finalReducers = {}
for (var i = 0; i < reducerKeys.length; i++) {
Expand All @@ -108,32 +126,43 @@ export default function combineReducers(reducers) {
sanityError = e
}

return function combination(state = {}, action) {
return function combination(state = options.create({}), action) {
if (sanityError) {
throw sanityError
}

if (process.env.NODE_ENV !== 'production') {
var warningMessage = getUnexpectedStateShapeWarningMessage(state, finalReducers, action)
var warningMessage = getUnexpectedStateShapeWarningMessage(state, options.keys, finalReducers, finalReducerKeys, action)
if (warningMessage) {
warning(warningMessage)
}
}

var hasChanged = false
var nextState = {}
var nextStateDescriptor = {}
for (var i = 0; i < finalReducerKeys.length; i++) {
var key = finalReducerKeys[i]
var reducer = finalReducers[key]
var previousStateForKey = state[key]
var nextStateForKey = reducer(previousStateForKey, action)

try {
var previousStateForKey = options.get(state, key)
} catch (e) {
throw new Error(`Could not get key "${key}" using ${options.get}`)
}

try {
var nextStateForKey = reducer(previousStateForKey, action)
} catch (e) {
throw new Error(`Reducer for key "${key}" failed`)
}

if (typeof nextStateForKey === 'undefined') {
var errorMessage = getUndefinedStateErrorMessage(key, action)
throw new Error(errorMessage)
}
nextState[key] = nextStateForKey
nextStateDescriptor[key] = nextStateForKey
hasChanged = hasChanged || nextStateForKey !== previousStateForKey
}
return hasChanged ? nextState : state
return hasChanged ? options.create(nextStateDescriptor) : state
}
}
61 changes: 61 additions & 0 deletions test/combineReducers.spec.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import expect from 'expect'
import { combineReducers } from '../src'
import Immutable from 'immutable'
import createStore, { ActionTypes } from '../src/createStore'

describe('Utils', () => {
Expand Down Expand Up @@ -31,6 +32,66 @@ describe('Utils', () => {
).toEqual([ 'stack' ])
})

it('can use custom create/get/keys methods', () => {
const create = (obj) =>
obj instanceof Immutable.Map ? obj : new Immutable.Map(obj)
const get = (obj, key) =>
obj.get(key)
const keys = (obj) =>
obj.keySeq().toArray()
const stack = (state = [], action) =>
action.type === 'PUSH' ? state.concat(action.value) : state
const PUSH_ONE = { type: 'PUSH', value: 1 }

const reducer = combineReducers({ stack }, { create, get, keys })

const s1 = reducer(undefined, PUSH_ONE)
expect(s1.get('stack')).toEqual( [ 1 ] )

const spy = expect.spyOn(console, 'error')

// throws for non-objects
expect(
() => reducer(2, PUSH_ONE)
).toThrow(
/Could not get key "stack"/
)
expect(spy.calls[0].arguments[0]).toMatch(
/The previous state received.*type of "Number"/
)

// throws if it can't get a prop
expect(() =>
reducer({ stack: [] }, PUSH_ONE)
).toThrow(
/Could not get key "stack".*/
)
expect(spy.calls[1].arguments[0]).toMatch(
/The provided options.keys failed.*/
)

// warns when it gets an unexpected key
reducer(create({ boof: 1 }), PUSH_ONE)
expect(spy.calls[2].arguments[0]).toMatch(
/Unexpected key "boof".*/
)

// warns when it can't check for unexpectedKeys
reducer({ get: () => [] }, PUSH_ONE)
expect(spy.calls[3].arguments[0]).toMatch(
/The provided options.keys failed on previous state.*/
)

// warns unexpected keys if it tries default key iterator
const reducer2 = combineReducers({ stack }, { create, get }) // no keys
reducer2(create(), {})
expect(spy.calls[4].arguments[0]).toMatch(
/Unexpected keys "size".*/
)

spy.restore()
})

it('throws an error if a reducer returns undefined handling an action', () => {
const reducer = combineReducers({
counter(state = 0, action) {
Expand Down