Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix location state (#394) #399

Merged
merged 7 commits into from
Mar 22, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,9 @@
"prop-types": "^15.7.2"
},
"peerDependencies": {
"immutable": "^3.8.1 || ^4.0.0-rc.1",
"history": "^4.7.2",
"immutable": "^3.8.1 || ^4.0.0-rc.1",
"lodash.isequalwith": "^4.4.0",
"react": "^16.4.0",
"react-redux": "^6.0.0 || ^7.1.0",
"react-router": "^4.3.1 || ^5.0.0",
Expand Down
6 changes: 4 additions & 2 deletions src/ConnectedRouter.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import React, { PureComponent } from 'react'
import PropTypes from 'prop-types'
import { connect, ReactReduxContext } from 'react-redux'
import { Router } from 'react-router'
import isEqualWith from 'lodash.isequalwith'
import { onLocationChanged } from './actions'
import createSelectors from './selectors'

Expand All @@ -18,7 +19,7 @@ const createConnectedRouter = (structure) => {
constructor(props) {
super(props)

const { store, history, onLocationChanged } = props
const { store, history, onLocationChanged, stateCompareFunction } = props

this.inTimeTravelling = false

Expand All @@ -45,7 +46,7 @@ const createConnectedRouter = (structure) => {
(pathnameInHistory !== pathnameInStore ||
searchInHistory !== searchInStore ||
hashInHistory !== hashInStore ||
stateInStore !== stateInHistory)
!isEqualWith(stateInStore, stateInHistory, stateCompareFunction))
) {
this.inTimeTravelling = true
// Update history's location to match store's location
Expand Down Expand Up @@ -109,6 +110,7 @@ const createConnectedRouter = (structure) => {
children: PropTypes.oneOfType([ PropTypes.func, PropTypes.node ]),
onLocationChanged: PropTypes.func.isRequired,
noInitialPop: PropTypes.bool,
stateCompareFunction: PropTypes.func,
}

const mapDispatchToProps = dispatch => ({
Expand Down
166 changes: 165 additions & 1 deletion test/ConnectedRouter.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import { createMemoryHistory } from 'history'
import { Route } from 'react-router'
import { Provider } from 'react-redux'
import createConnectedRouter from '../src/ConnectedRouter'
import { onLocationChanged } from '../src/actions'
import { onLocationChanged, LOCATION_CHANGE } from '../src/actions'
import plainStructure from '../src/structure/plain'
import immutableStructure from '../src/structure/immutable'
import seamlessImmutableStructure from '../src/structure/seamless-immutable'
Expand Down Expand Up @@ -136,6 +136,170 @@ describe('ConnectedRouter', () => {
expect(onLocationChangedSpy.mock.calls[1][0].state).toEqual({ foo: 'bar'})
})

it('updates history when store location state changes', () => {
store = createStore(
combineReducers({
router: connectRouter(props.history)
}),
compose(applyMiddleware(routerMiddleware(props.history)))
)

mount(
<Provider store={store}>
<ConnectedRouter {...props}>
<Route path="/" render={() => <div>Home</div>} />
</ConnectedRouter>
</Provider>
)

// Need to add PUSH action to history because initial POP action prevents history updates
props.history.push({ pathname: "/" })

store.dispatch({
type: LOCATION_CHANGE,
payload: {
location: {
pathname: '/',
search: '',
hash: '',
state: { foo: 'bar' }
},
action: 'PUSH',
}
})

expect(props.history.entries).toHaveLength(3)

store.dispatch({
type: LOCATION_CHANGE,
payload: {
location: {
pathname: '/',
search: '',
hash: '',
state: { foo: 'baz' }
},
action: 'PUSH',
}
})

expect(props.history.entries).toHaveLength(4)
})

it('does not update history when store location state is unchanged', () => {
store = createStore(
combineReducers({
router: connectRouter(props.history)
}),
compose(applyMiddleware(routerMiddleware(props.history)))
)

mount(
<Provider store={store}>
<ConnectedRouter {...props}>
<Route path="/" render={() => <div>Home</div>} />
</ConnectedRouter>
</Provider>
)

// Need to add PUSH action to history because initial POP action prevents history updates
props.history.push({ pathname: "/" })

store.dispatch({
type: LOCATION_CHANGE,
payload: {
location: {
pathname: '/',
search: '',
hash: '',
state: { foo: 'bar' }
},
action: 'PUSH',
}
})

expect(props.history.entries).toHaveLength(3)

store.dispatch({
type: LOCATION_CHANGE,
payload: {
location: {
pathname: '/',
search: '',
hash: '',
state: { foo: 'bar' }
},
action: 'PUSH',
}
})

expect(props.history.entries).toHaveLength(3)
})

it('supports custom location state compare function', () => {
store = createStore(
combineReducers({
router: connectRouter(props.history)
}),
compose(applyMiddleware(routerMiddleware(props.history)))
)

mount(
<Provider store={store}>
<ConnectedRouter
stateCompareFunction={(storeState, historyState) => {
// If the store and history states are not undefined,
// prevent history from updating when 'baz' is added to the store after 'bar'
if (storeState !== undefined && historyState !== undefined) {
if (storeState.foo === "baz" && historyState.foo === 'bar') {
return true
}
}

// Otherwise return a normal object comparison result
return JSON.stringify(storeState) === JSON.stringify(historyState)
}}
{...props}
>
<Route path="/" render={() => <div>Home</div>} />
</ConnectedRouter>
</Provider>
)

// Need to add PUSH action to history because initial POP action prevents history updates
props.history.push({ pathname: "/" })

store.dispatch({
type: LOCATION_CHANGE,
payload: {
location: {
pathname: '/',
search: '',
hash: '',
state: { foo: 'bar' }
},
action: 'PUSH',
}
})

expect(props.history.entries).toHaveLength(3)

store.dispatch({
type: LOCATION_CHANGE,
payload: {
location: {
pathname: '/',
search: '',
hash: '',
state: { foo: 'baz' }
},
action: 'PUSH',
}
})

expect(props.history.entries).toHaveLength(3)
})

it('only renders one time when mounted', () => {
let renderCount = 0

Expand Down
5 changes: 5 additions & 0 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -4733,6 +4733,11 @@ lodash.flattendeep@^4.4.0:
resolved "https://registry.yarnpkg.com/lodash.flattendeep/-/lodash.flattendeep-4.4.0.tgz#fb030917f86a3134e5bc9bec0d69e0013ddfedb2"
integrity sha1-+wMJF/hqMTTlvJvsDWngAT3f7bI=

lodash.isequalwith@^4.4.0:
version "4.4.0"
resolved "https://registry.yarnpkg.com/lodash.isequalwith/-/lodash.isequalwith-4.4.0.tgz#266726ddd528f854f21f4ea98a065606e0fbc6b0"
integrity sha1-Jmcm3dUo+FTyH06pigZWBuD7xrA=

lodash.isplainobject@^4.0.6:
version "4.0.6"
resolved "https://registry.yarnpkg.com/lodash.isplainobject/-/lodash.isplainobject-4.0.6.tgz#7c526a52d89b45c45cc690b88163be0497f550cb"
Expand Down