From f6ef06eef8bd0b71230cca3e04e4aa53cc482896 Mon Sep 17 00:00:00 2001 From: gnoff Date: Thu, 30 Jul 2015 01:02:44 -0700 Subject: [PATCH 01/10] WIP connectWrapper that will eventually replace connectDecorator and Connector --- src/components/createAll.js | 8 +- src/components/createConnectWrapper.js | 140 ++++++++++++++++++++ src/index.js | 2 +- src/native.js | 2 +- src/utils/wrapActionCreators.js | 5 + test/components/Connector.spec.js | 16 +-- test/components/Provider.spec.js | 4 +- test/components/connect.spec.js | 4 +- test/components/connectDecorator.spec.js | 156 +++++++++++++++++++++++ test/components/provide.spec.js | 4 +- 10 files changed, 323 insertions(+), 18 deletions(-) create mode 100644 src/components/createConnectWrapper.js create mode 100644 src/utils/wrapActionCreators.js create mode 100644 test/components/connectDecorator.spec.js diff --git a/src/components/createAll.js b/src/components/createAll.js index bd6383ccc..43cda3bfe 100644 --- a/src/components/createAll.js +++ b/src/components/createAll.js @@ -3,6 +3,7 @@ import createProvideDecorator from './createProvideDecorator'; import createConnector from './createConnector'; import createConnectDecorator from './createConnectDecorator'; +import createConnectWrapper from './createConnectWrapper'; export default function createAll(React) { // Wrapper components @@ -11,7 +12,10 @@ export default function createAll(React) { // Higher-order components (decorators) const provide = createProvideDecorator(React, Provider); - const connect = createConnectDecorator(React, Connector); + const connectDecorate = createConnectDecorator(React, Connector); + const connect = createConnectWrapper(React); - return { Provider, Connector, provide, connect }; + + + return { Provider, Connector, provide, connectDecorate, connect }; } diff --git a/src/components/createConnectWrapper.js b/src/components/createConnectWrapper.js new file mode 100644 index 000000000..80d6d6f45 --- /dev/null +++ b/src/components/createConnectWrapper.js @@ -0,0 +1,140 @@ +import createStoreShape from '../utils/createStoreShape'; +import getDisplayName from '../utils/getDisplayName'; +import shallowEqual from '../utils/shallowEqual'; +import isPlainObject from '../utils/isPlainObject'; +import wrapActionCreators from '../utils/wrapActionCreators'; +import invariant from 'invariant'; + +const emptySelector = () => ({}); + +const emptyBinder = () => ({}); + +const identityMerge = (slice, actionsCreators, props) => ({...slice, ...actionsCreators, ...props}); + + +export default function createConnectWrapper(React) { + const { Component, PropTypes } = React; + const storeShape = createStoreShape(PropTypes); + + return function connect(select, bindActionCreators, merge) { + + const subscribing = select ? true : false; + + select = select || emptySelector; + + bindActionCreators = bindActionCreators || emptyBinder; + + if (isPlainObject(bindActionCreators)) { + bindActionCreators = wrapActionCreators(bindActionCreators); + } + + merge = merge || identityMerge; + + return DecoratedComponent => class ConnectWrapper extends Component { + static displayName = `ConnectWrapper(${getDisplayName(DecoratedComponent)})`; + static DecoratedComponent = DecoratedComponent; + + static contextTypes = { + store: storeShape.isRequired + }; + + componentWillReceiveProps(nextProps) { + console.log('recieving props', this.props, nextProps) + } + + shouldComponentUpdate(nextProps, nextState) { + console.log('shallowEqual of props', shallowEqual(this.props, nextProps), this.props, nextProps) + return (this.subscribed && !this.isSliceEqual(this.state.slice, nextState.slice)) || + !shallowEqual(this.props, nextProps); + } + + isSliceEqual(slice, nextSlice) { + const isRefEqual = slice === nextSlice; + if (isRefEqual) { + return true; + } else if (typeof slice !== 'object' || typeof nextSlice !== 'object') { + return isRefEqual; + } + return shallowEqual(slice, nextSlice); + } + + constructor(props, context) { + super(props, context); + this.state = { + ...this.selectState(props, context), + ...this.bindActionCreators(context), + }; + } + + componentWillMount() { + console.log('will mount', this.props) + } + + componentDidMount() { + console.log('mounted', this.props) + if (subscribing) { + this.subscribed = true; + this.unsubscribe = this.context.store.subscribe(::this.handleChange); + } + } + + componentWillUnmount() { + if (subscribing) { + this.unsubscribe(); + } + } + + handleChange(props = this.props) { + const nextState = this.selectState(props, this.context); + if (!this.isSliceEqual(this.state.slice, nextState.slice)) { + this.setState(nextState); + } + } + + selectState(props = this.props, context = this.context) { + const state = context.store.getState(); + const slice = select(state); + + invariant( + isPlainObject(slice), + 'The return value of `select` prop must be an object. Instead received %s.', + slice + ); + + return { slice }; + } + + bindActionCreators(context = this.context) { + const { dispatch } = context.store; + const actionCreators = bindActionCreators(dispatch); + + invariant( + isPlainObject(actionCreators), + 'The return value of `bindActionCreators` prop must be an object. Instead received %s.', + actionCreators + ); + + return { actionCreators }; + } + + merge(props = this.props, state = this.state) { + const { slice, actionCreators } = state; + const merged = merge(slice, actionCreators, props); + + invariant( + isPlainObject(merged), + 'The return value of `merge` prop must be an object. Instead received %s.', + merged + ); + + console.log('merging with ', merged) + + return merged; + } + + render() { + return ; + } + }; + }; +} diff --git a/src/index.js b/src/index.js index a2058d695..3ac9aeeb7 100644 --- a/src/index.js +++ b/src/index.js @@ -1,4 +1,4 @@ import React from 'react'; import createAll from './components/createAll'; -export const { Provider, Connector, provide, connect } = createAll(React); +export const { Provider, Connector, provide, connectDecorate, connect } = createAll(React); diff --git a/src/native.js b/src/native.js index c6fc5363e..b7e1fe8d1 100644 --- a/src/native.js +++ b/src/native.js @@ -1,4 +1,4 @@ import React from 'react-native'; import createAll from './components/createAll'; -export const { Provider, Connector, provide, connect } = createAll(React); +export const { Provider, Connector, provide, connectDecorate, connect } = createAll(React); diff --git a/src/utils/wrapActionCreators.js b/src/utils/wrapActionCreators.js new file mode 100644 index 000000000..4d186f7b5 --- /dev/null +++ b/src/utils/wrapActionCreators.js @@ -0,0 +1,5 @@ +import { bindActionCreators } from 'redux' + +export default function wrapActionCreators (actionCreators) { + return dispatch => bindActionCreators(actionCreators, dispatch); +} \ No newline at end of file diff --git a/test/components/Connector.spec.js b/test/components/Connector.spec.js index bb4cef68b..6d386de82 100644 --- a/test/components/Connector.spec.js +++ b/test/components/Connector.spec.js @@ -1,7 +1,7 @@ import expect from 'expect'; import jsdomReact from './jsdomReact'; import React, { PropTypes, Component } from 'react/addons'; -import { createStore } from 'redux'; +import { createStore, combineReducers } from 'redux'; import { Connector } from '../../src/index'; const { TestUtils } = React.addons; @@ -32,7 +32,7 @@ describe('React', () => { } it('should receive the store in the context', () => { - const store = createStore({}); + const store = createStore(() => ({})); const tree = TestUtils.renderIntoDocument( @@ -74,7 +74,7 @@ describe('React', () => { const subscribe = store.subscribe; // Keep track of unsubscribe by wrapping subscribe() - const spy = expect.createSpy(() => {}); + const spy = expect.createSpy(() => ({})); store.subscribe = (listener) => { const unsubscribe = subscribe(listener); return () => { @@ -101,7 +101,7 @@ describe('React', () => { it('should shallowly compare the selected state to prevent unnecessary updates', () => { const store = createStore(stringBuilder); - const spy = expect.createSpy(() => {}); + const spy = expect.createSpy(() => ({})); function render({ string }) { spy(); return
; @@ -129,10 +129,10 @@ describe('React', () => { }); it('should recompute the state slice when the select prop changes', () => { - const store = createStore({ + const store = createStore(combineReducers({ a: () => 42, b: () => 72 - }); + })); function selectA(state) { return { result: state.a }; @@ -174,7 +174,7 @@ describe('React', () => { }); it('should pass dispatch() to the child function', () => { - const store = createStore({}); + const store = createStore(() => ({})); const tree = TestUtils.renderIntoDocument( @@ -191,7 +191,7 @@ describe('React', () => { }); it('should throw an error if select returns anything but a plain object', () => { - const store = createStore({}); + const store = createStore(() => ({})); expect(() => { TestUtils.renderIntoDocument( diff --git a/test/components/Provider.spec.js b/test/components/Provider.spec.js index d0daf08f1..a85f1b2b7 100644 --- a/test/components/Provider.spec.js +++ b/test/components/Provider.spec.js @@ -21,7 +21,7 @@ describe('React', () => { } it('should add the store to the child context', () => { - const store = createStore({}); + const store = createStore(() => ({})); const tree = TestUtils.renderIntoDocument( @@ -36,7 +36,7 @@ describe('React', () => { it('should replace just the reducer when receiving a new store in props', () => { const store1 = createStore((state = 10) => state + 1); const store2 = createStore((state = 10) => state * 2); - const spy = expect.createSpy(() => {}); + const spy = expect.createSpy(() => ({})); class ProviderContainer extends Component { state = { store: store1 }; diff --git a/test/components/connect.spec.js b/test/components/connect.spec.js index 743881097..4cdcfd452 100644 --- a/test/components/connect.spec.js +++ b/test/components/connect.spec.js @@ -2,7 +2,7 @@ import expect from 'expect'; import jsdomReact from './jsdomReact'; import React, { PropTypes, Component } from 'react/addons'; import { createStore } from 'redux'; -import { connect, Connector } from '../../src/index'; +import { connect } from '../../src/index'; const { TestUtils } = React.addons; @@ -46,7 +46,7 @@ describe('React', () => { expect(div.props.pass).toEqual('through'); expect(div.props.foo).toEqual('bar'); expect(() => - TestUtils.findRenderedComponentWithType(container, Connector) + TestUtils.findRenderedComponentWithType(container, Container) ).toNotThrow(); }); diff --git a/test/components/connectDecorator.spec.js b/test/components/connectDecorator.spec.js new file mode 100644 index 000000000..47b7a1063 --- /dev/null +++ b/test/components/connectDecorator.spec.js @@ -0,0 +1,156 @@ +import expect from 'expect'; +import jsdomReact from './jsdomReact'; +import React, { PropTypes, Component } from 'react/addons'; +import { createStore } from 'redux'; +import { connectDecorate, Connector } from '../../src/index'; + +const connect = connectDecorate; + +const { TestUtils } = React.addons; + +describe('React', () => { + describe('connectDecorate', () => { + jsdomReact(); + + // Mock minimal Provider interface + class Provider extends Component { + static childContextTypes = { + store: PropTypes.object.isRequired + } + + getChildContext() { + return { store: this.props.store }; + } + + render() { + return this.props.children(); + } + } + + it('should wrap the component into Provider', () => { + const store = createStore(() => ({ + foo: 'bar' + })); + + @connect(state => state) + class Container extends Component { + render() { + return
; + } + } + + const container = TestUtils.renderIntoDocument( + + {() => } + + ); + const div = TestUtils.findRenderedDOMComponentWithTag(container, 'div'); + expect(div.props.pass).toEqual('through'); + expect(div.props.foo).toEqual('bar'); + expect(() => + TestUtils.findRenderedComponentWithType(container, Connector) + ).toNotThrow(); + }); + + it('should handle additional prop changes in addition to slice', () => { + const store = createStore(() => ({ + foo: 'bar' + })); + + @connect(state => state) + class ConnectContainer extends Component { + render() { + return ( +
+ ); + } + } + + class Container extends Component { + constructor() { + super(); + this.state = { + bar: { + baz: '' + } + }; + } + componentDidMount() { + + // Simulate deep object mutation + this.state.bar.baz = 'through'; + this.setState({ + bar: this.state.bar + }); + } + render() { + return ( + + {() => } + + ); + } + } + + const container = TestUtils.renderIntoDocument(); + const div = TestUtils.findRenderedDOMComponentWithTag(container, 'div'); + expect(div.props.foo).toEqual('bar'); + expect(div.props.pass).toEqual('through'); + }); + + it('should pass the only argument as the select prop down', () => { + const store = createStore(() => ({ + foo: 'baz', + bar: 'baz' + })); + + function select({ foo }) { + return { foo }; + } + + @connect(select) + class Container extends Component { + render() { + return
; + } + } + + const container = TestUtils.renderIntoDocument( + + {() => } + + ); + const connector = TestUtils.findRenderedComponentWithType(container, Connector); + expect(connector.props.select({ + foo: 5, + bar: 7 + })).toEqual({ + foo: 5 + }); + }); + + it('should set the displayName correctly', () => { + @connect(state => state) + class Container extends Component { + render() { + return
; + } + } + + expect(Container.displayName).toBe('Connector(Container)'); + }); + + it('should expose the wrapped component as DecoratedComponent', () => { + class Container extends Component { + render() { + return
; + } + } + + const decorator = connect(state => state); + const decorated = decorator(Container); + + expect(decorated.DecoratedComponent).toBe(Container); + }); + }); +}); diff --git a/test/components/provide.spec.js b/test/components/provide.spec.js index add035f15..ea328d9a4 100644 --- a/test/components/provide.spec.js +++ b/test/components/provide.spec.js @@ -21,7 +21,7 @@ describe('React', () => { } it('should wrap the component into Provider', () => { - const store = createStore({}); + const store = createStore(() => ({})); @provide(store) class Container extends Component { @@ -42,7 +42,7 @@ describe('React', () => { }); it('sets the displayName correctly', () => { - @provide(createStore({})) + @provide(createStore(() => ({}))) class Container extends Component { render() { return
; From 063777a5d3c79db01c13838b5310d4c978268a0f Mon Sep 17 00:00:00 2001 From: gnoff Date: Thu, 30 Jul 2015 23:22:45 -0700 Subject: [PATCH 02/10] intermediate commit. old decorator and connector are still present --- src/components/createAll.js | 10 +- src/components/createConnectDecorator.js | 121 +++++- src/components/createConnectDecoratorOld.js | 25 ++ src/components/createConnectWrapper.js | 140 ------- src/index.js | 2 +- src/native.js | 2 +- src/utils/wrapActionCreators.js | 6 +- test/components/connect.spec.js | 394 +++++++++++++++++- ...or.spec.js => connectDecoratorOld.spec.js} | 4 +- test/utils/wrapActionCreators.js | 31 ++ 10 files changed, 555 insertions(+), 180 deletions(-) create mode 100644 src/components/createConnectDecoratorOld.js delete mode 100644 src/components/createConnectWrapper.js rename test/components/{connectDecorator.spec.js => connectDecoratorOld.spec.js} (97%) create mode 100644 test/utils/wrapActionCreators.js diff --git a/src/components/createAll.js b/src/components/createAll.js index 43cda3bfe..4dba1c179 100644 --- a/src/components/createAll.js +++ b/src/components/createAll.js @@ -2,8 +2,8 @@ import createProvider from './createProvider'; import createProvideDecorator from './createProvideDecorator'; import createConnector from './createConnector'; +import createConnectDecoratorOld from './createConnectDecoratorOld'; import createConnectDecorator from './createConnectDecorator'; -import createConnectWrapper from './createConnectWrapper'; export default function createAll(React) { // Wrapper components @@ -12,10 +12,8 @@ export default function createAll(React) { // Higher-order components (decorators) const provide = createProvideDecorator(React, Provider); - const connectDecorate = createConnectDecorator(React, Connector); - const connect = createConnectWrapper(React); + const connectDecoratorOld = createConnectDecoratorOld(React, Connector); + const connect = createConnectDecorator(React); - - - return { Provider, Connector, provide, connectDecorate, connect }; + return { Provider, Connector, provide, connect, connectDecoratorOld }; } diff --git a/src/components/createConnectDecorator.js b/src/components/createConnectDecorator.js index a3196e1c1..a12a1bdf4 100644 --- a/src/components/createConnectDecorator.js +++ b/src/components/createConnectDecorator.js @@ -1,24 +1,121 @@ +import createStoreShape from '../utils/createStoreShape'; import getDisplayName from '../utils/getDisplayName'; import shallowEqualScalar from '../utils/shallowEqualScalar'; +import shallowEqual from '../utils/shallowEqual'; +import isPlainObject from '../utils/isPlainObject'; +import wrapActionCreators from '../utils/wrapActionCreators'; +import invariant from 'invariant'; -export default function createConnectDecorator(React, Connector) { - const { Component } = React; +const emptySelector = () => ({}); - return function connect(select) { - return DecoratedComponent => class ConnectorDecorator extends Component { - static displayName = `Connector(${getDisplayName(DecoratedComponent)})`; +const emptyBinder = () => ({}); + +const identityMerge = (slice, actionsCreators, props) => ({...slice, ...actionsCreators, ...props}); + + +export default function createConnectDecorator(React) { + const { Component, PropTypes } = React; + const storeShape = createStoreShape(PropTypes); + + return function connect(select, dispatchBinder = emptyBinder, mergeHandler = identityMerge) { + + const subscribing = select ? true : false; + const selectState = select || emptySelector; + const bindActionCreators = isPlainObject(dispatchBinder) ? wrapActionCreators(dispatchBinder) : dispatchBinder; + const merge = mergeHandler; + + return DecoratedComponent => class ConnectDecorator extends Component { + static displayName = `ConnectDecorator(${getDisplayName(DecoratedComponent)})`; static DecoratedComponent = DecoratedComponent; - shouldComponentUpdate(nextProps) { - return !shallowEqualScalar(this.props, nextProps); + static contextTypes = { + store: storeShape.isRequired + }; + + shouldComponentUpdate(nextProps, nextState) { + return (this.subscribed && !this.isSliceEqual(this.state.slice, nextState.slice)) || + !shallowEqualScalar(this.props, nextProps); } - render() { - return ( - select(state, this.props)}> - {stuff => } - + isSliceEqual(slice, nextSlice) { + const isRefEqual = slice === nextSlice; + if (isRefEqual) { + return true; + } else if (typeof slice !== 'object' || typeof nextSlice !== 'object') { + return isRefEqual; + } + return shallowEqual(slice, nextSlice); + } + + constructor(props, context) { + super(props, context); + this.state = { + ...this.selectState(props, context), + ...this.bindActionCreators(context) + }; + } + + componentDidMount() { + if (subscribing) { + this.subscribed = true; + this.unsubscribe = this.context.store.subscribe(::this.handleChange); + } + } + + componentWillUnmount() { + if (subscribing) { + this.unsubscribe(); + } + } + + handleChange(props = this.props) { + const nextState = this.selectState(props, this.context); + if (!this.isSliceEqual(this.state.slice, nextState.slice)) { + this.setState(nextState); + } + } + + selectState(props = this.props, context = this.context) { + const state = context.store.getState(); + const slice = selectState(state); + + invariant( + isPlainObject(slice), + 'The return value of `select` prop must be an object. Instead received %s.', + slice + ); + + return { slice }; + } + + bindActionCreators(context = this.context) { + const { dispatch } = context.store; + const actionCreators = bindActionCreators(dispatch); + + invariant( + isPlainObject(actionCreators), + 'The return value of `bindActionCreators` prop must be an object. Instead received %s.', + actionCreators + ); + + return { actionCreators }; + } + + merge(props = this.props, state = this.state) { + const { slice, actionCreators } = state; + const merged = merge(slice, actionCreators, props); + + invariant( + isPlainObject(merged), + 'The return value of `merge` prop must be an object. Instead received %s.', + merged ); + + return merged; + } + + render() { + return ; } }; }; diff --git a/src/components/createConnectDecoratorOld.js b/src/components/createConnectDecoratorOld.js new file mode 100644 index 000000000..a3196e1c1 --- /dev/null +++ b/src/components/createConnectDecoratorOld.js @@ -0,0 +1,25 @@ +import getDisplayName from '../utils/getDisplayName'; +import shallowEqualScalar from '../utils/shallowEqualScalar'; + +export default function createConnectDecorator(React, Connector) { + const { Component } = React; + + return function connect(select) { + return DecoratedComponent => class ConnectorDecorator extends Component { + static displayName = `Connector(${getDisplayName(DecoratedComponent)})`; + static DecoratedComponent = DecoratedComponent; + + shouldComponentUpdate(nextProps) { + return !shallowEqualScalar(this.props, nextProps); + } + + render() { + return ( + select(state, this.props)}> + {stuff => } + + ); + } + }; + }; +} diff --git a/src/components/createConnectWrapper.js b/src/components/createConnectWrapper.js deleted file mode 100644 index 80d6d6f45..000000000 --- a/src/components/createConnectWrapper.js +++ /dev/null @@ -1,140 +0,0 @@ -import createStoreShape from '../utils/createStoreShape'; -import getDisplayName from '../utils/getDisplayName'; -import shallowEqual from '../utils/shallowEqual'; -import isPlainObject from '../utils/isPlainObject'; -import wrapActionCreators from '../utils/wrapActionCreators'; -import invariant from 'invariant'; - -const emptySelector = () => ({}); - -const emptyBinder = () => ({}); - -const identityMerge = (slice, actionsCreators, props) => ({...slice, ...actionsCreators, ...props}); - - -export default function createConnectWrapper(React) { - const { Component, PropTypes } = React; - const storeShape = createStoreShape(PropTypes); - - return function connect(select, bindActionCreators, merge) { - - const subscribing = select ? true : false; - - select = select || emptySelector; - - bindActionCreators = bindActionCreators || emptyBinder; - - if (isPlainObject(bindActionCreators)) { - bindActionCreators = wrapActionCreators(bindActionCreators); - } - - merge = merge || identityMerge; - - return DecoratedComponent => class ConnectWrapper extends Component { - static displayName = `ConnectWrapper(${getDisplayName(DecoratedComponent)})`; - static DecoratedComponent = DecoratedComponent; - - static contextTypes = { - store: storeShape.isRequired - }; - - componentWillReceiveProps(nextProps) { - console.log('recieving props', this.props, nextProps) - } - - shouldComponentUpdate(nextProps, nextState) { - console.log('shallowEqual of props', shallowEqual(this.props, nextProps), this.props, nextProps) - return (this.subscribed && !this.isSliceEqual(this.state.slice, nextState.slice)) || - !shallowEqual(this.props, nextProps); - } - - isSliceEqual(slice, nextSlice) { - const isRefEqual = slice === nextSlice; - if (isRefEqual) { - return true; - } else if (typeof slice !== 'object' || typeof nextSlice !== 'object') { - return isRefEqual; - } - return shallowEqual(slice, nextSlice); - } - - constructor(props, context) { - super(props, context); - this.state = { - ...this.selectState(props, context), - ...this.bindActionCreators(context), - }; - } - - componentWillMount() { - console.log('will mount', this.props) - } - - componentDidMount() { - console.log('mounted', this.props) - if (subscribing) { - this.subscribed = true; - this.unsubscribe = this.context.store.subscribe(::this.handleChange); - } - } - - componentWillUnmount() { - if (subscribing) { - this.unsubscribe(); - } - } - - handleChange(props = this.props) { - const nextState = this.selectState(props, this.context); - if (!this.isSliceEqual(this.state.slice, nextState.slice)) { - this.setState(nextState); - } - } - - selectState(props = this.props, context = this.context) { - const state = context.store.getState(); - const slice = select(state); - - invariant( - isPlainObject(slice), - 'The return value of `select` prop must be an object. Instead received %s.', - slice - ); - - return { slice }; - } - - bindActionCreators(context = this.context) { - const { dispatch } = context.store; - const actionCreators = bindActionCreators(dispatch); - - invariant( - isPlainObject(actionCreators), - 'The return value of `bindActionCreators` prop must be an object. Instead received %s.', - actionCreators - ); - - return { actionCreators }; - } - - merge(props = this.props, state = this.state) { - const { slice, actionCreators } = state; - const merged = merge(slice, actionCreators, props); - - invariant( - isPlainObject(merged), - 'The return value of `merge` prop must be an object. Instead received %s.', - merged - ); - - console.log('merging with ', merged) - - return merged; - } - - render() { - return ; - } - }; - }; -} diff --git a/src/index.js b/src/index.js index 3ac9aeeb7..3990fe043 100644 --- a/src/index.js +++ b/src/index.js @@ -1,4 +1,4 @@ import React from 'react'; import createAll from './components/createAll'; -export const { Provider, Connector, provide, connectDecorate, connect } = createAll(React); +export const { Provider, Connector, provide, connect, connectDecoratorOld } = createAll(React); diff --git a/src/native.js b/src/native.js index b7e1fe8d1..7fc78db8a 100644 --- a/src/native.js +++ b/src/native.js @@ -1,4 +1,4 @@ import React from 'react-native'; import createAll from './components/createAll'; -export const { Provider, Connector, provide, connectDecorate, connect } = createAll(React); +export const { Provider, Connector, provide, connect, connectDecoratorOld } = createAll(React); diff --git a/src/utils/wrapActionCreators.js b/src/utils/wrapActionCreators.js index 4d186f7b5..983fbe606 100644 --- a/src/utils/wrapActionCreators.js +++ b/src/utils/wrapActionCreators.js @@ -1,5 +1,5 @@ -import { bindActionCreators } from 'redux' +import { bindActionCreators } from 'redux'; -export default function wrapActionCreators (actionCreators) { +export default function wrapActionCreators(actionCreators) { return dispatch => bindActionCreators(actionCreators, dispatch); -} \ No newline at end of file +} diff --git a/test/components/connect.spec.js b/test/components/connect.spec.js index 4cdcfd452..5938d99c3 100644 --- a/test/components/connect.spec.js +++ b/test/components/connect.spec.js @@ -1,7 +1,7 @@ import expect from 'expect'; import jsdomReact from './jsdomReact'; import React, { PropTypes, Component } from 'react/addons'; -import { createStore } from 'redux'; +import { createStore, combineReducers } from 'redux'; import { connect } from '../../src/index'; const { TestUtils } = React.addons; @@ -25,6 +25,34 @@ describe('React', () => { } } + function stringBuilder(prev = '', action) { + return action.type === 'APPEND' + ? prev + action.body + : prev; + } + + it('should receive the store in the context', () => { + const store = createStore(() => ({})); + + @connect() + class Container extends Component { + render() { + return
; + } + } + + const tree = TestUtils.renderIntoDocument( + + {() => ( + + )} + + ); + + const container = TestUtils.findRenderedComponentWithType(tree, Container); + expect(container.context.store).toBe(store); + }); + it('should wrap the component into Provider', () => { const store = createStore(() => ({ foo: 'bar' @@ -50,6 +78,33 @@ describe('React', () => { ).toNotThrow(); }); + it('should subscribe to the store changes', () => { + const store = createStore(stringBuilder); + + @connect(state => ({string: state}) ) + class Container extends Component { + render() { + return
; + } + } + + const tree = TestUtils.renderIntoDocument( + + {() => ( + + )} + + ); + + const div = TestUtils.findRenderedDOMComponentWithTag(tree, 'div'); + + expect(div.props.string).toBe(''); + store.dispatch({ type: 'APPEND', body: 'a'}); + expect(div.props.string).toBe('a'); + store.dispatch({ type: 'APPEND', body: 'b'}); + expect(div.props.string).toBe('ab'); + }); + it('should handle additional prop changes in addition to slice', () => { const store = createStore(() => ({ foo: 'bar' @@ -96,17 +151,101 @@ describe('React', () => { expect(div.props.pass).toEqual('through'); }); - it('should pass the only argument as the select prop down', () => { + it('should allow for merge to incorporate state and prop changes', () => { + const store = createStore(stringBuilder); + + function doSomething(thing) { + return { + type: 'APPEND', + body: thing + }; + } + + @connect( + state => ({stateThing: state}), + dispatch => ({doSomething: (whatever) => dispatch(doSomething(whatever)) }), + (stateProps, actionProps, parentProps) => ({ + ...stateProps, + ...actionProps, + mergedDoSomething: (thing) => { + const seed = stateProps.stateThing === '' ? 'HELLO ' : ''; + actionProps.doSomething(seed + thing + parentProps.extra); + } + }) + ) + class Container extends Component { + render() { + return
; + }; + } + + class OuterContainer extends Component { + constructor() { + super(); + this.state = { extra: 'z' }; + } + + render() { + return ( + + {() => } + + ); + } + } + + const tree = TestUtils.renderIntoDocument(); + const div = TestUtils.findRenderedDOMComponentWithTag(tree, 'div'); + + expect(div.props.stateThing).toBe(''); + div.props.mergedDoSomething('a'); + expect(div.props.stateThing).toBe('HELLO az'); + div.props.mergedDoSomething('b'); + expect(div.props.stateThing).toBe('HELLO azbz'); + tree.setState({extra: 'Z'}); + div.props.mergedDoSomething('c'); + expect(div.props.stateThing).toBe('HELLO azbzcZ'); + }); + + it('should merge actionProps into DecoratedComponent', () => { const store = createStore(() => ({ - foo: 'baz', - bar: 'baz' + foo: 'bar' })); - function select({ foo }) { - return { foo }; + @connect( + state => state, + dispatch => ({ dispatch }) + ) + class Container extends Component { + render() { + return
; + } } - @connect(select) + const container = TestUtils.renderIntoDocument( + + {() => } + + ); + const div = TestUtils.findRenderedDOMComponentWithTag(container, 'div'); + expect(div.props.dispatch).toEqual(store.dispatch); + expect(div.props.foo).toEqual('bar'); + expect(() => + TestUtils.findRenderedComponentWithType(container, Container) + ).toNotThrow(); + const decorated = TestUtils.findRenderedComponentWithType(container, Container); + expect(decorated.subscribed).toBe(true); + }); + + it('should not subscribe to stores if select argument is null', () => { + const store = createStore(() => ({ + foo: 'bar' + })); + + @connect( + null, + dispatch => ({ dispatch }) + ) class Container extends Component { render() { return
; @@ -118,13 +257,238 @@ describe('React', () => { {() => } ); - const connector = TestUtils.findRenderedComponentWithType(container, Connector); - expect(connector.props.select({ - foo: 5, - bar: 7 - })).toEqual({ - foo: 5 - }); + const div = TestUtils.findRenderedDOMComponentWithTag(container, 'div'); + expect(div.props.dispatch).toEqual(store.dispatch); + expect(div.props.foo).toBe(undefined); + expect(() => + TestUtils.findRenderedComponentWithType(container, Container) + ).toNotThrow(); + const decorated = TestUtils.findRenderedComponentWithType(container, Container); + expect(decorated.subscribed).toNotBe(true); + + }); + + it('should unsubscribe before unmounting', () => { + const store = createStore(stringBuilder); + const subscribe = store.subscribe; + + // Keep track of unsubscribe by wrapping subscribe() + const spy = expect.createSpy(() => ({})); + store.subscribe = (listener) => { + const unsubscribe = subscribe(listener); + return () => { + spy(); + return unsubscribe(); + }; + }; + + @connect( + state => ({string: state}), + dispatch => ({ dispatch }) + ) + class Container extends Component { + render() { + return
; + } + } + + const tree = TestUtils.renderIntoDocument( + + {() => ( + + )} + + ); + + const connector = TestUtils.findRenderedComponentWithType(tree, Container); + expect(spy.calls.length).toBe(0); + connector.componentWillUnmount(); + expect(spy.calls.length).toBe(1); + }); + + it('should shallowly compare the selected state to prevent unnecessary updates', () => { + const store = createStore(stringBuilder); + const spy = expect.createSpy(() => ({})); + function render({ string }) { + spy(); + return
; + } + + @connect( + state => ({string: state}), + dispatch => ({ dispatch }) + ) + class Container extends Component { + render() { + return render(this.props); + } + } + + const tree = TestUtils.renderIntoDocument( + + {() => ( + + )} + + ); + + const div = TestUtils.findRenderedDOMComponentWithTag(tree, 'div'); + expect(spy.calls.length).toBe(1); + expect(div.props.string).toBe(''); + store.dispatch({ type: 'APPEND', body: 'a'}); + expect(spy.calls.length).toBe(2); + store.dispatch({ type: 'APPEND', body: 'b'}); + expect(spy.calls.length).toBe(3); + store.dispatch({ type: 'APPEND', body: ''}); + expect(spy.calls.length).toBe(3); + }); + + it('should recompute the state slice when the select prop changes', () => { + const store = createStore(combineReducers({ + a: () => 42, + b: () => 72 + })); + + function selectA(state) { + return { result: state.a }; + } + + function selectB(state) { + return { result: state.b }; + } + + function render({ result }) { + return
{result}
; + } + + function getContainer(select) { + return ( + @connect(select) + class Container extends Component { + render() { + return this.props.children(this.props); + } + } + ); + } + + class OuterContainer extends Component { + constructor() { + super(); + this.state = { select: selectA }; + } + + render() { + return ( + + {() => { + const Container = getContainer(this.state.select); + return ( + + {render} + + ); + }} + + ); + } + } + + let tree = TestUtils.renderIntoDocument(); + let div = TestUtils.findRenderedDOMComponentWithTag(tree, 'div'); + expect(div.props.children).toBe(42); + + tree.setState({ select: selectB }); + expect(div.props.children).toBe(72); + }); + + it('should throw an error if select, bindActionCreators, or merge returns anything but a plain object', () => { + const store = createStore(() => ({})); + + function makeContainer(select, bindActionCreators, merge) { + return React.createElement( + @connect(select, bindActionCreators, merge) + class Container extends Component { + render() { + return
; + } + } + ); + } + + function AwesomeMap() { } + + expect(() => { + TestUtils.renderIntoDocument( + + { () => makeContainer(() => 1, () => ({}), () => ({})) } + + ); + }).toThrow(/select/); + + expect(() => { + TestUtils.renderIntoDocument( + + { () => makeContainer(() => 'hey', () => ({}), () => ({})) } + + ); + }).toThrow(/select/); + + expect(() => { + TestUtils.renderIntoDocument( + + { () => makeContainer(() => new AwesomeMap(), () => ({}), () => ({})) } + + ); + }).toThrow(/select/); + + expect(() => { + TestUtils.renderIntoDocument( + + { () => makeContainer(() => ({}), () => 1, () => ({})) } + + ); + }).toThrow(/bindActionCreators/); + + expect(() => { + TestUtils.renderIntoDocument( + + { () => makeContainer(() => ({}), () => 'hey', () => ({})) } + + ); + }).toThrow(/bindActionCreators/); + + expect(() => { + TestUtils.renderIntoDocument( + + { () => makeContainer(() => ({}), () => new AwesomeMap(), () => ({})) } + + ); + }).toThrow(/bindActionCreators/); + + expect(() => { + TestUtils.renderIntoDocument( + + { () => makeContainer(() => ({}), () => ({}), () => 1) } + + ); + }).toThrow(/merge/); + + expect(() => { + TestUtils.renderIntoDocument( + + { () => makeContainer(() => ({}), () => ({}), () => 'hey') } + + ); + }).toThrow(/merge/); + + expect(() => { + TestUtils.renderIntoDocument( + + { () => makeContainer(() => ({}), () => ({}), () => new AwesomeMap()) } + + ); + }).toThrow(/merge/); }); it('should set the displayName correctly', () => { @@ -135,7 +499,7 @@ describe('React', () => { } } - expect(Container.displayName).toBe('Connector(Container)'); + expect(Container.displayName).toBe('ConnectDecorator(Container)'); }); it('should expose the wrapped component as DecoratedComponent', () => { diff --git a/test/components/connectDecorator.spec.js b/test/components/connectDecoratorOld.spec.js similarity index 97% rename from test/components/connectDecorator.spec.js rename to test/components/connectDecoratorOld.spec.js index 47b7a1063..36b491a38 100644 --- a/test/components/connectDecorator.spec.js +++ b/test/components/connectDecoratorOld.spec.js @@ -2,9 +2,9 @@ import expect from 'expect'; import jsdomReact from './jsdomReact'; import React, { PropTypes, Component } from 'react/addons'; import { createStore } from 'redux'; -import { connectDecorate, Connector } from '../../src/index'; +import { connectDecoratorOld, Connector } from '../../src/index'; -const connect = connectDecorate; +const connect = connectDecoratorOld; const { TestUtils } = React.addons; diff --git a/test/utils/wrapActionCreators.js b/test/utils/wrapActionCreators.js new file mode 100644 index 000000000..af31ce4d6 --- /dev/null +++ b/test/utils/wrapActionCreators.js @@ -0,0 +1,31 @@ +import expect from 'expect'; +import wrapActionCreators from '../../src/utils/wrapActionCreators'; + +describe('Utils', () => { + describe('wrapActionCreators', () => { + it('should return a function that wraps argument in a call to bindActionCreators', () => { + + function dispatch(action) { + return { + dispatched: action + }; + } + + const actionResult = {an: 'action'}; + + const actionCreators = { + action: () => actionResult + }; + + const wrapped = wrapActionCreators(actionCreators); + expect(wrapped).toBeA(Function); + expect(() => wrapped(dispatch)).toNotThrow(); + expect(() => wrapped().action()).toThrow(); + + const bound = wrapped(dispatch); + expect(bound.action).toNotThrow(); + expect(bound.action().dispatched).toBe(actionResult); + + }); + }); +}); From ce8bf1ee5a5c5c09bcf4ea03149aa0bddecfae29 Mon Sep 17 00:00:00 2001 From: gnoff Date: Thu, 30 Jul 2015 23:30:35 -0700 Subject: [PATCH 03/10] implemented new connect api. takes in selectState, dispatchBinder, merge and puts sequential results of such on props of decorated component. Connector is intended to be deprecated. older connectDecorator is now called connecctDeprecated --- src/components/createAll.js | 6 +++--- ...tDecoratorOld.js => createConnectDecoratorDeprecated.js} | 0 src/index.js | 2 +- src/native.js | 2 +- ...onnectDecoratorOld.spec.js => connectDeprecated.spec.js} | 4 ++-- 5 files changed, 7 insertions(+), 7 deletions(-) rename src/components/{createConnectDecoratorOld.js => createConnectDecoratorDeprecated.js} (100%) rename test/components/{connectDecoratorOld.spec.js => connectDeprecated.spec.js} (97%) diff --git a/src/components/createAll.js b/src/components/createAll.js index 4dba1c179..c0de66bfd 100644 --- a/src/components/createAll.js +++ b/src/components/createAll.js @@ -2,7 +2,7 @@ import createProvider from './createProvider'; import createProvideDecorator from './createProvideDecorator'; import createConnector from './createConnector'; -import createConnectDecoratorOld from './createConnectDecoratorOld'; +import createConnectDecoratorDeprecated from './createConnectDecoratorDeprecated'; import createConnectDecorator from './createConnectDecorator'; export default function createAll(React) { @@ -12,8 +12,8 @@ export default function createAll(React) { // Higher-order components (decorators) const provide = createProvideDecorator(React, Provider); - const connectDecoratorOld = createConnectDecoratorOld(React, Connector); + const connectDeprecated = createConnectDecoratorDeprecated(React, Connector); const connect = createConnectDecorator(React); - return { Provider, Connector, provide, connect, connectDecoratorOld }; + return { Provider, Connector, provide, connect, connectDeprecated }; } diff --git a/src/components/createConnectDecoratorOld.js b/src/components/createConnectDecoratorDeprecated.js similarity index 100% rename from src/components/createConnectDecoratorOld.js rename to src/components/createConnectDecoratorDeprecated.js diff --git a/src/index.js b/src/index.js index 3990fe043..f09a801ee 100644 --- a/src/index.js +++ b/src/index.js @@ -1,4 +1,4 @@ import React from 'react'; import createAll from './components/createAll'; -export const { Provider, Connector, provide, connect, connectDecoratorOld } = createAll(React); +export const { Provider, Connector, provide, connect, connectDeprecated } = createAll(React); diff --git a/src/native.js b/src/native.js index 7fc78db8a..16471679b 100644 --- a/src/native.js +++ b/src/native.js @@ -1,4 +1,4 @@ import React from 'react-native'; import createAll from './components/createAll'; -export const { Provider, Connector, provide, connect, connectDecoratorOld } = createAll(React); +export const { Provider, Connector, provide, connect, connectDeprecated } = createAll(React); diff --git a/test/components/connectDecoratorOld.spec.js b/test/components/connectDeprecated.spec.js similarity index 97% rename from test/components/connectDecoratorOld.spec.js rename to test/components/connectDeprecated.spec.js index 36b491a38..3a65f89b0 100644 --- a/test/components/connectDecoratorOld.spec.js +++ b/test/components/connectDeprecated.spec.js @@ -2,9 +2,9 @@ import expect from 'expect'; import jsdomReact from './jsdomReact'; import React, { PropTypes, Component } from 'react/addons'; import { createStore } from 'redux'; -import { connectDecoratorOld, Connector } from '../../src/index'; +import { connectDeprecated, Connector } from '../../src/index'; -const connect = connectDecoratorOld; +const connect = connectDeprecated; const { TestUtils } = React.addons; From a78837612b017e3604e9de42c6bfb59062545f19 Mon Sep 17 00:00:00 2001 From: gnoff Date: Thu, 30 Jul 2015 23:56:03 -0700 Subject: [PATCH 04/10] forgot to add {...this.props} to DecoratedComponent in render --- src/components/createConnectDecorator.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/createConnectDecorator.js b/src/components/createConnectDecorator.js index a12a1bdf4..705349009 100644 --- a/src/components/createConnectDecorator.js +++ b/src/components/createConnectDecorator.js @@ -115,7 +115,7 @@ export default function createConnectDecorator(React) { } render() { - return ; + return ; } }; }; From b446894a1d21aa948a34fc39223b42129ef82765 Mon Sep 17 00:00:00 2001 From: gnoff Date: Fri, 31 Jul 2015 00:19:38 -0700 Subject: [PATCH 05/10] use ref callback to place instance of underlying component on wrapped component. provide wrapper method to get underlying ref to provide access to unerlying refs methods --- src/components/createConnectDecorator.js | 6 ++++- test/components/connect.spec.js | 34 ++++++++++++++++++++++++ 2 files changed, 39 insertions(+), 1 deletion(-) diff --git a/src/components/createConnectDecorator.js b/src/components/createConnectDecorator.js index 705349009..d65f8878e 100644 --- a/src/components/createConnectDecorator.js +++ b/src/components/createConnectDecorator.js @@ -114,8 +114,12 @@ export default function createConnectDecorator(React) { return merged; } + getUnderlyingRef() { + return this.underlyingRef; + } + render() { - return ; + return (this.underlyingRef = component)} {...this.props} {...this.merge()} />; } }; }; diff --git a/test/components/connect.spec.js b/test/components/connect.spec.js index 5938d99c3..d8e35026b 100644 --- a/test/components/connect.spec.js +++ b/test/components/connect.spec.js @@ -514,5 +514,39 @@ describe('React', () => { expect(decorated.DecoratedComponent).toBe(Container); }); + + it('should return the instance of the wrapped component for use in calling child methods', () => { + const store = createStore(() => ({})); + + const someData = { + some: 'data' + }; + + class Container extends Component { + someInstanceMethod() { + return someData; + } + + render() { + return
; + } + } + + const decorator = connect(state => state); + const Decorated = decorator(Container); + + const tree = TestUtils.renderIntoDocument( + + {() => ( + + )} + + ); + + const decorated = TestUtils.findRenderedComponentWithType(tree, Decorated); + + expect(() => decorated.someInstanceMethod()).toThrow(); + expect(decorated.getUnderlyingRef().someInstanceMethod()).toBe(someData); + }); }); }); From df18c6d3ab273e75b5d7a3c080daade08cff84e0 Mon Sep 17 00:00:00 2001 From: gnoff Date: Fri, 31 Jul 2015 00:24:24 -0700 Subject: [PATCH 06/10] remvoing deprecated code for Connector and original connectDecorator --- src/components/createAll.js | 6 +- .../createConnectDecoratorDeprecated.js | 25 -- src/components/createConnector.js | 88 ------ test/components/Connector.spec.js | 295 ------------------ test/components/connectDeprecated.spec.js | 156 --------- 5 files changed, 1 insertion(+), 569 deletions(-) delete mode 100644 src/components/createConnectDecoratorDeprecated.js delete mode 100644 src/components/createConnector.js delete mode 100644 test/components/Connector.spec.js delete mode 100644 test/components/connectDeprecated.spec.js diff --git a/src/components/createAll.js b/src/components/createAll.js index c0de66bfd..4699d819b 100644 --- a/src/components/createAll.js +++ b/src/components/createAll.js @@ -1,19 +1,15 @@ import createProvider from './createProvider'; import createProvideDecorator from './createProvideDecorator'; -import createConnector from './createConnector'; -import createConnectDecoratorDeprecated from './createConnectDecoratorDeprecated'; import createConnectDecorator from './createConnectDecorator'; export default function createAll(React) { // Wrapper components const Provider = createProvider(React); - const Connector = createConnector(React); // Higher-order components (decorators) const provide = createProvideDecorator(React, Provider); - const connectDeprecated = createConnectDecoratorDeprecated(React, Connector); const connect = createConnectDecorator(React); - return { Provider, Connector, provide, connect, connectDeprecated }; + return { Provider, provide, connect }; } diff --git a/src/components/createConnectDecoratorDeprecated.js b/src/components/createConnectDecoratorDeprecated.js deleted file mode 100644 index a3196e1c1..000000000 --- a/src/components/createConnectDecoratorDeprecated.js +++ /dev/null @@ -1,25 +0,0 @@ -import getDisplayName from '../utils/getDisplayName'; -import shallowEqualScalar from '../utils/shallowEqualScalar'; - -export default function createConnectDecorator(React, Connector) { - const { Component } = React; - - return function connect(select) { - return DecoratedComponent => class ConnectorDecorator extends Component { - static displayName = `Connector(${getDisplayName(DecoratedComponent)})`; - static DecoratedComponent = DecoratedComponent; - - shouldComponentUpdate(nextProps) { - return !shallowEqualScalar(this.props, nextProps); - } - - render() { - return ( - select(state, this.props)}> - {stuff => } - - ); - } - }; - }; -} diff --git a/src/components/createConnector.js b/src/components/createConnector.js deleted file mode 100644 index 2318092e5..000000000 --- a/src/components/createConnector.js +++ /dev/null @@ -1,88 +0,0 @@ -import createStoreShape from '../utils/createStoreShape'; -import shallowEqual from '../utils/shallowEqual'; -import isPlainObject from '../utils/isPlainObject'; -import invariant from 'invariant'; - -export default function createConnector(React) { - const { Component, PropTypes } = React; - const storeShape = createStoreShape(PropTypes); - - return class Connector extends Component { - static contextTypes = { - store: storeShape.isRequired - }; - - static propTypes = { - children: PropTypes.func.isRequired, - select: PropTypes.func.isRequired - }; - - static defaultProps = { - select: state => state - }; - - shouldComponentUpdate(nextProps, nextState) { - return !this.isSliceEqual(this.state.slice, nextState.slice) || - !shallowEqual(this.props, nextProps); - } - - isSliceEqual(slice, nextSlice) { - const isRefEqual = slice === nextSlice; - if (isRefEqual) { - return true; - } else if (typeof slice !== 'object' || typeof nextSlice !== 'object') { - return isRefEqual; - } - return shallowEqual(slice, nextSlice); - } - - constructor(props, context) { - super(props, context); - this.state = this.selectState(props, context); - } - - componentDidMount() { - this.unsubscribe = this.context.store.subscribe(::this.handleChange); - this.handleChange(); - } - - componentWillReceiveProps(nextProps) { - if (nextProps.select !== this.props.select) { - // Force the state slice recalculation - this.handleChange(nextProps); - } - } - - componentWillUnmount() { - this.unsubscribe(); - } - - handleChange(props = this.props) { - const nextState = this.selectState(props, this.context); - if (!this.isSliceEqual(this.state.slice, nextState.slice)) { - this.setState(nextState); - } - } - - selectState(props, context) { - const state = context.store.getState(); - const slice = props.select(state); - - invariant( - isPlainObject(slice), - 'The return value of `select` prop must be an object. Instead received %s.', - slice - ); - - return { slice }; - } - - render() { - const { children } = this.props; - const { slice } = this.state; - const { store: { dispatch } } = this.context; - - return children({ dispatch, ...slice }); - } - }; -} diff --git a/test/components/Connector.spec.js b/test/components/Connector.spec.js deleted file mode 100644 index 6d386de82..000000000 --- a/test/components/Connector.spec.js +++ /dev/null @@ -1,295 +0,0 @@ -import expect from 'expect'; -import jsdomReact from './jsdomReact'; -import React, { PropTypes, Component } from 'react/addons'; -import { createStore, combineReducers } from 'redux'; -import { Connector } from '../../src/index'; - -const { TestUtils } = React.addons; - -describe('React', () => { - describe('Connector', () => { - jsdomReact(); - - // Mock minimal Provider interface - class Provider extends Component { - static childContextTypes = { - store: PropTypes.object.isRequired - } - - getChildContext() { - return { store: this.props.store }; - } - - render() { - return this.props.children(); - } - } - - function stringBuilder(prev = '', action) { - return action.type === 'APPEND' - ? prev + action.body - : prev; - } - - it('should receive the store in the context', () => { - const store = createStore(() => ({})); - - const tree = TestUtils.renderIntoDocument( - - {() => ( - - {() =>
} - - )} - - ); - - const connector = TestUtils.findRenderedComponentWithType(tree, Connector); - expect(connector.context.store).toBe(store); - }); - - it('should subscribe to the store changes', () => { - const store = createStore(stringBuilder); - - const tree = TestUtils.renderIntoDocument( - - {() => ( - ({ string })}> - {({ string }) =>
} - - )} - - ); - - const div = TestUtils.findRenderedDOMComponentWithTag(tree, 'div'); - expect(div.props.string).toBe(''); - store.dispatch({ type: 'APPEND', body: 'a'}); - expect(div.props.string).toBe('a'); - store.dispatch({ type: 'APPEND', body: 'b'}); - expect(div.props.string).toBe('ab'); - }); - - it('should unsubscribe before unmounting', () => { - const store = createStore(stringBuilder); - const subscribe = store.subscribe; - - // Keep track of unsubscribe by wrapping subscribe() - const spy = expect.createSpy(() => ({})); - store.subscribe = (listener) => { - const unsubscribe = subscribe(listener); - return () => { - spy(); - return unsubscribe(); - }; - }; - - const tree = TestUtils.renderIntoDocument( - - {() => ( - ({ string })}> - {({ string }) =>
} - - )} - - ); - - const connector = TestUtils.findRenderedComponentWithType(tree, Connector); - expect(spy.calls.length).toBe(0); - connector.componentWillUnmount(); - expect(spy.calls.length).toBe(1); - }); - - it('should shallowly compare the selected state to prevent unnecessary updates', () => { - const store = createStore(stringBuilder); - const spy = expect.createSpy(() => ({})); - function render({ string }) { - spy(); - return
; - } - - const tree = TestUtils.renderIntoDocument( - - {() => ( - ({ string })}> - {render} - - )} - - ); - - const div = TestUtils.findRenderedDOMComponentWithTag(tree, 'div'); - expect(spy.calls.length).toBe(1); - expect(div.props.string).toBe(''); - store.dispatch({ type: 'APPEND', body: 'a'}); - expect(spy.calls.length).toBe(2); - store.dispatch({ type: 'APPEND', body: 'b'}); - expect(spy.calls.length).toBe(3); - store.dispatch({ type: 'APPEND', body: ''}); - expect(spy.calls.length).toBe(3); - }); - - it('should recompute the state slice when the select prop changes', () => { - const store = createStore(combineReducers({ - a: () => 42, - b: () => 72 - })); - - function selectA(state) { - return { result: state.a }; - } - - function selectB(state) { - return { result: state.b }; - } - - function render({ result }) { - return
{result}
; - } - - class Container extends Component { - constructor() { - super(); - this.state = { select: selectA }; - } - - render() { - return ( - - {() => - - {render} - - } - - ); - } - } - - let tree = TestUtils.renderIntoDocument(); - let div = TestUtils.findRenderedDOMComponentWithTag(tree, 'div'); - expect(div.props.children).toBe(42); - - tree.setState({ select: selectB }); - expect(div.props.children).toBe(72); - }); - - it('should pass dispatch() to the child function', () => { - const store = createStore(() => ({})); - - const tree = TestUtils.renderIntoDocument( - - {() => ( - - {({ dispatch }) =>
} - - )} - - ); - - const div = TestUtils.findRenderedDOMComponentWithTag(tree, 'div'); - expect(div.props.dispatch).toBe(store.dispatch); - }); - - it('should throw an error if select returns anything but a plain object', () => { - const store = createStore(() => ({})); - - expect(() => { - TestUtils.renderIntoDocument( - - {() => ( - 1}> - {() =>
} - - )} - - ); - }).toThrow(/select/); - - expect(() => { - TestUtils.renderIntoDocument( - - {() => ( - 'hey'}> - {() =>
} - - )} - - ); - }).toThrow(/select/); - - function AwesomeMap() { } - - expect(() => { - TestUtils.renderIntoDocument( - - {() => ( - new AwesomeMap()}> - {() =>
} - - )} - - ); - }).toThrow(/select/); - }); - - it('should not setState when renderToString is called on the server', () => { - const { renderToString } = React; - const store = createStore(stringBuilder); - - class TestComp extends Component { - componentWillMount() { - store.dispatch({ - type: 'APPEND', - body: 'a' - }); - } - - render() { - return
{this.props.string}
; - } - } - - const el = ( - - {() => ( - ({ string })}> - {({ string }) => } - - )} - - ); - - expect(() => renderToString(el)).toNotThrow(); - }); - - it('should handle dispatch inside componentDidMount', () => { - const store = createStore(stringBuilder); - - class TestComp extends Component { - componentDidMount() { - store.dispatch({ - type: 'APPEND', - body: 'a' - }); - } - - render() { - return
{this.props.string}
; - } - } - - const tree = TestUtils.renderIntoDocument( - - {() => ( - ({ string })}> - {({ string }) => } - - )} - - ); - - const testComp = TestUtils.findRenderedComponentWithType(tree, TestComp); - expect(testComp.props.string).toBe('a'); - }); - }); -}); diff --git a/test/components/connectDeprecated.spec.js b/test/components/connectDeprecated.spec.js deleted file mode 100644 index 3a65f89b0..000000000 --- a/test/components/connectDeprecated.spec.js +++ /dev/null @@ -1,156 +0,0 @@ -import expect from 'expect'; -import jsdomReact from './jsdomReact'; -import React, { PropTypes, Component } from 'react/addons'; -import { createStore } from 'redux'; -import { connectDeprecated, Connector } from '../../src/index'; - -const connect = connectDeprecated; - -const { TestUtils } = React.addons; - -describe('React', () => { - describe('connectDecorate', () => { - jsdomReact(); - - // Mock minimal Provider interface - class Provider extends Component { - static childContextTypes = { - store: PropTypes.object.isRequired - } - - getChildContext() { - return { store: this.props.store }; - } - - render() { - return this.props.children(); - } - } - - it('should wrap the component into Provider', () => { - const store = createStore(() => ({ - foo: 'bar' - })); - - @connect(state => state) - class Container extends Component { - render() { - return
; - } - } - - const container = TestUtils.renderIntoDocument( - - {() => } - - ); - const div = TestUtils.findRenderedDOMComponentWithTag(container, 'div'); - expect(div.props.pass).toEqual('through'); - expect(div.props.foo).toEqual('bar'); - expect(() => - TestUtils.findRenderedComponentWithType(container, Connector) - ).toNotThrow(); - }); - - it('should handle additional prop changes in addition to slice', () => { - const store = createStore(() => ({ - foo: 'bar' - })); - - @connect(state => state) - class ConnectContainer extends Component { - render() { - return ( -
- ); - } - } - - class Container extends Component { - constructor() { - super(); - this.state = { - bar: { - baz: '' - } - }; - } - componentDidMount() { - - // Simulate deep object mutation - this.state.bar.baz = 'through'; - this.setState({ - bar: this.state.bar - }); - } - render() { - return ( - - {() => } - - ); - } - } - - const container = TestUtils.renderIntoDocument(); - const div = TestUtils.findRenderedDOMComponentWithTag(container, 'div'); - expect(div.props.foo).toEqual('bar'); - expect(div.props.pass).toEqual('through'); - }); - - it('should pass the only argument as the select prop down', () => { - const store = createStore(() => ({ - foo: 'baz', - bar: 'baz' - })); - - function select({ foo }) { - return { foo }; - } - - @connect(select) - class Container extends Component { - render() { - return
; - } - } - - const container = TestUtils.renderIntoDocument( - - {() => } - - ); - const connector = TestUtils.findRenderedComponentWithType(container, Connector); - expect(connector.props.select({ - foo: 5, - bar: 7 - })).toEqual({ - foo: 5 - }); - }); - - it('should set the displayName correctly', () => { - @connect(state => state) - class Container extends Component { - render() { - return
; - } - } - - expect(Container.displayName).toBe('Connector(Container)'); - }); - - it('should expose the wrapped component as DecoratedComponent', () => { - class Container extends Component { - render() { - return
; - } - } - - const decorator = connect(state => state); - const decorated = decorator(Container); - - expect(decorated.DecoratedComponent).toBe(Container); - }); - }); -}); From 74c3d9d11f732e95b81936255b2f010b44f2873f Mon Sep 17 00:00:00 2001 From: gnoff Date: Fri, 31 Jul 2015 00:26:28 -0700 Subject: [PATCH 07/10] move props in identityMerge to front to allow override by slice and actionCreators, remove from DecoratedComponent render --- src/components/createConnectDecorator.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/components/createConnectDecorator.js b/src/components/createConnectDecorator.js index 705349009..db56935a3 100644 --- a/src/components/createConnectDecorator.js +++ b/src/components/createConnectDecorator.js @@ -10,7 +10,7 @@ const emptySelector = () => ({}); const emptyBinder = () => ({}); -const identityMerge = (slice, actionsCreators, props) => ({...slice, ...actionsCreators, ...props}); +const identityMerge = (slice, actionsCreators, props) => ({ ...props, ...slice, ...actionsCreators}); export default function createConnectDecorator(React) { @@ -115,7 +115,7 @@ export default function createConnectDecorator(React) { } render() { - return ; + return ; } }; }; From 170b1b7195ec9ee9f79a3e9d23b9e69f4058d07c Mon Sep 17 00:00:00 2001 From: Josh Story Date: Fri, 31 Jul 2015 10:36:29 -0700 Subject: [PATCH 08/10] changed ConnectDecorator#bindActionCreators to ConnectDecorator#bindDispatch and updated corresponding method names. this is to avoid confusion with the redux utility bindActionCreators and the fact that the second argument doesn not have to necessarily be related to action creators in any way --- src/components/createConnectDecorator.js | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/components/createConnectDecorator.js b/src/components/createConnectDecorator.js index 4349d4fc3..51510e361 100644 --- a/src/components/createConnectDecorator.js +++ b/src/components/createConnectDecorator.js @@ -21,7 +21,7 @@ export default function createConnectDecorator(React) { const subscribing = select ? true : false; const selectState = select || emptySelector; - const bindActionCreators = isPlainObject(dispatchBinder) ? wrapActionCreators(dispatchBinder) : dispatchBinder; + const bindDispatch = isPlainObject(dispatchBinder) ? wrapActionCreators(dispatchBinder) : dispatchBinder; const merge = mergeHandler; return DecoratedComponent => class ConnectDecorator extends Component { @@ -51,7 +51,7 @@ export default function createConnectDecorator(React) { super(props, context); this.state = { ...this.selectState(props, context), - ...this.bindActionCreators(context) + ...this.bindDispatch(context) }; } @@ -88,13 +88,13 @@ export default function createConnectDecorator(React) { return { slice }; } - bindActionCreators(context = this.context) { + bindDispatch(context = this.context) { const { dispatch } = context.store; - const actionCreators = bindActionCreators(dispatch); + const actionCreators = bindDispatch(dispatch); invariant( isPlainObject(actionCreators), - 'The return value of `bindActionCreators` prop must be an object. Instead received %s.', + 'The return value of `bindDispatch` prop must be an object. Instead received %s.', actionCreators ); From 105a2aa3b613caf54a18bc8904c75192a5084836 Mon Sep 17 00:00:00 2001 From: Josh Story Date: Fri, 31 Jul 2015 10:39:13 -0700 Subject: [PATCH 09/10] remove test for changing select prop as it no longer applies to new api --- test/components/connect.spec.js | 59 --------------------------------- 1 file changed, 59 deletions(-) diff --git a/test/components/connect.spec.js b/test/components/connect.spec.js index d8e35026b..2ffb2bec4 100644 --- a/test/components/connect.spec.js +++ b/test/components/connect.spec.js @@ -343,65 +343,6 @@ describe('React', () => { expect(spy.calls.length).toBe(3); }); - it('should recompute the state slice when the select prop changes', () => { - const store = createStore(combineReducers({ - a: () => 42, - b: () => 72 - })); - - function selectA(state) { - return { result: state.a }; - } - - function selectB(state) { - return { result: state.b }; - } - - function render({ result }) { - return
{result}
; - } - - function getContainer(select) { - return ( - @connect(select) - class Container extends Component { - render() { - return this.props.children(this.props); - } - } - ); - } - - class OuterContainer extends Component { - constructor() { - super(); - this.state = { select: selectA }; - } - - render() { - return ( - - {() => { - const Container = getContainer(this.state.select); - return ( - - {render} - - ); - }} - - ); - } - } - - let tree = TestUtils.renderIntoDocument(); - let div = TestUtils.findRenderedDOMComponentWithTag(tree, 'div'); - expect(div.props.children).toBe(42); - - tree.setState({ select: selectB }); - expect(div.props.children).toBe(72); - }); - it('should throw an error if select, bindActionCreators, or merge returns anything but a plain object', () => { const store = createStore(() => ({})); From c54a8b80da1dcf865bf54da02fffda84c0094fe3 Mon Sep 17 00:00:00 2001 From: gnoff Date: Wed, 5 Aug 2015 21:48:29 -0700 Subject: [PATCH 10/10] fixing test reference to bindActionCreators which was renamed bindDispatch --- test/components/connect.spec.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/test/components/connect.spec.js b/test/components/connect.spec.js index 2ffb2bec4..227d5f123 100644 --- a/test/components/connect.spec.js +++ b/test/components/connect.spec.js @@ -343,7 +343,7 @@ describe('React', () => { expect(spy.calls.length).toBe(3); }); - it('should throw an error if select, bindActionCreators, or merge returns anything but a plain object', () => { + it('should throw an error if select, bindDispatch, or merge returns anything but a plain object', () => { const store = createStore(() => ({})); function makeContainer(select, bindActionCreators, merge) { @@ -389,7 +389,7 @@ describe('React', () => { { () => makeContainer(() => ({}), () => 1, () => ({})) } ); - }).toThrow(/bindActionCreators/); + }).toThrow(/bindDispatch/); expect(() => { TestUtils.renderIntoDocument( @@ -397,7 +397,7 @@ describe('React', () => { { () => makeContainer(() => ({}), () => 'hey', () => ({})) } ); - }).toThrow(/bindActionCreators/); + }).toThrow(/bindDispatch/); expect(() => { TestUtils.renderIntoDocument( @@ -405,7 +405,7 @@ describe('React', () => { { () => makeContainer(() => ({}), () => new AwesomeMap(), () => ({})) } ); - }).toThrow(/bindActionCreators/); + }).toThrow(/bindDispatch/); expect(() => { TestUtils.renderIntoDocument(