From 111700b057c3fd59ba8a1e86ce623878a63de55e Mon Sep 17 00:00:00 2001 From: Tim Griesser Date: Sun, 8 Nov 2015 13:38:58 -0500 Subject: [PATCH 1/4] Initial pass at wrapMapState & wrapMapDispatch --- src/components/connect.js | 35 +++++++++++++-------- test/components/connect.spec.js | 56 ++++++++++++++++++++++++++++++++- 2 files changed, 77 insertions(+), 14 deletions(-) diff --git a/src/components/connect.js b/src/components/connect.js index 5b31632d1..2df1bbe02 100644 --- a/src/components/connect.js +++ b/src/components/connect.js @@ -22,24 +22,24 @@ function getDisplayName(WrappedComponent) { let nextVersion = 0 export default function connect(mapStateToProps, mapDispatchToProps, mergeProps, options = {}) { + const { pure = true, withRef = false, wrapMapState = false, wrapMapDispatch = false } = options const shouldSubscribe = Boolean(mapStateToProps) const finalMapStateToProps = mapStateToProps || defaultMapStateToProps const finalMapDispatchToProps = isPlainObject(mapDispatchToProps) ? wrapActionCreators(mapDispatchToProps) : mapDispatchToProps || defaultMapDispatchToProps const finalMergeProps = mergeProps || defaultMergeProps - const shouldUpdateStateProps = finalMapStateToProps.length > 1 - const shouldUpdateDispatchProps = finalMapDispatchToProps.length > 1 - const { pure = true, withRef = false } = options + const shouldUpdateStateProps = wrapMapState || finalMapStateToProps.length > 1 + const shouldUpdateDispatchProps = wrapMapDispatch || finalMapDispatchToProps.length > 1 // Helps track hot reloading. const version = nextVersion++ - function computeStateProps(store, props) { + function computeStateProps(connectMapStateToProps, store, props) { const state = store.getState() const stateProps = shouldUpdateStateProps ? - finalMapStateToProps(state, props) : - finalMapStateToProps(state) + connectMapStateToProps(state, props) : + connectMapStateToProps(state) invariant( isPlainObject(stateProps), @@ -49,11 +49,11 @@ export default function connect(mapStateToProps, mapDispatchToProps, mergeProps, return stateProps } - function computeDispatchProps(store, props) { + function computeDispatchProps(connectMapDispatchToProps, store, props) { const { dispatch } = store const dispatchProps = shouldUpdateDispatchProps ? - finalMapDispatchToProps(dispatch, props) : - finalMapDispatchToProps(dispatch) + connectMapDispatchToProps(dispatch, props) : + connectMapDispatchToProps(dispatch) invariant( isPlainObject(dispatchProps), @@ -116,12 +116,20 @@ export default function connect(mapStateToProps, mapDispatchToProps, mergeProps, `or explicitly pass "store" as a prop to "${this.constructor.displayName}".` ) - this.stateProps = computeStateProps(this.store, props) - this.dispatchProps = computeDispatchProps(this.store, props) + this.wrapMapToProps() + this.stateProps = computeStateProps(this.connectMapStateToProps, this.store, props) + this.dispatchProps = computeDispatchProps(this.connectMapDispatchToProps, this.store, props) this.state = { storeState: null } this.updateState() } + wrapMapToProps() { + this.connectMapStateToProps = wrapMapState + ? wrapMapState(finalMapStateToProps) : finalMapStateToProps + this.connectMapDispatchToProps = wrapMapDispatch + ? wrapMapDispatch(finalMapDispatchToProps) : finalMapDispatchToProps + } + computeNextState(props = this.props) { return computeNextState( this.stateProps, @@ -131,7 +139,7 @@ export default function connect(mapStateToProps, mapDispatchToProps, mergeProps, } updateStateProps(props = this.props) { - const nextStateProps = computeStateProps(this.store, props) + const nextStateProps = computeStateProps(this.connectMapStateToProps, this.store, props) if (shallowEqual(nextStateProps, this.stateProps)) { return false } @@ -141,7 +149,7 @@ export default function connect(mapStateToProps, mapDispatchToProps, mergeProps, } updateDispatchProps(props = this.props) { - const nextDispatchProps = computeDispatchProps(this.store, props) + const nextDispatchProps = computeDispatchProps(this.connectMapDispatchToProps, this.store, props) if (shallowEqual(nextDispatchProps, this.dispatchProps)) { return false } @@ -226,6 +234,7 @@ export default function connect(mapStateToProps, mapDispatchToProps, mergeProps, this.version = version // Update the state and bindings. + this.wrapMapToProps() this.trySubscribe() this.updateStateProps() this.updateDispatchProps() diff --git a/test/components/connect.spec.js b/test/components/connect.spec.js index bae556e0a..09563352b 100644 --- a/test/components/connect.spec.js +++ b/test/components/connect.spec.js @@ -4,6 +4,7 @@ import ReactDOM from 'react-dom' import TestUtils from 'react-addons-test-utils' import { createStore } from 'redux' import { connect } from '../../src/index' +import shallowEqual from '../../src/utils/shallowEqual' describe('React', () => { describe('connect', () => { @@ -190,7 +191,7 @@ describe('React', () => { return ( - + ) } } @@ -1369,5 +1370,58 @@ describe('React', () => { // But render is not because it did not make any actual changes expect(renderCalls).toBe(1) }) + + it('accepts a wrapMapState option for configuring mapStateToProps for the component', () => { + + const store = createStore(() => ({ + prefix: 'name: ' + })) + let memoizeHits = 0 + let memoizeMisses = 0 + let renderCalls = 0 + + function memoize(func) { + let lastResult, lastArgs = lastResult = null + return (...args) => { + if (lastArgs && args.every((value, index) => shallowEqual(value, lastArgs[index]))) { + memoizeHits++ + return lastResult + } else if (lastArgs) { + memoizeMisses++ + } + lastArgs = args + lastResult = func(...args) + return lastResult + } + } + + function computeValue(state, props) { + return { value: props.prefix + state.name } + } + + @connect(() => memoize(computeValue), null, null, { wrapMapState: (fn) => fn() }) + class Container extends Component { + componentDidMount() { + this.forceUpdate() + } + render() { + renderCalls++ + return
{this.props.value}
+ } + } + + TestUtils.renderIntoDocument( + +
+ + +
+ + ) + + expect(renderCalls).toEqual(4) + expect(memoizeHits).toEqual(2) + expect(memoizeMisses).toEqual(0) + }) }) }) From 04267dc00fe55cc91e2bc1053aad6a6267f0127c Mon Sep 17 00:00:00 2001 From: Tim Griesser Date: Sun, 8 Nov 2015 14:06:53 -0500 Subject: [PATCH 2/4] api change, wrapMapState function -> mapStateThunk boolean --- src/components/connect.js | 40 ++++++++++++++++----------------- test/components/connect.spec.js | 4 ++-- 2 files changed, 22 insertions(+), 22 deletions(-) diff --git a/src/components/connect.js b/src/components/connect.js index 2df1bbe02..acdf26a2d 100644 --- a/src/components/connect.js +++ b/src/components/connect.js @@ -22,24 +22,24 @@ function getDisplayName(WrappedComponent) { let nextVersion = 0 export default function connect(mapStateToProps, mapDispatchToProps, mergeProps, options = {}) { - const { pure = true, withRef = false, wrapMapState = false, wrapMapDispatch = false } = options + const { pure = true, withRef = false, mapStateThunk = false, mapDispatchThunk = false } = options const shouldSubscribe = Boolean(mapStateToProps) const finalMapStateToProps = mapStateToProps || defaultMapStateToProps const finalMapDispatchToProps = isPlainObject(mapDispatchToProps) ? wrapActionCreators(mapDispatchToProps) : mapDispatchToProps || defaultMapDispatchToProps const finalMergeProps = mergeProps || defaultMergeProps - const shouldUpdateStateProps = wrapMapState || finalMapStateToProps.length > 1 - const shouldUpdateDispatchProps = wrapMapDispatch || finalMapDispatchToProps.length > 1 + const shouldUpdateStateProps = mapStateThunk || finalMapStateToProps.length > 1 + const shouldUpdateDispatchProps = mapDispatchThunk || finalMapDispatchToProps.length > 1 // Helps track hot reloading. const version = nextVersion++ - function computeStateProps(connectMapStateToProps, store, props) { + function computeStateProps(mapState, store, props) { const state = store.getState() const stateProps = shouldUpdateStateProps ? - connectMapStateToProps(state, props) : - connectMapStateToProps(state) + mapState(state, props) : + mapState(state) invariant( isPlainObject(stateProps), @@ -49,11 +49,11 @@ export default function connect(mapStateToProps, mapDispatchToProps, mergeProps, return stateProps } - function computeDispatchProps(connectMapDispatchToProps, store, props) { + function computeDispatchProps(mapDispatch, store, props) { const { dispatch } = store const dispatchProps = shouldUpdateDispatchProps ? - connectMapDispatchToProps(dispatch, props) : - connectMapDispatchToProps(dispatch) + mapDispatch(dispatch, props) : + mapDispatch(dispatch) invariant( isPlainObject(dispatchProps), @@ -116,18 +116,18 @@ export default function connect(mapStateToProps, mapDispatchToProps, mergeProps, `or explicitly pass "store" as a prop to "${this.constructor.displayName}".` ) - this.wrapMapToProps() - this.stateProps = computeStateProps(this.connectMapStateToProps, this.store, props) - this.dispatchProps = computeDispatchProps(this.connectMapDispatchToProps, this.store, props) + this.updateThunks() + this.stateProps = computeStateProps(this.finalMapStateToProps, this.store, props) + this.dispatchProps = computeDispatchProps(this.finalMapDispatchToProps, this.store, props) this.state = { storeState: null } this.updateState() } - wrapMapToProps() { - this.connectMapStateToProps = wrapMapState - ? wrapMapState(finalMapStateToProps) : finalMapStateToProps - this.connectMapDispatchToProps = wrapMapDispatch - ? wrapMapDispatch(finalMapDispatchToProps) : finalMapDispatchToProps + updateThunks() { + this.finalMapStateToProps = mapStateThunk + ? finalMapStateToProps() : finalMapStateToProps + this.finalMapDispatchToProps = mapDispatchThunk + ? finalMapDispatchToProps() : finalMapDispatchToProps } computeNextState(props = this.props) { @@ -139,7 +139,7 @@ export default function connect(mapStateToProps, mapDispatchToProps, mergeProps, } updateStateProps(props = this.props) { - const nextStateProps = computeStateProps(this.connectMapStateToProps, this.store, props) + const nextStateProps = computeStateProps(this.finalMapStateToProps, this.store, props) if (shallowEqual(nextStateProps, this.stateProps)) { return false } @@ -149,7 +149,7 @@ export default function connect(mapStateToProps, mapDispatchToProps, mergeProps, } updateDispatchProps(props = this.props) { - const nextDispatchProps = computeDispatchProps(this.connectMapDispatchToProps, this.store, props) + const nextDispatchProps = computeDispatchProps(this.finalMapDispatchToProps, this.store, props) if (shallowEqual(nextDispatchProps, this.dispatchProps)) { return false } @@ -234,7 +234,7 @@ export default function connect(mapStateToProps, mapDispatchToProps, mergeProps, this.version = version // Update the state and bindings. - this.wrapMapToProps() + this.updateThunks() this.trySubscribe() this.updateStateProps() this.updateDispatchProps() diff --git a/test/components/connect.spec.js b/test/components/connect.spec.js index 09563352b..1c0ba9b35 100644 --- a/test/components/connect.spec.js +++ b/test/components/connect.spec.js @@ -1371,7 +1371,7 @@ describe('React', () => { expect(renderCalls).toBe(1) }) - it('accepts a wrapMapState option for configuring mapStateToProps for the component', () => { + it('accepts mapStateThunk for a unique mapStateToProps per component', () => { const store = createStore(() => ({ prefix: 'name: ' @@ -1399,7 +1399,7 @@ describe('React', () => { return { value: props.prefix + state.name } } - @connect(() => memoize(computeValue), null, null, { wrapMapState: (fn) => fn() }) + @connect(() => memoize(computeValue), null, null, { mapStateThunk: true }) class Container extends Component { componentDidMount() { this.forceUpdate() From 813b629474c1bd246ba04a34ceaa17cd5d0860e1 Mon Sep 17 00:00:00 2001 From: Tim Griesser Date: Sun, 8 Nov 2015 14:13:33 -0500 Subject: [PATCH 3/4] mapStateThunk -> stateThunk --- src/components/connect.js | 10 +++++----- test/components/connect.spec.js | 2 +- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/components/connect.js b/src/components/connect.js index acdf26a2d..197764186 100644 --- a/src/components/connect.js +++ b/src/components/connect.js @@ -22,15 +22,15 @@ function getDisplayName(WrappedComponent) { let nextVersion = 0 export default function connect(mapStateToProps, mapDispatchToProps, mergeProps, options = {}) { - const { pure = true, withRef = false, mapStateThunk = false, mapDispatchThunk = false } = options + const { pure = true, withRef = false, stateThunk = false, dispatchThunk = false } = options const shouldSubscribe = Boolean(mapStateToProps) const finalMapStateToProps = mapStateToProps || defaultMapStateToProps const finalMapDispatchToProps = isPlainObject(mapDispatchToProps) ? wrapActionCreators(mapDispatchToProps) : mapDispatchToProps || defaultMapDispatchToProps const finalMergeProps = mergeProps || defaultMergeProps - const shouldUpdateStateProps = mapStateThunk || finalMapStateToProps.length > 1 - const shouldUpdateDispatchProps = mapDispatchThunk || finalMapDispatchToProps.length > 1 + const shouldUpdateStateProps = stateThunk || finalMapStateToProps.length > 1 + const shouldUpdateDispatchProps = dispatchThunk || finalMapDispatchToProps.length > 1 // Helps track hot reloading. const version = nextVersion++ @@ -124,9 +124,9 @@ export default function connect(mapStateToProps, mapDispatchToProps, mergeProps, } updateThunks() { - this.finalMapStateToProps = mapStateThunk + this.finalMapStateToProps = stateThunk ? finalMapStateToProps() : finalMapStateToProps - this.finalMapDispatchToProps = mapDispatchThunk + this.finalMapDispatchToProps = dispatchThunk ? finalMapDispatchToProps() : finalMapDispatchToProps } diff --git a/test/components/connect.spec.js b/test/components/connect.spec.js index 1c0ba9b35..fccce03ba 100644 --- a/test/components/connect.spec.js +++ b/test/components/connect.spec.js @@ -1399,7 +1399,7 @@ describe('React', () => { return { value: props.prefix + state.name } } - @connect(() => memoize(computeValue), null, null, { mapStateThunk: true }) + @connect(() => memoize(computeValue), null, null, { stateThunk: true }) class Container extends Component { componentDidMount() { this.forceUpdate() From 7dad6c52eea92e325d28ae26630577ac122e47d0 Mon Sep 17 00:00:00 2001 From: Tim Griesser Date: Mon, 9 Nov 2015 07:20:24 -0500 Subject: [PATCH 4/4] assignMapFunctions, for configurable connect --- src/components/connect.js | 18 ++++++++---------- test/components/connect.spec.js | 14 ++++++++++++-- 2 files changed, 20 insertions(+), 12 deletions(-) diff --git a/src/components/connect.js b/src/components/connect.js index 197764186..ef30c62f6 100644 --- a/src/components/connect.js +++ b/src/components/connect.js @@ -22,15 +22,15 @@ function getDisplayName(WrappedComponent) { let nextVersion = 0 export default function connect(mapStateToProps, mapDispatchToProps, mergeProps, options = {}) { - const { pure = true, withRef = false, stateThunk = false, dispatchThunk = false } = options + const { pure = true, withRef = false } = options const shouldSubscribe = Boolean(mapStateToProps) const finalMapStateToProps = mapStateToProps || defaultMapStateToProps const finalMapDispatchToProps = isPlainObject(mapDispatchToProps) ? wrapActionCreators(mapDispatchToProps) : mapDispatchToProps || defaultMapDispatchToProps const finalMergeProps = mergeProps || defaultMergeProps - const shouldUpdateStateProps = stateThunk || finalMapStateToProps.length > 1 - const shouldUpdateDispatchProps = dispatchThunk || finalMapDispatchToProps.length > 1 + const shouldUpdateStateProps = finalMapStateToProps.length > 1 + const shouldUpdateDispatchProps = finalMapDispatchToProps.length > 1 // Helps track hot reloading. const version = nextVersion++ @@ -116,18 +116,16 @@ export default function connect(mapStateToProps, mapDispatchToProps, mergeProps, `or explicitly pass "store" as a prop to "${this.constructor.displayName}".` ) - this.updateThunks() + this.assignMapFunctions() this.stateProps = computeStateProps(this.finalMapStateToProps, this.store, props) this.dispatchProps = computeDispatchProps(this.finalMapDispatchToProps, this.store, props) this.state = { storeState: null } this.updateState() } - updateThunks() { - this.finalMapStateToProps = stateThunk - ? finalMapStateToProps() : finalMapStateToProps - this.finalMapDispatchToProps = dispatchThunk - ? finalMapDispatchToProps() : finalMapDispatchToProps + assignMapFunctions() { + this.finalMapStateToProps = finalMapStateToProps + this.finalMapDispatchToProps = finalMapDispatchToProps } computeNextState(props = this.props) { @@ -234,7 +232,7 @@ export default function connect(mapStateToProps, mapDispatchToProps, mergeProps, this.version = version // Update the state and bindings. - this.updateThunks() + this.assignMapFunctions() this.trySubscribe() this.updateStateProps() this.updateDispatchProps() diff --git a/test/components/connect.spec.js b/test/components/connect.spec.js index fccce03ba..5e41c932f 100644 --- a/test/components/connect.spec.js +++ b/test/components/connect.spec.js @@ -1371,7 +1371,7 @@ describe('React', () => { expect(renderCalls).toBe(1) }) - it('accepts mapStateThunk for a unique mapStateToProps per component', () => { + it('defines finalMapStateToProps on the component', () => { const store = createStore(() => ({ prefix: 'name: ' @@ -1395,11 +1395,21 @@ describe('React', () => { } } + function memoizer(WrappedComponent) { + let assignMapFunctions = WrappedComponent.prototype.assignMapFunctions + WrappedComponent.prototype.assignMapFunctions = function () { + assignMapFunctions.call(this) + this.finalMapStateToProps = memoize(this.finalMapStateToProps) + } + return WrappedComponent + } + function computeValue(state, props) { return { value: props.prefix + state.name } } - @connect(() => memoize(computeValue), null, null, { stateThunk: true }) + @memoizer + @connect(computeValue, null, null, { stateThunk: true }) class Container extends Component { componentDidMount() { this.forceUpdate()