From 82576ea25f72285dd29cfce08752e2b5e75bc6fe Mon Sep 17 00:00:00 2001 From: Brendon Pagano Date: Fri, 14 Dec 2018 05:35:10 -0200 Subject: [PATCH 1/6] Replace Component by PureComponent to enable shallow compare --- src/ConnectedRouter.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/ConnectedRouter.js b/src/ConnectedRouter.js index 90a39863..ce7ccb8b 100644 --- a/src/ConnectedRouter.js +++ b/src/ConnectedRouter.js @@ -1,4 +1,4 @@ -import React, { Component } from 'react' +import React, { PureComponent } from 'react' import PropTypes from 'prop-types' import { connect, ReactReduxContext } from 'react-redux' import { Router } from 'react-router' @@ -15,7 +15,7 @@ const createConnectedRouter = (structure) => { * This creates uni-directional flow from history->store->router->components. */ - class ConnectedRouter extends Component { + class ConnectedRouter extends PureComponent { constructor(props) { super(props) From a97b2048714d069d8d4abd5d16c194b83ead3532 Mon Sep 17 00:00:00 2001 From: Brendon Pagano Date: Sat, 15 Dec 2018 16:15:26 -0200 Subject: [PATCH 2/6] Add test to count the times rendered when mounted --- test/ConnectedRouter.test.js | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/test/ConnectedRouter.test.js b/test/ConnectedRouter.test.js index 6a613899..24a87808 100644 --- a/test/ConnectedRouter.test.js +++ b/test/ConnectedRouter.test.js @@ -123,6 +123,25 @@ describe('ConnectedRouter', () => { expect(onLocationChangedSpy.mock.calls).toHaveLength(3) }) + + it('only renders once when mounted', () => { + let renderCount = 0 + + const RenderCounter = () => { + renderCount++ + return null + } + + mount( + + + } /> + + + ) + + expect(renderCount).toBe(1) + }) }) describe('with immutable structure', () => { From e1bcf9970199d217d0e1b095ffea8c0ba64d4e38 Mon Sep 17 00:00:00 2001 From: Brendon Pagano Date: Sat, 15 Dec 2018 21:29:50 -0200 Subject: [PATCH 3/6] Create a test for non-related actions being fired --- test/ConnectedRouter.test.js | 47 ++++++++++++++++++++++++++++++++---- 1 file changed, 42 insertions(+), 5 deletions(-) diff --git a/test/ConnectedRouter.test.js b/test/ConnectedRouter.test.js index 24a87808..82a59d16 100644 --- a/test/ConnectedRouter.test.js +++ b/test/ConnectedRouter.test.js @@ -2,7 +2,7 @@ import 'raf/polyfill' import React, { Children, Component } from 'react' import PropTypes from 'prop-types' import configureStore from 'redux-mock-store' -import { createStore, combineReducers } from 'redux' +import { createStore, combineReducers, applyMiddleware, compose } from 'redux' import { ActionCreators, instrument } from 'redux-devtools' import Enzyme from 'enzyme' import Adapter from 'enzyme-adapter-react-16' @@ -14,7 +14,7 @@ import { onLocationChanged } from '../src/actions' import plainStructure from '../src/structure/plain' import immutableStructure from '../src/structure/immutable' import seamlessImmutableStructure from '../src/structure/seamless-immutable' -import { connectRouter, ConnectedRouter } from '../src' +import { connectRouter, ConnectedRouter, routerMiddleware } from '../src' Enzyme.configure({ adapter: new Adapter() }) @@ -124,7 +124,7 @@ describe('ConnectedRouter', () => { expect(onLocationChangedSpy.mock.calls).toHaveLength(3) }) - it('only renders once when mounted', () => { + it('only renders one time when mounted', () => { let renderCount = 0 const RenderCounter = () => { @@ -135,14 +135,51 @@ describe('ConnectedRouter', () => { mount( - } /> + ) expect(renderCount).toBe(1) }) - }) + + it('does not render again when non-related action is fired', () => { + // Initialize the render counter variable + let renderCount = 0 + + // Create redux store with router state + store = createStore( + combineReducers({ + incrementReducer: (state = 0, action = {}) => { + if (action.type === 'testAction') + return ++state + + return state + }, + router: connectRouter(history) + }), + compose(applyMiddleware(routerMiddleware(history))) + ) + + + const RenderCounter = () => { + renderCount++ + return null + } + + mount( + + + + + + ) + + store.dispatch({ type: 'testAction' }) + history.push('/new-location') + expect(renderCount).toBe(2) + }) + }) describe('with immutable structure', () => { let ConnectedRouter From 52363916484b6bbd8fa294eb4c9bfbedc9f23f69 Mon Sep 17 00:00:00 2001 From: Brendon Pagano Date: Sat, 15 Dec 2018 21:40:55 -0200 Subject: [PATCH 4/6] Keep the same prop usage from other tests --- test/ConnectedRouter.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/ConnectedRouter.test.js b/test/ConnectedRouter.test.js index 82a59d16..b32c34be 100644 --- a/test/ConnectedRouter.test.js +++ b/test/ConnectedRouter.test.js @@ -134,7 +134,7 @@ describe('ConnectedRouter', () => { mount( - + From 1131d796968e9d5b69a27de4e0314b806aa10b35 Mon Sep 17 00:00:00 2001 From: Brendon Pagano Date: Thu, 27 Dec 2018 11:03:50 -0200 Subject: [PATCH 5/6] Remove MockProvider and use redux's Provider instead --- test/ConnectedRouter.test.js | 86 +++++++++++------------------------- 1 file changed, 26 insertions(+), 60 deletions(-) diff --git a/test/ConnectedRouter.test.js b/test/ConnectedRouter.test.js index b32c34be..a2dabd8d 100644 --- a/test/ConnectedRouter.test.js +++ b/test/ConnectedRouter.test.js @@ -1,6 +1,5 @@ import 'raf/polyfill' -import React, { Children, Component } from 'react' -import PropTypes from 'prop-types' +import React from 'react' import configureStore from 'redux-mock-store' import { createStore, combineReducers, applyMiddleware, compose } from 'redux' import { ActionCreators, instrument } from 'redux-devtools' @@ -8,7 +7,7 @@ import Enzyme from 'enzyme' import Adapter from 'enzyme-adapter-react-16' import { createMemoryHistory } from 'history' import { Route } from 'react-router' -import { ReactReduxContext } from 'react-redux' +import { Provider } from 'react-redux' import createConnectedRouter from '../src/ConnectedRouter' import { onLocationChanged } from '../src/actions' import plainStructure from '../src/structure/plain' @@ -69,11 +68,11 @@ describe('ConnectedRouter', () => { it('calls `props.onLocationChanged()` when location changes.', () => { mount( - +
Home
} /> - + ) expect(onLocationChangedSpy.mock.calls).toHaveLength(1) @@ -86,11 +85,11 @@ describe('ConnectedRouter', () => { it('unlistens the history object when unmounted.', () => { const wrapper = mount( - +
Home
} /> - + ) expect(onLocationChangedSpy.mock.calls).toHaveLength(1) @@ -109,11 +108,11 @@ describe('ConnectedRouter', () => { it('supports custom context', () => { const context = React.createContext(null) mount( - +
Home
} /> - + ) expect(onLocationChangedSpy.mock.calls).toHaveLength(1) @@ -133,11 +132,11 @@ describe('ConnectedRouter', () => { } mount( - + - + ) expect(renderCount).toBe(1) @@ -168,11 +167,11 @@ describe('ConnectedRouter', () => { } mount( - + - + ) store.dispatch({ type: 'testAction' }) @@ -190,11 +189,11 @@ describe('ConnectedRouter', () => { it('calls `props.onLocationChanged()` when location changes.', () => { mount( - +
Home
} /> - + ) expect(onLocationChangedSpy.mock.calls).toHaveLength(1) @@ -207,11 +206,11 @@ describe('ConnectedRouter', () => { it('unlistens the history object when unmounted.', () => { const wrapper = mount( - +
Home
} /> - + ) expect(onLocationChangedSpy.mock.calls).toHaveLength(1) @@ -230,11 +229,11 @@ describe('ConnectedRouter', () => { it('supports custom context', () => { const context = React.createContext(null) mount( - +
Home
} /> - + ) expect(onLocationChangedSpy.mock.calls).toHaveLength(1) @@ -255,11 +254,11 @@ describe('ConnectedRouter', () => { it('calls `props.onLocationChanged()` when location changes.', () => { mount( - +
Home
} /> - + ) expect(onLocationChangedSpy.mock.calls).toHaveLength(1) @@ -272,11 +271,11 @@ describe('ConnectedRouter', () => { it('unlistens the history object when unmounted.', () => { const wrapper = mount( - +
Home
} /> - + ) expect(onLocationChangedSpy.mock.calls).toHaveLength(1) @@ -310,11 +309,11 @@ describe('ConnectedRouter', () => { it('resets to the initial url', () => { mount( - +
Test
 - + ) let currentPath @@ -332,11 +331,11 @@ describe('ConnectedRouter', () => { it('handles toggle after history change', () => { mount( - +
Test
 - + ) let currentPath @@ -356,36 +355,3 @@ describe('ConnectedRouter', () => { }) }) }) - -// MockProvider mocks react-redux's Provider component -class MockProvider extends Component { - constructor(props) { - super(props) - const { store } = props - this.state = { - storeState: store.getState(), - store, - } - } - render() { - const Context = this.props.context || ReactReduxContext - - return ( - - {Children.only(this.props.children)} - - ) - } -} - -const storeShape = PropTypes.shape({ - subscribe: PropTypes.func.isRequired, - dispatch: PropTypes.func.isRequired, - getState: PropTypes.func.isRequired, -}) - -MockProvider.propTypes = { - context: PropTypes.object, - store: storeShape.isRequired, - children: PropTypes.element.isRequired, -} From 7b22f4db24516adf1b284bdd84be37f6cfc9b3f2 Mon Sep 17 00:00:00 2001 From: Brendon Pagano Date: Thu, 27 Dec 2018 11:06:30 -0200 Subject: [PATCH 6/6] Remove location and action props from ConnectedRouter --- src/ConnectedRouter.js | 15 ++------------- 1 file changed, 2 insertions(+), 13 deletions(-) diff --git a/src/ConnectedRouter.js b/src/ConnectedRouter.js index ce7ccb8b..67d15635 100644 --- a/src/ConnectedRouter.js +++ b/src/ConnectedRouter.js @@ -6,8 +6,7 @@ import { onLocationChanged } from './actions' import createSelectors from './selectors' const createConnectedRouter = (structure) => { - const { getIn } = structure - const { getRouter, getLocation } = createSelectors(structure) + const { getLocation } = createSelectors(structure) /* * ConnectedRouter listens to a history object passed from props. * When history is changed, it dispatches action to redux store. @@ -90,21 +89,11 @@ const createConnectedRouter = (structure) => { location: PropTypes.object.isRequired, push: PropTypes.func.isRequired, }).isRequired, - location: PropTypes.oneOfType([ - PropTypes.object, - PropTypes.string, - ]).isRequired, - action: PropTypes.string.isRequired, basename: PropTypes.string, children: PropTypes.oneOfType([ PropTypes.func, PropTypes.node ]), onLocationChanged: PropTypes.func.isRequired, } - const mapStateToProps = state => ({ - action: getIn(getRouter(state), ['action']), - location: getIn(getRouter(state), ['location']), - }) - const mapDispatchToProps = dispatch => ({ onLocationChanged: (location, action) => dispatch(onLocationChanged(location, action)) }) @@ -127,7 +116,7 @@ const createConnectedRouter = (structure) => { context: PropTypes.object, } - return connect(mapStateToProps, mapDispatchToProps)(ConnectedRouterWithContext) + return connect(null, mapDispatchToProps)(ConnectedRouterWithContext) } export default createConnectedRouter