From 7bf2a4ff52fb02a15cea8e1b4bbd332c4d67b986 Mon Sep 17 00:00:00 2001 From: sebastien Date: Tue, 20 Jun 2017 22:35:25 +0200 Subject: [PATCH 1/3] New PR for mapStateToProps shorthand syntax (replaces https://github.com/reactjs/react-redux/pull/323) --- docs/api.md | 19 +++++++++++++ src/connect/mapStateToProps.js | 9 ++++++- src/connect/wrapMapToProps.js | 30 +++++++++++++++++++++ test/components/connect.spec.js | 48 +++++++++++++++++++++++++++++++++ 4 files changed, 105 insertions(+), 1 deletion(-) diff --git a/docs/api.md b/docs/api.md index ac0248b0b..2e02c7b13 100644 --- a/docs/api.md +++ b/docs/api.md @@ -55,6 +55,8 @@ It does not modify the component class passed to it; instead, it *returns* a new * [`mapStateToProps(state, [ownProps]): stateProps`] \(*Function*): If this argument is specified, the new component will subscribe to Redux store updates. This means that any time the store is updated, `mapStateToProps` will be called. The results of `mapStateToProps` must be a plain object, which will be merged into the component’s props. If you don't want to subscribe to store updates, pass `null` or `undefined` in place of `mapStateToProps`. If your `mapStateToProps` function is declared as taking two parameters, it will be called with the store state as the first parameter and the props passed to the connected component as the second parameter, and will also be re-invoked whenever the connected component receives new props as determined by shallow equality comparisons. (The second parameter is normally referred to as `ownProps` by convention.) + + >Note: if your mapStateToProps do not need to access component properties, you can use a shorthand syntax by passing an object whose values are "selectors" (See [reselect](https://github.com/reactjs/reselect)) >Note: in advanced scenarios where you need more control over the rendering performance, `mapStateToProps()` can also return a function. In this case, *that* function will be used as `mapStateToProps()` for a particular component instance. This allows you to do per-instance memoization. You can refer to [#279](https://github.com/reactjs/react-redux/pull/279) and the tests it adds for more details. Most apps never need this. @@ -274,6 +276,23 @@ function mapDispatchToProps(dispatch) { export default connect(mapStateToProps, mapDispatchToProps)(TodoApp) ``` +##### Inject `todos` with shorthand syntax, and all todoActionCreators and counterActionCreators directly as props + +```js +import * as todoActionCreators from './todoActionCreators' +import * as counterActionCreators from './counterActionCreators' +import { bindActionCreators } from 'redux' + + +const todosSelector = state => state.todos + +function mapDispatchToProps(dispatch) { + return bindActionCreators(Object.assign({}, todoActionCreators, counterActionCreators), dispatch) +} + +export default connect({todos: todosSelector}, mapDispatchToProps)(TodoApp) +``` + ##### Inject `todos` of a specific user depending on props ```js diff --git a/src/connect/mapStateToProps.js b/src/connect/mapStateToProps.js index 039291b0a..51b9b72b3 100644 --- a/src/connect/mapStateToProps.js +++ b/src/connect/mapStateToProps.js @@ -1,4 +1,4 @@ -import { wrapMapToPropsConstant, wrapMapToPropsFunc } from './wrapMapToProps' +import { wrapMapToPropsConstant, wrapMapToPropsFunc, wrapMapStateObject } from './wrapMapToProps' export function whenMapStateToPropsIsFunction(mapStateToProps) { return (typeof mapStateToProps === 'function') @@ -6,6 +6,12 @@ export function whenMapStateToPropsIsFunction(mapStateToProps) { : undefined } +export function whenMapStateToPropsIsObject(mapStateToProps) { + return (typeof mapStateToProps === 'object') + ? wrapMapStateObject(mapStateToProps, 'mapStateToProps') + : undefined +} + export function whenMapStateToPropsIsMissing(mapStateToProps) { return (!mapStateToProps) ? wrapMapToPropsConstant(() => ({})) @@ -14,5 +20,6 @@ export function whenMapStateToPropsIsMissing(mapStateToProps) { export default [ whenMapStateToPropsIsFunction, + whenMapStateToPropsIsObject, whenMapStateToPropsIsMissing ] diff --git a/src/connect/wrapMapToProps.js b/src/connect/wrapMapToProps.js index 6b39c7961..33cda6843 100644 --- a/src/connect/wrapMapToProps.js +++ b/src/connect/wrapMapToProps.js @@ -1,4 +1,5 @@ import verifyPlainObject from '../utils/verifyPlainObject' +import invariant from 'invariant' export function wrapMapToPropsConstant(getConstant) { return function initConstantSelector(dispatch, options) { @@ -66,3 +67,32 @@ export function wrapMapToPropsFunc(mapToProps, methodName) { return proxy } } + + +function mapValues(obj, fn) { + return Object.keys(obj).reduce((result, key) => { + result[key] = fn(obj[key], key) + return result + }, {}) +} + +export function wrapMapStateObject(mapStateToProps, methodName) { + + const needsProps = Object.keys(mapStateToProps) + .reduce((useProps, key) => { + const type = typeof mapStateToProps[key] + invariant( + type === 'function', + 'mapStateToProps object key %s expected to be a function, instead saw %s', + key, + type + ) + return useProps || mapStateToProps[key].length !== 1 + }, false) + + const mapToPropsFn = needsProps + ? (state, props) => mapValues(mapStateToProps, fn => fn(state, props)) + : state => mapValues(mapStateToProps, fn => fn(state)) + + return wrapMapToPropsFunc(mapToPropsFn, methodName) +} diff --git a/test/components/connect.spec.js b/test/components/connect.spec.js index bdd4725e2..9d27a5b94 100644 --- a/test/components/connect.spec.js +++ b/test/components/connect.spec.js @@ -108,6 +108,54 @@ describe('React', () => { TestUtils.findRenderedComponentWithType(container, Container) ).toNotThrow() }) + + it('should pass state to given component, with shorthand syntax', () => { + const store = createStore(() => ({ + foo: 'bar', + baz: 42, + hello: 'world' + })) + + @connect({ + foo: state => state.foo, + baz: state => state.baz + }) + class Container extends Component { + render() { + return + } + } + + const container = TestUtils.renderIntoDocument( + + + + ) + const stub = TestUtils.findRenderedComponentWithType(container, Passthrough) + expect(stub.props.pass).toEqual('through') + expect(stub.props.foo).toEqual('bar') + expect(stub.props.baz).toEqual(42) + expect(stub.props.hello).toEqual(undefined) + expect(() => + TestUtils.findRenderedComponentWithType(container, Container) + ).toNotThrow() + }) + + it('should throw error if connect is called with shorthand syntax and one object value is not a function', () => { + expect(() => + @connect({ + foo: state => state.foo, + baz: 'badValue' + }) + class Container extends Component { + render() { + return
+ } + } + ).toThrow( + /mapStateToProps object key baz expected to be a function, instead saw string/ + ) + }) it('should subscribe class components to the store changes', () => { const store = createStore(stringBuilder) From 06e882cf0399b12d1d0b1ea95b48b92f479a9beb Mon Sep 17 00:00:00 2001 From: sebastien Date: Wed, 21 Jun 2017 11:55:00 +0200 Subject: [PATCH 2/3] Shorthand syntax: add factories support --- docs/api.md | 9 ++++++++- src/connect/wrapMapToProps.js | 14 ++++++++++---- test/components/connect.spec.js | 31 +++++++++++++++++++++++++++++++ 3 files changed, 49 insertions(+), 5 deletions(-) diff --git a/docs/api.md b/docs/api.md index 2e02c7b13..a1e040f18 100644 --- a/docs/api.md +++ b/docs/api.md @@ -56,7 +56,14 @@ It does not modify the component class passed to it; instead, it *returns* a new If your `mapStateToProps` function is declared as taking two parameters, it will be called with the store state as the first parameter and the props passed to the connected component as the second parameter, and will also be re-invoked whenever the connected component receives new props as determined by shallow equality comparisons. (The second parameter is normally referred to as `ownProps` by convention.) - >Note: if your mapStateToProps do not need to access component properties, you can use a shorthand syntax by passing an object whose values are "selectors" (See [reselect](https://github.com/reactjs/reselect)) + >Note: mapStateToProps can also be passed as an user-friendly object syntax. Keys are prop names and values are selectors (or factories). This is quite similar to [createStructuredSelector](https://github.com/reactjs/reselect#createstructuredselectorinputselectors-selectorcreator--createselector) of [reselect](https://github.com/reactjs/reselect) + +```javascript +Component = connect({ + deviceOrientation: state => state.deviceOrientation, + user: (intialState,initialProps) => (state) => state.users[initialProps.userId], +})(Component) +``` >Note: in advanced scenarios where you need more control over the rendering performance, `mapStateToProps()` can also return a function. In this case, *that* function will be used as `mapStateToProps()` for a particular component instance. This allows you to do per-instance memoization. You can refer to [#279](https://github.com/reactjs/react-redux/pull/279) and the tests it adds for more details. Most apps never need this. diff --git a/src/connect/wrapMapToProps.js b/src/connect/wrapMapToProps.js index 33cda6843..7b14785b5 100644 --- a/src/connect/wrapMapToProps.js +++ b/src/connect/wrapMapToProps.js @@ -90,9 +90,15 @@ export function wrapMapStateObject(mapStateToProps, methodName) { return useProps || mapStateToProps[key].length !== 1 }, false) - const mapToPropsFn = needsProps - ? (state, props) => mapValues(mapStateToProps, fn => fn(state, props)) - : state => mapValues(mapStateToProps, fn => fn(state)) + const selectorsMapWrapped = mapValues(mapStateToProps, fn => wrapMapToPropsFunc(fn,methodName)) + + return (...args) => { + + const selectorsMap = mapValues(selectorsMapWrapped,fn => fn(...args)) + + return needsProps + ? (state, props) => mapValues(selectorsMap, fn => fn(state, props)) + : state => mapValues(selectorsMap, fn => fn(state)) + } - return wrapMapToPropsFunc(mapToPropsFn, methodName) } diff --git a/test/components/connect.spec.js b/test/components/connect.spec.js index 9d27a5b94..cc77484eb 100644 --- a/test/components/connect.spec.js +++ b/test/components/connect.spec.js @@ -140,6 +140,37 @@ describe('React', () => { TestUtils.findRenderedComponentWithType(container, Container) ).toNotThrow() }) + + it('should pass state to given component, with shorthand factory syntax', () => { + const store = createStore(() => ({ + baz: 'baz', + })) + + @connect({ + bazNormal: state => state.baz, + bazFactory: (intialState) => (state) => intialState.baz + state.baz, + bazFactoryWithProps: (intialState, initialProps) => (state, props) => intialState.baz + initialProps.pass + state.baz + props.pass + }) + class Container extends Component { + render() { + return + } + } + + const container = TestUtils.renderIntoDocument( + + + + ) + const stub = TestUtils.findRenderedComponentWithType(container, Passthrough) + expect(stub.props.pass).toEqual('through') + expect(stub.props.bazNormal).toEqual('baz') + expect(stub.props.bazFactory).toEqual("bazbaz") + expect(stub.props.bazFactoryWithProps).toEqual('bazthroughbazthrough') + expect(() => + TestUtils.findRenderedComponentWithType(container, Container) + ).toNotThrow() + }) it('should throw error if connect is called with shorthand syntax and one object value is not a function', () => { expect(() => From b6570c2ad6c66ba5a0bd645285365c3946f69f77 Mon Sep 17 00:00:00 2001 From: sebastien Date: Wed, 21 Jun 2017 12:28:26 +0200 Subject: [PATCH 3/3] shorthand syntax: fix warnings --- src/connect/wrapMapToProps.js | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/src/connect/wrapMapToProps.js b/src/connect/wrapMapToProps.js index 7b14785b5..eddc7aa92 100644 --- a/src/connect/wrapMapToProps.js +++ b/src/connect/wrapMapToProps.js @@ -36,7 +36,7 @@ export function getDependsOnOwnProps(mapToProps) { // * On first call, verifies the first result is a plain object, in order to warn // the developer that their mapToProps function is not returning a valid result. // -export function wrapMapToPropsFunc(mapToProps, methodName) { +export function wrapMapToPropsFunc(mapToProps, methodName, bypassVerifyPlainObject) { return function initProxySelector(dispatch, { displayName }) { const proxy = function mapToPropsProxy(stateOrDispatch, ownProps) { return proxy.dependsOnOwnProps @@ -58,7 +58,7 @@ export function wrapMapToPropsFunc(mapToProps, methodName) { props = proxy(stateOrDispatch, ownProps) } - if (process.env.NODE_ENV !== 'production') + if (process.env.NODE_ENV !== 'production' && !bypassVerifyPlainObject) verifyPlainObject(props, displayName, methodName) return props @@ -90,15 +90,19 @@ export function wrapMapStateObject(mapStateToProps, methodName) { return useProps || mapStateToProps[key].length !== 1 }, false) - const selectorsMapWrapped = mapValues(mapStateToProps, fn => wrapMapToPropsFunc(fn,methodName)) + const selectorsMapWrapped = mapValues(mapStateToProps, fn => wrapMapToPropsFunc(fn,methodName,true)) return (...args) => { const selectorsMap = mapValues(selectorsMapWrapped,fn => fn(...args)) - return needsProps + const mapStateToProps = needsProps ? (state, props) => mapValues(selectorsMap, fn => fn(state, props)) : state => mapValues(selectorsMap, fn => fn(state)) + + mapStateToProps.dependsOnOwnProps = needsProps + + return mapStateToProps; } }