From 47d00243be1cbb4d50099c1fda3f45cd5575732e Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Sat, 30 Jan 2016 21:16:23 +0000 Subject: [PATCH] Object returned from bindActionCreators() should only include functions --- src/bindActionCreators.js | 17 +++++++++++------ src/combineReducers.js | 27 +++++++++++++++++---------- src/utils/mapValues.js | 13 ------------- src/utils/pick.js | 15 --------------- test/bindActionCreators.spec.js | 25 ++++++++++++++++++++++++- test/utils/mapValues.spec.js | 16 ---------------- test/utils/pick.spec.js | 16 ---------------- 7 files changed, 52 insertions(+), 77 deletions(-) delete mode 100644 src/utils/mapValues.js delete mode 100644 src/utils/pick.js delete mode 100644 test/utils/mapValues.spec.js delete mode 100644 test/utils/pick.spec.js diff --git a/src/bindActionCreators.js b/src/bindActionCreators.js index 59fc861b7a..e70e270fd4 100644 --- a/src/bindActionCreators.js +++ b/src/bindActionCreators.js @@ -1,5 +1,3 @@ -import mapValues from './utils/mapValues' - function bindActionCreator(actionCreator, dispatch) { return (...args) => dispatch(actionCreator(...args)) } @@ -30,14 +28,21 @@ export default function bindActionCreators(actionCreators, dispatch) { return bindActionCreator(actionCreators, dispatch) } - if (typeof actionCreators !== 'object' || actionCreators === null || actionCreators === undefined) { + if (typeof actionCreators !== 'object' || actionCreators === null) { throw new Error( `bindActionCreators expected an object or a function, instead received ${actionCreators === null ? 'null' : typeof actionCreators}. ` + `Did you write "import ActionCreators from" instead of "import * as ActionCreators from"?` ) } - return mapValues(actionCreators, actionCreator => - bindActionCreator(actionCreator, dispatch) - ) + var keys = Object.keys(actionCreators) + var boundActionCreators = {} + for (var i = 0; i < keys.length; i++) { + var key = keys[i] + var actionCreator = actionCreators[key] + if (typeof actionCreator === 'function') { + boundActionCreators[key] = bindActionCreator(actionCreator, dispatch) + } + } + return boundActionCreators } diff --git a/src/combineReducers.js b/src/combineReducers.js index d9490634ee..9569f0cffe 100644 --- a/src/combineReducers.js +++ b/src/combineReducers.js @@ -1,7 +1,5 @@ import { ActionTypes } from './createStore' import isPlainObject from './utils/isPlainObject' -import mapValues from './utils/mapValues' -import pick from './utils/pick' import warning from './utils/warning' function getUndefinedStateErrorMessage(key, action) { @@ -92,11 +90,18 @@ function assertReducerSanity(reducers) { * @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) { - var finalReducers = pick(reducers, (val) => typeof val === 'function') - var sanityError + var reducerKeys = Object.keys(reducers) + var finalReducers = {} + for (var i = 0; i < reducerKeys.length; i++) { + var key = reducerKeys[i] + if (typeof reducers[key] === 'function') { + finalReducers[key] = reducers[key] + } + } + var finalReducerKeys = Object.keys(finalReducers) + var sanityError try { assertReducerSanity(finalReducers) } catch (e) { @@ -116,17 +121,19 @@ export default function combineReducers(reducers) { } var hasChanged = false - var finalState = mapValues(finalReducers, (reducer, key) => { + var nextState = {} + 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) if (typeof nextStateForKey === 'undefined') { var errorMessage = getUndefinedStateErrorMessage(key, action) throw new Error(errorMessage) } + nextState[key] = nextStateForKey hasChanged = hasChanged || nextStateForKey !== previousStateForKey - return nextStateForKey - }) - - return hasChanged ? finalState : state + } + return hasChanged ? nextState : state } } diff --git a/src/utils/mapValues.js b/src/utils/mapValues.js deleted file mode 100644 index 165d6667ad..0000000000 --- a/src/utils/mapValues.js +++ /dev/null @@ -1,13 +0,0 @@ -/** - * Applies a function to every key-value pair inside an object. - * - * @param {Object} obj The source object. - * @param {Function} fn The mapper function that receives the value and the key. - * @returns {Object} A new object that contains the mapped values for the keys. - */ -export default function mapValues(obj, fn) { - return Object.keys(obj).reduce((result, key) => { - result[key] = fn(obj[key], key) - return result - }, {}) -} diff --git a/src/utils/pick.js b/src/utils/pick.js deleted file mode 100644 index a61e1f1e7b..0000000000 --- a/src/utils/pick.js +++ /dev/null @@ -1,15 +0,0 @@ -/** - * Picks key-value pairs from an object where values satisfy a predicate. - * - * @param {Object} obj The object to pick from. - * @param {Function} fn The predicate the values must satisfy to be copied. - * @returns {Object} The object with the values that satisfied the predicate. - */ -export default function pick(obj, fn) { - return Object.keys(obj).reduce((result, key) => { - if (fn(obj[key])) { - result[key] = obj[key] - } - return result - }, {}) -} diff --git a/test/bindActionCreators.spec.js b/test/bindActionCreators.spec.js index cbc025f5a8..f9fcc5c264 100644 --- a/test/bindActionCreators.spec.js +++ b/test/bindActionCreators.spec.js @@ -5,9 +5,16 @@ import * as actionCreators from './helpers/actionCreators' describe('bindActionCreators', () => { let store + let actionCreatorFunctions beforeEach(() => { store = createStore(todos) + actionCreatorFunctions = { ...actionCreators } + Object.keys(actionCreatorFunctions).forEach(key => { + if (typeof actionCreatorFunctions[key] !== 'function') { + delete actionCreatorFunctions[key] + } + }) }) it('wraps the action creators with the dispatch function', () => { @@ -15,7 +22,7 @@ describe('bindActionCreators', () => { expect( Object.keys(boundActionCreators) ).toEqual( - Object.keys(actionCreators) + Object.keys(actionCreatorFunctions) ) const action = boundActionCreators.addTodo('Hello') @@ -27,6 +34,22 @@ describe('bindActionCreators', () => { ]) }) + it('skips non-function values in the passed object', () => { + const boundActionCreators = bindActionCreators({ + ...actionCreators, + foo: 42, + bar: 'baz', + wow: undefined, + much: {}, + test: null + }, store.dispatch) + expect( + Object.keys(boundActionCreators) + ).toEqual( + Object.keys(actionCreatorFunctions) + ) + }) + it('supports wrapping a single function only', () => { const actionCreator = actionCreators.addTodo const boundActionCreator = bindActionCreators(actionCreator, store.dispatch) diff --git a/test/utils/mapValues.spec.js b/test/utils/mapValues.spec.js deleted file mode 100644 index 3aef6fe3b6..0000000000 --- a/test/utils/mapValues.spec.js +++ /dev/null @@ -1,16 +0,0 @@ -import expect from 'expect' -import mapValues from '../../src/utils/mapValues' - -describe('mapValues', () => { - it('returns object with mapped values', () => { - const test = { - a: 'c', - b: 'd' - } - expect(mapValues(test, (val, key) => val + key)).toEqual({ - a: 'ca', - b: 'db' - }) - }) -}) - diff --git a/test/utils/pick.spec.js b/test/utils/pick.spec.js deleted file mode 100644 index 745ba72c29..0000000000 --- a/test/utils/pick.spec.js +++ /dev/null @@ -1,16 +0,0 @@ -import expect from 'expect' -import pick from '../../src/utils/pick' - -describe('pick', () => { - it('returns object with picked values', () => { - const test = { - name: 'lily', - age: 20 - } - expect( - pick(test, x => typeof x === 'string') - ).toEqual({ - name: 'lily' - }) - }) -})