diff --git a/.gitignore b/.gitignore index 251f3eb95..827b359e3 100644 --- a/.gitignore +++ b/.gitignore @@ -12,6 +12,7 @@ test/react/*/test/**/*.spec.js test/react/**/src test/jest-config.json lcov.info +.idea/ lib/core/metadata.js lib/core/MetadataBlog.js diff --git a/.travis.yml b/.travis.yml index fc3568e1e..f3ac4f30a 100644 --- a/.travis.yml +++ b/.travis.yml @@ -2,9 +2,6 @@ language: node_js node_js: node cache: npm env: - - REACT=16.4 - - REACT=16.5 - - REACT=16.6 - REACT=16.8 script: - npm test diff --git a/docs/api/connect.md b/docs/api/connect.md index 39105ade6..4ad61e58c 100644 --- a/docs/api/connect.md +++ b/docs/api/connect.md @@ -121,7 +121,7 @@ The second parameter is normally referred to as `ownProps` by convention. ```js // binds on component re-rendering - - + - ) - } - } - - const childCalls = [] - @connect((state, parentProps) => { - childMapStateInvokes++ - childCalls.push([state, parentProps.parentState]) - // The state from parent props should always be consistent with the current state - expect(state).toEqual(parentProps.parentState) - return {} - }) - class ChildContainer extends Component { - render() { - return - } - } + + ) - const tester = rtl.render( - - - - ) + rtl.act(() => { + store.dispatch({ type: 'test' }) + }) - expect(childMapStateInvokes).toBe(1) - expect(childCalls).toEqual([['a', 'a']]) - - // The store state stays consistent when setState calls are batched - ReactDOM.unstable_batchedUpdates(() => { - store.dispatch({ type: 'APPEND', body: 'c' }) - }) - expect(childMapStateInvokes).toBe(2) - expect(childCalls).toEqual([['a', 'a'], ['ac', 'ac']]) - - // setState calls DOM handlers are batched - const button = tester.getByText('change') - rtl.fireEvent.click(button) - expect(childMapStateInvokes).toBe(3) - - store.dispatch({ type: 'APPEND', body: 'd' }) - expect(childMapStateInvokes).toBe(4) - expect(childCalls).toEqual([ - ['a', 'a'], - ['ac', 'ac'], - ['acb', 'acb'], - ['acbd', 'acbd'] - ]) - }) + expect(initialOwnProps).toBe(undefined) + expect(initialState).not.toBe(undefined) + expect(secondaryOwnProps).not.toBe(undefined) + expect(secondaryOwnProps.name).toBe('a') + }) - it('should not render the wrapped component when mapState does not produce change', () => { - const store = createStore(stringBuilder) - let renderCalls = 0 - let mapStateCalls = 0 + it('should allow providing a factory function to mapDispatchToProps', () => { + let updatedCount = 0 + let memoizedReturnCount = 0 + const store = createStore(() => ({ value: 1 })) + + const mapDispatchFactory = () => { + let lastProp, lastResult + return (dispatch, props) => { + if (props.name === lastProp) { + memoizedReturnCount++ + return lastResult + } + lastProp = props.name + return (lastResult = { someObject: { dispatchFn: dispatch } }) + } + } + function mergeParentDispatch(stateProps, dispatchProps, parentProps) { + return { ...stateProps, ...dispatchProps, name: parentProps.name } + } - @connect(() => { - mapStateCalls++ - return {} // no change! - }) - class Container extends Component { - render() { - renderCalls++ - return + @connect( + null, + mapDispatchFactory, + mergeParentDispatch + ) + class Passthrough extends Component { + componentDidUpdate() { + updatedCount++ + } + render() { + return
+ } } - } - rtl.render( - - - - ) + class Container extends Component { + constructor(props) { + super(props) + this.state = { count: 0 } + } + componentDidMount() { + this.setState({ count: 1 }) + } + render() { + const { count } = this.state + return ( +
+ + +
+ ) + } + } - expect(renderCalls).toBe(1) - expect(mapStateCalls).toBe(1) + rtl.render( + + + + ) - store.dispatch({ type: 'APPEND', body: 'a' }) + rtl.act(() => { + store.dispatch({ type: 'test' }) + }) - // After store a change mapState has been called - expect(mapStateCalls).toBe(2) - // But render is not because it did not make any actual changes - expect(renderCalls).toBe(1) + expect(updatedCount).toBe(0) + expect(memoizedReturnCount).toBe(2) + }) }) - it('should bail out early if mapState does not depend on props', () => { - const store = createStore(stringBuilder) - let renderCalls = 0 - let mapStateCalls = 0 + describe('Error handling for invalid arguments', () => { + function renderWithBadConnect(Component) { + const store = createStore(() => ({})) + const spy = jest.spyOn(console, 'error').mockImplementation(() => {}) - @connect(state => { - mapStateCalls++ - return state === 'aaa' ? { change: 1 } : {} - }) - class Container extends Component { - render() { - renderCalls++ - return + try { + rtl.render( + + + + ) + return null + } catch (error) { + return error.message + } finally { + spy.mockRestore() } } - rtl.render( - - - - ) + it('should throw a helpful error for invalid mapStateToProps arguments', () => { + @connect('invalid') + class InvalidMapState extends React.Component { + render() { + return
+ } + } + + const error = renderWithBadConnect(InvalidMapState) + expect(error).toContain('string') + expect(error).toContain('mapStateToProps') + expect(error).toContain('InvalidMapState') + }) - expect(renderCalls).toBe(1) - expect(mapStateCalls).toBe(1) + it('should throw a helpful error for invalid mapDispatchToProps arguments', () => { + @connect( + null, + 'invalid' + ) + class InvalidMapDispatch extends React.Component { + render() { + return
+ } + } - store.dispatch({ type: 'APPEND', body: 'a' }) - expect(mapStateCalls).toBe(2) - expect(renderCalls).toBe(1) + const error = renderWithBadConnect(InvalidMapDispatch) + expect(error).toContain('string') + expect(error).toContain('mapDispatchToProps') + expect(error).toContain('InvalidMapDispatch') + }) - store.dispatch({ type: 'APPEND', body: 'a' }) - expect(mapStateCalls).toBe(3) - expect(renderCalls).toBe(1) + it('should throw a helpful error for invalid mergeProps arguments', () => { + @connect( + null, + null, + 'invalid' + ) + class InvalidMerge extends React.Component { + render() { + return
+ } + } - store.dispatch({ type: 'APPEND', body: 'a' }) - expect(mapStateCalls).toBe(4) - expect(renderCalls).toBe(2) + const error = renderWithBadConnect(InvalidMerge) + expect(error).toContain('string') + expect(error).toContain('mergeProps') + expect(error).toContain('InvalidMerge') + }) }) - it('should not swallow errors when bailing out early', () => { - const spy = jest.spyOn(console, 'error').mockImplementation(() => {}) - const store = createStore(stringBuilder) - let renderCalls = 0 - let mapStateCalls = 0 - - @connect(state => { - mapStateCalls++ - if (state === 'a') { - throw new Error('Oops') - } else { - return {} + describe('Error handling for removed API options and StrictMode', () => { + it('should error on withRef=true', () => { + class Container extends Component { + render() { + return
hi
+ } } + expect(() => + connect( + undefined, + undefined, + undefined, + { withRef: true } + )(Container) + ).toThrow(/withRef is removed/) }) - class Container extends Component { - render() { - renderCalls++ - return - } - } - - rtl.render( - - - - ) - - expect(renderCalls).toBe(1) - expect(mapStateCalls).toBe(1) - expect(() => store.dispatch({ type: 'APPEND', body: 'a' })).toThrow() - spy.mockRestore() - }) + it('should error on receiving a custom store key', () => { + const connectOptions = { storeKey: 'customStoreKey' } - it('should allow providing a factory function to mapStateToProps', () => { - let updatedCount = 0 - let memoizedReturnCount = 0 - const store = createStore(() => ({ value: 1 })) - - const mapStateFactory = () => { - let lastProp, lastVal, lastResult - return (state, props) => { - if (props.name === lastProp && lastVal === state.value) { - memoizedReturnCount++ - return lastResult - } - lastProp = props.name - lastVal = state.value - return (lastResult = { - someObject: { prop: props.name, stateVal: state.value } - }) - } - } + expect(() => { + @connect( + undefined, + undefined, + undefined, + connectOptions + ) + class Container extends Component { + render() { + return + } + } + new Container() + }).toThrow(/storeKey has been removed/) + }) - @connect(mapStateFactory) - class Container extends Component { - componentDidUpdate() { - updatedCount++ + it.skip('should error on custom store', () => { + function Comp() { + return
hi
} - render() { - return + const Container = connect()(Comp) + function Oops() { + return } - } - - rtl.render( - -
- - -
- - ) + expect(() => { + rtl.render() + }).toThrow(/Passing redux store/) + }) - store.dispatch({ type: 'test' }) - expect(updatedCount).toBe(0) - expect(memoizedReturnCount).toBe(2) - }) + it('should error on renderCount prop if specified in connect options', () => { + function Comp(props) { + return
{props.count}
+ } + expect(() => { + connect( + undefined, + undefined, + undefined, + { renderCountProp: 'count' } + )(Comp) + }).toThrow(/renderCountProp is removed/) + }) - it('should allow a mapStateToProps factory consuming just state to return a function that gets ownProps', () => { - const store = createStore(() => ({ value: 1 })) - - let initialState - let initialOwnProps - let secondaryOwnProps - const mapStateFactory = function(factoryInitialState) { - initialState = factoryInitialState - initialOwnProps = arguments[1] - return (state, props) => { - secondaryOwnProps = props - return {} + it('works in without warnings (React 16.3+)', () => { + if (!React.StrictMode) { + return } - } + const spy = jest.spyOn(console, 'error').mockImplementation(() => {}) + const store = createStore(stringBuilder) - @connect(mapStateFactory) - class Container extends Component { - render() { - return + @connect(state => ({ string: state })) + class Container extends Component { + render() { + return + } } - } - rtl.render( - -
- -
- - ) + rtl.render( + + + + + + ) - store.dispatch({ type: 'test' }) - expect(initialOwnProps).toBe(undefined) - expect(initialState).not.toBe(undefined) - expect(secondaryOwnProps).not.toBe(undefined) - expect(secondaryOwnProps.name).toBe('a') + expect(spy).not.toHaveBeenCalled() + }) }) - it('should allow providing a factory function to mapDispatchToProps', () => { - let updatedCount = 0 - let memoizedReturnCount = 0 - const store = createStore(() => ({ value: 1 })) + describe('Subscription and update timing correctness', () => { + it('should pass state consistently to mapState', () => { + const store = createStore(stringBuilder) + + rtl.act(() => { + store.dispatch({ type: 'APPEND', body: 'a' }) + }) + + let childMapStateInvokes = 0 - const mapDispatchFactory = () => { - let lastProp, lastResult - return (dispatch, props) => { - if (props.name === lastProp) { - memoizedReturnCount++ - return lastResult + @connect(state => ({ state })) + class Container extends Component { + emitChange() { + store.dispatch({ type: 'APPEND', body: 'b' }) } - lastProp = props.name - return (lastResult = { someObject: { dispatchFn: dispatch } }) - } - } - function mergeParentDispatch(stateProps, dispatchProps, parentProps) { - return { ...stateProps, ...dispatchProps, name: parentProps.name } - } - @connect( - null, - mapDispatchFactory, - mergeParentDispatch - ) - class Passthrough extends Component { - componentDidUpdate() { - updatedCount++ - } - render() { - return
+ render() { + return ( +
+ + +
+ ) + } } - } - class Container extends Component { - constructor(props) { - super(props) - this.state = { count: 0 } - } - componentDidMount() { - this.setState({ count: 1 }) - } - render() { - const { count } = this.state - return ( -
- - -
- ) + const childCalls = [] + @connect((state, parentProps) => { + childMapStateInvokes++ + childCalls.push([state, parentProps.parentState]) + // The state from parent props should always be consistent with the current state + expect(state).toEqual(parentProps.parentState) + return {} + }) + class ChildContainer extends Component { + render() { + return + } } - } - - rtl.render( - - - - ) - store.dispatch({ type: 'test' }) - expect(updatedCount).toBe(0) - expect(memoizedReturnCount).toBe(2) - }) + const tester = rtl.render( + + + + ) - it('should not call update if mergeProps return value has not changed', () => { - let mapStateCalls = 0 - let renderCalls = 0 - const store = createStore(stringBuilder) + expect(childMapStateInvokes).toBe(1) + expect(childCalls).toEqual([['a', 'a']]) - @connect( - () => ({ a: ++mapStateCalls }), - null, - () => ({ changed: false }) - ) - class Container extends Component { - render() { - renderCalls++ - return - } - } + rtl.act(() => { + store.dispatch({ type: 'APPEND', body: 'c' }) + }) + expect(childMapStateInvokes).toBe(2) + expect(childCalls).toEqual([['a', 'a'], ['ac', 'ac']]) - rtl.render( - - - - ) + // setState calls DOM handlers are batched + const button = tester.getByText('change') + rtl.fireEvent.click(button) + expect(childMapStateInvokes).toBe(3) - expect(renderCalls).toBe(1) - expect(mapStateCalls).toBe(1) + rtl.act(() => { + store.dispatch({ type: 'APPEND', body: 'd' }) + }) - store.dispatch({ type: 'APPEND', body: 'a' }) + expect(childMapStateInvokes).toBe(4) + expect(childCalls).toEqual([ + ['a', 'a'], + ['ac', 'ac'], + ['acb', 'acb'], + ['acbd', 'acbd'] + ]) + }) - expect(mapStateCalls).toBe(2) - expect(renderCalls).toBe(1) - }) + it('should invoke mapState always with latest props', () => { + const store = createStore((state = 0) => state + 1) - it('should update impure components with custom mergeProps', () => { - let store = createStore(() => ({})) - let renderCount = 0 + let propsPassedIn - @connect( - null, - null, - () => ({ a: 1 }), - { pure: false } - ) - class Container extends React.Component { - render() { - ++renderCount - return
+ @connect(reduxCount => { + return { reduxCount } + }) + class InnerComponent extends Component { + render() { + propsPassedIn = this.props + return + } } - } - class Parent extends React.Component { - componentDidMount() { - this.forceUpdate() - } - render() { - return + class OuterComponent extends Component { + constructor() { + super() + this.state = { count: 0 } + } + + render() { + return + } } - } - rtl.render( - - - - - - ) + let outerComponent + rtl.render( + + (outerComponent = c)} /> + + ) + outerComponent.setState(({ count }) => ({ count: count + 1 })) + store.dispatch({ type: '' }) - expect(renderCount).toBe(2) - }) + expect(propsPassedIn.count).toEqual(1) + expect(propsPassedIn.reduxCount).toEqual(2) + }) - it('should allow to clean up child state in parent componentWillUnmount', () => { - function reducer(state = { data: null }, action) { - switch (action.type) { - case 'fetch': - return { data: { profile: { name: 'April' } } } - case 'clean': - return { data: null } - default: - return state - } - } + it('should use the latest props when updated between actions', () => { + const reactCallbackMiddleware = store => { + let callback - @connect(null) - class Parent extends React.Component { - componentWillUnmount() { - this.props.dispatch({ type: 'clean' }) - } + return next => action => { + if (action.type === 'SET_COMPONENT_CALLBACK') { + callback = action.payload + } + + if (callback && action.type === 'INC1') { + // Deliberately create multiple updates of different types in a row: + // 1) Store update causes subscriber notifications + next(action) + // 2) React setState outside batching causes a sync re-render. + // Because we're not using `act()`, this won't flush pending passive effects, + // simulating + callback() + // 3) Second dispatch causes subscriber notifications again. If `connect` is working + // correctly, nested subscriptions won't execute until the parents have rendered, + // to ensure that the subscriptions have access to the latest wrapper props. + store.dispatch({ type: 'INC2' }) + return + } - render() { - return + next(action) + } } - } - @connect(state => ({ - profile: state.data.profile - })) - class Child extends React.Component { - render() { - return null + const counter = (state = 0, action) => { + if (action.type === 'INC1') { + return state + 1 + } else if (action.type === 'INC2') { + return state + 2 + } + return state } - } - const store = createStore(reducer) - store.dispatch({ type: 'fetch' }) - const div = document.createElement('div') - ReactDOM.render( - - - , - div - ) + const store = createStore( + counter, + applyMiddleware(reactCallbackMiddleware) + ) - ReactDOM.unmountComponentAtNode(div) - }) + const Child = connect(count => ({ count }))(function(props) { + return ( +
+ ) + }) + class Parent extends Component { + constructor() { + super() + this.state = { + prop: 'a' + } + this.inc1 = () => store.dispatch({ type: 'INC1' }) + store.dispatch({ + type: 'SET_COMPONENT_CALLBACK', + payload: () => this.setState({ prop: 'b' }) + }) + } - it('should allow custom displayName', () => { - @connect( - null, - null, - null, - { getDisplayName: name => `Custom(${name})` } - ) - class MyComponent extends React.Component { - render() { - return
+ render() { + return ( + + + + ) + } } - } - expect(MyComponent.displayName).toEqual('Custom(MyComponent)') - }) + let parent + const rendered = rtl.render( (parent = ref)} />) + expect(rendered.getByTestId('child').dataset.count).toEqual('0') + expect(rendered.getByTestId('child').dataset.prop).toEqual('a') - it('should update impure components whenever the state of the store changes', () => { - const store = createStore(() => ({})) - let renderCount = 0 + // Force the multi-update sequence by running this bound action creator + parent.inc1() - @connect( - () => ({}), - null, - null, - { pure: false } - ) - class ImpureComponent extends React.Component { - render() { - ++renderCount - return
- } - } + // The connected child component _should_ have rendered with the latest Redux + // store value (3) _and_ the latest wrapper prop ('b'). + expect(rendered.getByTestId('child').dataset.count).toEqual('3') + expect(rendered.getByTestId('child').dataset.prop).toEqual('b') + }) - rtl.render( - - - - ) + it('should invoke mapState always with latest store state', () => { + const store = createStore((state = 0) => state + 1) - const rendersBeforeStateChange = renderCount - store.dispatch({ type: 'ACTION' }) - expect(renderCount).toBe(rendersBeforeStateChange + 1) - }) + let reduxCountPassedToMapState - function renderWithBadConnect(Component) { - const store = createStore(() => ({})) - const spy = jest.spyOn(console, 'error').mockImplementation(() => {}) + @connect(reduxCount => { + reduxCountPassedToMapState = reduxCount + return reduxCount < 2 ? { a: 'a' } : { a: 'b' } + }) + class InnerComponent extends Component { + render() { + return + } + } - try { + class OuterComponent extends Component { + constructor() { + super() + this.state = { count: 0 } + } + + render() { + return + } + } + + let outerComponent rtl.render( - + (outerComponent = c)} /> ) - return null - } catch (error) { - return error.message - } finally { - spy.mockRestore() - } - } - it('should throw a helpful error for invalid mapStateToProps arguments', () => { - @connect('invalid') - class InvalidMapState extends React.Component { - render() { - return
- } - } + store.dispatch({ type: '' }) + store.dispatch({ type: '' }) + outerComponent.setState(({ count }) => ({ count: count + 1 })) - const error = renderWithBadConnect(InvalidMapState) - expect(error).toContain('string') - expect(error).toContain('mapStateToProps') - expect(error).toContain('InvalidMapState') + expect(reduxCountPassedToMapState).toEqual(3) + }) }) - it('should throw a helpful error for invalid mapDispatchToProps arguments', () => { - @connect( - null, - 'invalid' - ) - class InvalidMapDispatch extends React.Component { - render() { - return
- } + it("should enforce top-down updates to ensure a deleted child's mapState doesn't throw errors", () => { + const initialState = { + a: { id: 'a', name: 'Item A' }, + b: { id: 'b', name: 'Item B' }, + c: { id: 'c', name: 'Item C' } } - const error = renderWithBadConnect(InvalidMapDispatch) - expect(error).toContain('string') - expect(error).toContain('mapDispatchToProps') - expect(error).toContain('InvalidMapDispatch') - }) - - it('should throw a helpful error for invalid mergeProps arguments', () => { - @connect( - null, - null, - 'invalid' - ) - class InvalidMerge extends React.Component { - render() { - return
+ const reducer = (state = initialState, action) => { + switch (action.type) { + case 'DELETE_B': { + const newState = { ...state } + delete newState.b + return newState + } + default: + return state } } - const error = renderWithBadConnect(InvalidMerge) - expect(error).toContain('string') - expect(error).toContain('mergeProps') - expect(error).toContain('InvalidMerge') - }) + const store = createStore(reducer) - it('should notify nested components through a blocking component', () => { - @connect(state => ({ count: state })) - class Parent extends Component { - render() { - return ( - - - - ) - } - } + const ListItem = ({ name }) =>
Name: {name}
- class BlockUpdates extends Component { - shouldComponentUpdate() { - return false - } - render() { - return this.props.children - } - } + let thrownError = null - const mapStateToProps = jest.fn(state => ({ count: state })) - @connect(mapStateToProps) - class Child extends Component { - render() { - return
{this.props.count}
+ const listItemMapState = (state, ownProps) => { + try { + const item = state[ownProps.id] + // If this line executes when item B has been deleted, it will throw an error. + // For this test to succeed, we should never execute mapState for item B after the item + // has been deleted, because the parent should re-render the component out of existence. + const { name } = item + return { name } + } catch (e) { + thrownError = e } } - const store = createStore((state = 0, action) => - action.type === 'INC' ? state + 1 : state - ) - rtl.render( - - - - ) - - expect(mapStateToProps).toHaveBeenCalledTimes(1) - store.dispatch({ type: 'INC' }) - expect(mapStateToProps).toHaveBeenCalledTimes(2) - }) + const ConnectedListItem = connect(listItemMapState)(ListItem) - it('should subscribe properly when a middle connected component does not subscribe', () => { - @connect(state => ({ count: state })) - class A extends React.Component { - render() { - return - } + const appMapState = state => { + const itemIds = Object.keys(state) + return { itemIds } } - @connect() // no mapStateToProps. therefore it should be transparent for subscriptions - class B extends React.Component { - render() { - return - } - } + function App({ itemIds }) { + const items = itemIds.map(id => ) - @connect((state, props) => { - expect(props.count).toBe(state) - return { count: state * 10 + props.count } - }) - class C extends React.Component { - render() { - return
{this.props.count}
- } + return ( +
+ {items} + +
+ ) } - const store = createStore((state = 0, action) => - action.type === 'INC' ? (state += 1) : state - ) + const ConnectedApp = connect(appMapState)(App) + rtl.render( - + ) - store.dispatch({ type: 'INC' }) + // This should execute without throwing an error by itself + rtl.act(() => { + store.dispatch({ type: 'DELETE_B' }) + }) + + expect(thrownError).toBe(null) }) - it('should subscribe properly when a new store is provided via props', () => { - const store1 = createStore((state = 0, action) => - action.type === 'INC' ? state + 1 : state - ) - const store2 = createStore((state = 0, action) => - action.type === 'INC' ? state + 1 : state - ) - const customContext = React.createContext() + it('should re-throw errors that occurred in a mapState/mapDispatch function', () => { + const counter = (state = 0, action) => + action.type === 'INCREMENT' ? state + 1 : state - @connect( - state => ({ count: state }), - undefined, - undefined, - { context: customContext } - ) - class A extends Component { - render() { - return - } - } + const store = createStore(counter) - const mapStateToPropsB = jest.fn(state => ({ count: state })) - @connect( - mapStateToPropsB, - undefined, - undefined, - { context: customContext } - ) - class B extends Component { - render() { - return + const appMapState = state => { + if (state >= 1) { + throw new Error('KABOOM!') } - } - const mapStateToPropsC = jest.fn(state => ({ count: state })) - @connect( - mapStateToPropsC, - undefined, - undefined, - { context: customContext } - ) - class C extends Component { - render() { - return - } + return { counter: state } } - const mapStateToPropsD = jest.fn(state => ({ count: state })) - @connect(mapStateToPropsD) - class D extends Component { - render() { - return
{this.props.count}
- } - } + const App = ({ counter }) =>
Count: {counter}
+ const ConnectedApp = connect(appMapState)(App) rtl.render( - - - - + + ) - expect(mapStateToPropsB).toHaveBeenCalledTimes(1) - expect(mapStateToPropsC).toHaveBeenCalledTimes(1) - expect(mapStateToPropsD).toHaveBeenCalledTimes(1) - - store1.dispatch({ type: 'INC' }) - expect(mapStateToPropsB).toHaveBeenCalledTimes(1) - expect(mapStateToPropsC).toHaveBeenCalledTimes(1) - expect(mapStateToPropsD).toHaveBeenCalledTimes(2) - - store2.dispatch({ type: 'INC' }) - expect(mapStateToPropsB).toHaveBeenCalledTimes(2) - expect(mapStateToPropsC).toHaveBeenCalledTimes(2) - expect(mapStateToPropsD).toHaveBeenCalledTimes(2) - }) - it('works in without warnings (React 16.3+)', () => { - if (!React.StrictMode) { - return - } + // Turn off extra console logging const spy = jest.spyOn(console, 'error').mockImplementation(() => {}) - const store = createStore(stringBuilder) - - @connect(state => ({ string: state })) - class Container extends Component { - render() { - return - } - } - - rtl.render( - - - - - - ) - - expect(spy).not.toHaveBeenCalled() - }) - - it('should error on withRef=true', () => { - class Container extends Component { - render() { - return
hi
- } - } - expect(() => - connect( - undefined, - undefined, - undefined, - { withRef: true } - )(Container) - ).toThrow(/withRef is removed/) - }) - - it('should error on receiving a custom store key', () => { - const connectOptions = { storeKey: 'customStoreKey' } - - expect(() => { - @connect( - undefined, - undefined, - undefined, - connectOptions - ) - class Container extends Component { - render() { - return - } - } - new Container() - }).toThrow(/storeKey has been removed/) - }) - - it('should error on custom store', () => { - function Comp() { - return
hi
- } - const Container = connect()(Comp) - function Oops() { - return - } - expect(() => { - rtl.render() - }).toThrow(/Passing redux store/) - }) - it('should error on renderCount prop if specified in connect options', () => { - function Comp(props) { - return
{props.count}
- } expect(() => { - connect( - undefined, - undefined, - undefined, - { renderCountProp: 'count' } - )(Comp) - }).toThrow(/renderCountProp is removed/) - }) - - it('should not error on valid component with circular structure', () => { - const createComp = Tag => { - const Comp = React.forwardRef(function Comp(props) { - return {props.count} + rtl.act(() => { + store.dispatch({ type: 'INCREMENT' }) }) - Comp.__real = Comp - return Comp - } + }).toThrow('KABOOM!') - expect(() => { - connect()(createComp('div')) - }).not.toThrow() + spy.mockRestore() }) }) }) diff --git a/test/components/connectAdvanced.spec.js b/test/components/connectAdvanced.spec.js index 6a47c8579..fcce46b55 100644 --- a/test/components/connectAdvanced.spec.js +++ b/test/components/connectAdvanced.spec.js @@ -6,7 +6,7 @@ import 'jest-dom/extend-expect' describe('React', () => { describe('connectAdvanced', () => { - it('should map state and render once on mount', () => { + it('should map state and render on mount', () => { const initialState = { foo: 'bar' } @@ -36,7 +36,10 @@ describe('React', () => { expect(tester.getByTestId('foo')).toHaveTextContent('bar') - expect(mapCount).toEqual(1) + // Implementation detail: + // 1) Initial render + // 2) Post-mount subscription and update check + expect(mapCount).toEqual(2) expect(renderCount).toEqual(1) }) @@ -67,10 +70,12 @@ describe('React', () => { ) - store.dispatch({ type: 'NEW_REFERENCE' }) + rtl.act(() => { + store.dispatch({ type: 'NEW_REFERENCE' }) + }) // Should have mapped the state on mount and on the dispatch - expect(mapCount).toEqual(2) + expect(mapCount).toEqual(3) // Should have rendered on mount and after the dispatch bacause the map // state returned new reference @@ -113,8 +118,11 @@ describe('React', () => { expect(tester.getByTestId('foo')).toHaveTextContent('bar') - // The state should have been mapped twice: on mount and on the dispatch - expect(mapCount).toEqual(2) + // The state should have been mapped 3 times: + // 1) Initial render + // 2) Post-mount update check + // 3) Dispatch + expect(mapCount).toEqual(3) // But the render should have been called only on mount since the map state // did not return a new reference @@ -172,8 +180,11 @@ describe('React', () => { outerComponent.setFoo('BAR') - // map on mount and on prop change - expect(mapCount).toEqual(2) + // The state should have been mapped 3 times: + // 1) Initial render + // 2) Post-mount update check + // 3) Prop change + expect(mapCount).toEqual(3) // render only on mount but skip on prop change because no new // reference was returned diff --git a/test/components/hooks.spec.js b/test/components/hooks.spec.js new file mode 100644 index 000000000..3da3521ce --- /dev/null +++ b/test/components/hooks.spec.js @@ -0,0 +1,122 @@ +/*eslint-disable react/prop-types*/ + +import React from 'react' +import { createStore } from 'redux' +import { Provider as ProviderMock, connect } from '../../src/index.js' +import * as rtl from 'react-testing-library' +import 'jest-dom/extend-expect' + +describe('React', () => { + describe('connect', () => { + afterEach(() => rtl.cleanup()) + + it('should render on useEffect hook state update', () => { + const store = createStore((state, action) => { + let newState = + state !== undefined + ? state + : { + byId: {}, + list: [] + } + switch (action.type) { + case 'FOO': + newState = { + ...newState, + list: [1], + byId: { 1: 'foo' } + } + break + } + return newState + }) + + const mapStateSpy1 = jest.fn() + const renderSpy1 = jest.fn() + + let component1StateList + + const component1Decorator = connect(state => { + mapStateSpy1() + + return { + list: state.list + } + }) + + const component1 = props => { + const [state, setState] = React.useState({ list: props.list }) + + component1StateList = state.list + + React.useEffect(() => { + setState(prevState => ({ ...prevState, list: props.list })) + }, [props.list]) + + renderSpy1() + + return + } + + const Component1 = component1Decorator(component1) + + const mapStateSpy2 = jest.fn() + const renderSpy2 = jest.fn() + + const component2Decorator = connect((state, ownProps) => { + mapStateSpy2() + + return { + mappedProp: ownProps.list.map(id => state.byId[id]) + } + }) + + const component2 = props => { + renderSpy2() + + expect(props.list).toBe(component1StateList) + + return
Hello
+ } + + const Component2 = component2Decorator(component2) + + rtl.render( + + + + ) + + // 1. Initial render + expect(mapStateSpy1).toHaveBeenCalledTimes(1) + + // 1.Initial render + // 2. C1 useEffect + expect(renderSpy1).toHaveBeenCalledTimes(2) + + // 1. Initial render + expect(mapStateSpy2).toHaveBeenCalledTimes(1) + + // 1. Initial render + expect(renderSpy2).toHaveBeenCalledTimes(1) + + rtl.act(() => { + store.dispatch({ type: 'FOO' }) + }) + + // 2. Store dispatch + expect(mapStateSpy1).toHaveBeenCalledTimes(2) + + // 3. Store dispatch + // 4. C1 useEffect + expect(renderSpy1).toHaveBeenCalledTimes(4) + + // 2. Connect(C2) subscriber + // 3. Ignored prev child props in re-render and re-runs mapState + expect(mapStateSpy2).toHaveBeenCalledTimes(3) + + // 2. Batched update from nested subscriber / C1 re-render + expect(renderSpy2).toHaveBeenCalledTimes(2) + }) + }) +}) diff --git a/test/integration/dynamic-reducers.spec.js b/test/integration/dynamic-reducers.spec.js new file mode 100644 index 000000000..dc2e3561b --- /dev/null +++ b/test/integration/dynamic-reducers.spec.js @@ -0,0 +1,165 @@ +/*eslint-disable react/prop-types*/ + +import React from 'react' +import ReactDOMServer from 'react-dom/server' +import { createStore, combineReducers } from 'redux' +import { connect, Provider, ReactReduxContext } from '../../src/index.js' +import * as rtl from 'react-testing-library' + +describe('React', () => { + /* + For SSR to work, there are three options for injecting + dynamic reducers: + + 1. Make sure all dynamic reducers are known before rendering + (requires keeping knowledge about this outside of the + React component-tree) + 2. Double rendering (first render injects required reducers) + 3. Inject reducers as a side effect during the render phase + (in construct or render), and try to control for any + issues with that. This requires grabbing the store from + context and possibly patching any storeState that exists + on there, these are undocumented APIs that might change + at any time. + + Because the tradeoffs in 1 and 2 are quite hefty and also + because it's the popular approach, this test targets nr 3. + */ + describe('dynamic reducers', () => { + const InjectReducersContext = React.createContext(null) + + function ExtraReducersProvider({ children, reducers }) { + return ( + + {injectReducers => ( + + {reduxContext => { + const latestState = reduxContext.store.getState() + const contextState = reduxContext.storeState + + let shouldInject = false + let shouldPatch = false + + for (const key of Object.keys(reducers)) { + // If any key does not exist in the latest version + // of the state, we need to inject reducers + if (!(key in latestState)) { + shouldInject = true + } + // If state exists on the context, and if any reducer + // key is not included there, we need to patch it up + // Only patching if storeState exists makes this test + // work with multiple React-Redux approaches + if (contextState && !(key in contextState)) { + shouldPatch = true + } + } + + if (shouldInject) { + injectReducers(reducers) + } + + if (shouldPatch) { + // A safer way to do this would be to patch the storeState + // manually with the state from the new reducers, since + // this would better avoid tearing in a future concurrent world + const patchedReduxContext = { + ...reduxContext, + storeState: reduxContext.store.getState() + } + return ( + + {children} + + ) + } + + return children + }} + + )} + + ) + } + + const initialReducer = { + initial: (state = { greeting: 'Hello world' }) => state + } + const dynamicReducer = { + dynamic: (state = { greeting: 'Hello dynamic world' }) => state + } + + function Greeter({ greeting }) { + return
{greeting}
+ } + + const InitialGreeting = connect(state => ({ + greeting: state.initial.greeting + }))(Greeter) + const DynamicGreeting = connect(state => ({ + greeting: state.dynamic.greeting + }))(Greeter) + + function createInjectReducers(store, initialReducer) { + let reducers = initialReducer + return function injectReducers(newReducers) { + reducers = { ...reducers, ...newReducers } + store.replaceReducer(combineReducers(reducers)) + } + } + + let store + let injectReducers + + beforeEach(() => { + // These could be singletons on the client, but + // need to be separate per request on the server + store = createStore(combineReducers(initialReducer)) + injectReducers = createInjectReducers(store, initialReducer) + }) + + it('should render child with initial state on the client', () => { + const { getByText } = rtl.render( + + + + + + + + + ) + + getByText('Hello world') + getByText('Hello dynamic world') + }) + it('should render child with initial state on the server', () => { + // In order to keep these tests together in the same file, + // we aren't currently rendering this test in the node test + // environment + // This generates errors for using useLayoutEffect in v7 + // We hide that error by disabling console.error here + + jest.spyOn(console, 'error') + // eslint-disable-next-line no-console + console.error.mockImplementation(() => {}) + + const markup = ReactDOMServer.renderToString( + + + + + + + + + ) + + expect(markup).toContain('Hello world') + expect(markup).toContain('Hello dynamic world') + + // eslint-disable-next-line no-console + console.error.mockRestore() + }) + }) +}) diff --git a/test/integration/server-rendering.spec.js b/test/integration/server-rendering.spec.js index 6bd703678..dbf172241 100644 --- a/test/integration/server-rendering.spec.js +++ b/test/integration/server-rendering.spec.js @@ -1,3 +1,11 @@ +/** + * @jest-environment node + * + * Set this so that `window` is undefined to correctly mimic a Node SSR scenario. + * That allows connectAdvanced to fall back to `useEffect` instead of `useLayoutEffect` + * to avoid ugly console warnings when used with SSR. + */ + /*eslint-disable react/prop-types*/ import React from 'react' @@ -20,6 +28,28 @@ describe('React', () => {
) + class Dispatcher extends React.Component { + constructor(props) { + super(props) + if (props.constructAction) { + props.dispatch(props.constructAction) + } + } + UNSAFE_componentWillMount() { + if (this.props.willMountAction) { + this.props.dispatch(this.props.willMountAction) + } + } + render() { + if (this.props.renderAction) { + this.props.dispatch(this.props.renderAction) + } + + return + } + } + const ConnectedDispatcher = connect()(Dispatcher) + it('should be able to render connected component with props and state from store', () => { const store = createStore(greetingReducer) @@ -32,6 +62,22 @@ describe('React', () => { expect(markup).toContain('Hello world') }) + it('should run in an SSR environment without logging warnings about useLayoutEffect', () => { + const store = createStore(greetingReducer) + + const spy = jest.spyOn(console, 'error').mockImplementation(() => {}) + + const markup = renderToString( + + + + ) + + expect(spy).toHaveBeenCalledTimes(0) + + spy.mockRestore() + }) + it('should render with updated state if actions are dispatched before render', () => { const store = createStore(greetingReducer) @@ -47,42 +93,43 @@ describe('React', () => { expect(store.getState().greeting).toContain('Hi') }) - it('should render children with original state even if actions are dispatched in ancestor', () => { + it('should render children with updated state if actions are dispatched in ancestor', () => { /* Dispatching during construct, render or willMount is almost always a bug with SSR (or otherwise) This behaviour is undocumented and is likely to change between implementations, this test only verifies current behaviour + + Note: this test fails in v6, because we use context to propagate the store state, and the entire + tree will see the same state during the render pass. + In all other versions, including v7, the store state may change as actions are dispatched + during lifecycle methods, and components will see that new state immediately as they read it. */ const store = createStore(greetingReducer) - class Dispatcher extends React.Component { - constructor(props) { - super(props) - props.dispatch(props.action) - } - UNSAFE_componentWillMount() { - this.props.dispatch(this.props.action) - } - render() { - this.props.dispatch(this.props.action) - - return - } - } - const ConnectedDispatcher = connect()(Dispatcher) - - const action = { type: 'Update', payload: { greeting: 'Hi' } } + const constructAction = { type: 'Update', payload: { greeting: 'Hi' } } + const willMountAction = { type: 'Update', payload: { greeting: 'Hiya' } } + const renderAction = { type: 'Update', payload: { greeting: 'Hey' } } const markup = renderToString( - + + + ) - expect(markup).toContain('Hello world') - expect(store.getState().greeting).toContain('Hi') + expect(markup).toContain('Hi world') + expect(markup).toContain('Hiya world') + expect(markup).toContain('Hey world') + expect(store.getState().greeting).toContain('Hey') }) it('should render children with changed state if actions are dispatched in ancestor and new Provider wraps children', () => { @@ -92,35 +139,12 @@ describe('React', () => { This behaviour is undocumented and is likely to change between implementations, this test only verifies current behaviour + + This test works both when state is fetched directly in connected + components and when it is fetched in a Provider and placed on context */ const store = createStore(greetingReducer) - class Dispatcher extends React.Component { - constructor(props) { - super(props) - if (props.constructAction) { - props.dispatch(props.constructAction) - } - } - UNSAFE_componentWillMount() { - if (this.props.willMountAction) { - this.props.dispatch(this.props.willMountAction) - } - } - render() { - if (this.props.renderAction) { - this.props.dispatch(this.props.renderAction) - } - - return ( - - - - ) - } - } - const ConnectedDispatcher = connect()(Dispatcher) - const constructAction = { type: 'Update', payload: { greeting: 'Hi' } } const willMountAction = { type: 'Update', payload: { greeting: 'Hiya' } } const renderAction = { type: 'Update', payload: { greeting: 'Hey' } } diff --git a/test/react/16.8/package-lock.json b/test/react/16.8/package-lock.json index df461400e..5e00a715c 100644 --- a/test/react/16.8/package-lock.json +++ b/test/react/16.8/package-lock.json @@ -293,33 +293,33 @@ } }, "react": { - "version": "16.8.2", - "resolved": "https://registry.npmjs.org/react/-/react-16.8.2.tgz", - "integrity": "sha512-aB2ctx9uQ9vo09HVknqv3DGRpI7OIGJhCx3Bt0QqoRluEjHSaObJl+nG12GDdYH6sTgE7YiPJ6ZUyMx9kICdXw==", + "version": "16.8.4", + "resolved": "https://registry.npmjs.org/react/-/react-16.8.4.tgz", + "integrity": "sha512-0GQ6gFXfUH7aZcjGVymlPOASTuSjlQL4ZtVC5YKH+3JL6bBLCVO21DknzmaPlI90LN253ojj02nsapy+j7wIjg==", "dev": true, "requires": { "loose-envify": "^1.1.0", "object-assign": "^4.1.1", "prop-types": "^15.6.2", - "scheduler": "^0.13.2" + "scheduler": "^0.13.4" } }, "react-dom": { - "version": "16.8.2", - "resolved": "https://registry.npmjs.org/react-dom/-/react-dom-16.8.2.tgz", - "integrity": "sha512-cPGfgFfwi+VCZjk73buu14pYkYBR1b/SRMSYqkLDdhSEHnSwcuYTPu6/Bh6ZphJFIk80XLvbSe2azfcRzNF+Xg==", + "version": "16.8.4", + "resolved": "https://registry.npmjs.org/react-dom/-/react-dom-16.8.4.tgz", + "integrity": "sha512-Ob2wK7XG2tUDt7ps7LtLzGYYB6DXMCLj0G5fO6WeEICtT4/HdpOi7W/xLzZnR6RCG1tYza60nMdqtxzA8FaPJQ==", "dev": true, "requires": { "loose-envify": "^1.1.0", "object-assign": "^4.1.1", "prop-types": "^15.6.2", - "scheduler": "^0.13.2" + "scheduler": "^0.13.4" } }, "react-is": { - "version": "16.8.2", - "resolved": "https://registry.npmjs.org/react-is/-/react-is-16.8.2.tgz", - "integrity": "sha512-D+NxhSR2HUCjYky1q1DwpNUD44cDpUXzSmmFyC3ug1bClcU/iDNy0YNn1iwme28fn+NFhpA13IndOd42CrFb+Q==", + "version": "16.8.4", + "resolved": "https://registry.npmjs.org/react-is/-/react-is-16.8.4.tgz", + "integrity": "sha512-PVadd+WaUDOAciICm/J1waJaSvgq+4rHE/K70j0PFqKhkTBsPv/82UGQJNXAngz1fOQLLxI6z1sEDmJDQhCTAA==", "dev": true }, "react-testing-library": { @@ -357,9 +357,9 @@ "dev": true }, "scheduler": { - "version": "0.13.2", - "resolved": "https://registry.npmjs.org/scheduler/-/scheduler-0.13.2.tgz", - "integrity": "sha512-qK5P8tHS7vdEMCW5IPyt8v9MJOHqTrOUgPXib7tqm9vh834ibBX5BNhwkplX/0iOzHW5sXyluehYfS9yrkz9+w==", + "version": "0.13.4", + "resolved": "https://registry.npmjs.org/scheduler/-/scheduler-0.13.4.tgz", + "integrity": "sha512-cvSOlRPxOHs5dAhP9yiS/6IDmVAVxmk33f0CtTJRkmUWcb1Us+t7b1wqdzoC0REw2muC9V5f1L/w5R5uKGaepA==", "dev": true, "requires": { "loose-envify": "^1.1.0", diff --git a/test/react/16.8/package.json b/test/react/16.8/package.json index 301f12c15..f1cc380f6 100644 --- a/test/react/16.8/package.json +++ b/test/react/16.8/package.json @@ -2,8 +2,8 @@ "private": true, "devDependencies": { "create-react-class": "^15.6.3", - "react": "16.8", - "react-dom": "16.8" + "react": "^16.8.4", + "react-dom": "^16.8.4" }, "dependencies": { "jest-dom": "^3.1.2", diff --git a/test/run-tests.js b/test/run-tests.js index 9830503b5..f91da18a9 100644 --- a/test/run-tests.js +++ b/test/run-tests.js @@ -1,7 +1,7 @@ const npmRun = require('npm-run') const fs = require('fs') const path = require('path') -const LATEST_VERSION = '16.6' +const LATEST_VERSION = '16.8' const version = process.env.REACT || LATEST_VERSION let jestConfig = { @@ -33,9 +33,8 @@ const configFilePath = path.join(__dirname, 'jest-config.json') fs.writeFileSync(configFilePath, JSON.stringify(jestConfig)) -const commandLine = `jest -c "${configFilePath}" ${process.argv.slice(2).join(' ')}` +const commandLine = `jest -c "${configFilePath}" ${process.argv + .slice(2) + .join(' ')}` -npmRun.execSync( - commandLine, - { stdio: 'inherit' } -) +npmRun.execSync(commandLine, { stdio: 'inherit' })