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

Object returned from bindActionCreators() should only include functions #1329

Merged
merged 1 commit into from
Jan 30, 2016

Conversation

gaearon
Copy link
Contributor

@gaearon gaearon commented Jan 30, 2016

Currently the input object to bindActionCreators is not filtered to include just functions. Because of this, the return value may include weird things. For example, if you put action constants into actions.js together with action creators, the object returned by bindActionCreators (and potentially merged into your props if you use React) will include keys like MY_CONSTANT. The values under these keys will be broken functions that attempt to dispatch the result of calling MY_CONSTANT (and, obviously, failing). This is a buggy behavior that could not be relied upon in any way (it would crash if you tried to call those functions) so fixing it isn’t a breaking change.

While fixing it, I went with hardcoding some for loops instead of mapValues / pick to have better control over where we are allocating and iterating over things and why. I’m not counting on performance improvements from this but I don’t want to pick and then mapValues when I can do this with a single allocation, and now that I write custom code in bindActionCreators, I might as well do it in combineReducers and hoist the Object.keys() call outside the combined reducer.

Incidentally this also means we don't need utilities from Lodash other than isPlainObject. (cc @jdalton)
Hopefully I'm not making the perf worse with this 😅

gaearon added a commit that referenced this pull request Jan 30, 2016
Object returned from bindActionCreators() should only include functions
@gaearon gaearon merged commit 7f301d7 into master Jan 30, 2016
@gaearon gaearon deleted the bind-action-creators-1 branch January 30, 2016 21:40
@gaearon gaearon mentioned this pull request Jan 30, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant