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

Prevent double-rendering on initialization #218

Merged
merged 1 commit into from Dec 27, 2018
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 4 additions & 2 deletions src/ConnectedRouter.js
Expand Up @@ -58,8 +58,10 @@ const createConnectedRouter = (structure) => {

// Listen to history changes
this.unlisten = history.listen(handleLocationChange)
// Dispatch a location change action for the initial location
handleLocationChange(history.location, history.action)
// Dispatch a location change action for the initial location.
// This makes it backward-compatible with react-router-redux.
// But, we add `isFirstRendering` to `true` to prevent double-rendering.
handleLocationChange(history.location, history.action, true)
}

componentWillUnmount() {
Expand Down
3 changes: 2 additions & 1 deletion src/actions.js
Expand Up @@ -4,11 +4,12 @@
*/
export const LOCATION_CHANGE = '@@router/LOCATION_CHANGE'

export const onLocationChanged = (location, action) => ({
export const onLocationChanged = (location, action, isFirstRendering = false) => ({
type: LOCATION_CHANGE,
payload: {
location,
action,
isFirstRendering,
}
})

Expand Down
7 changes: 6 additions & 1 deletion src/reducer.js
Expand Up @@ -18,7 +18,12 @@ const createConnectRouter = (structure) => {
*/
return (state = initialRouterState, { type, payload } = {}) => {
if (type === LOCATION_CHANGE) {
return merge(state, payload)
const { location, action, isFirstRendering } = payload
// Don't update the state ref for the first rendering
// to prevent the double-rendering issue on initilization
return isFirstRendering
? state
: merge(state, { location, action })
}

return state
Expand Down
18 changes: 18 additions & 0 deletions test/actions.test.js
Expand Up @@ -21,6 +21,24 @@ describe('Actions', () => {
hash: '',
},
action: 'POP',
isFirstRendering: false,
},
}
expect(actualAction).toEqual(expectedAction)
})

it('returns correct action when calling onLocationChanged() for the first rendering', () => {
const actualAction = onLocationChanged({ pathname: '/', search: '', hash: '' }, 'POP', true)
const expectedAction = {
type: LOCATION_CHANGE,
payload: {
location: {
pathname: '/',
search: '',
hash: '',
},
action: 'POP',
isFirstRendering: true,
},
}
expect(actualAction).toEqual(expectedAction)
Expand Down
92 changes: 91 additions & 1 deletion test/reducer.test.js
Expand Up @@ -92,6 +92,36 @@ describe('connectRouter', () => {
const nextState = rootReducer(currentState, action)
expect(nextState).toBe(currentState)
})

it('does not change state ref when receiving LOCATION_CHANGE for the first rendering', () => {
const rootReducer = combineReducers({
router: connectRouter(mockHistory)
})
const currentState = {
router: {
location: {
pathname: '/',
search: '',
hash: '',
},
action: 'POP',
},
}
const action = {
type: LOCATION_CHANGE,
payload: {
location: {
pathname: '/',
search: '',
hash: '',
},
action: 'POP',
isFirstRendering: true,
}
}
const nextState = rootReducer(currentState, action)
expect(nextState).toBe(currentState)
})
})

describe('with immutable structure', () => {
Expand Down Expand Up @@ -143,6 +173,36 @@ describe('connectRouter', () => {
})
expect(nextState).toEqual(expectedState)
})

it('does not change state ref when receiving LOCATION_CHANGE for the first rendering', () => {
const rootReducer = combineReducers({
router: connectRouter(mockHistory)
})
const currentState = {
router: {
location: {
pathname: '/',
search: '',
hash: '',
},
action: 'POP',
},
}
const action = {
type: LOCATION_CHANGE,
payload: {
location: {
pathname: '/',
search: '',
hash: '',
},
action: 'POP',
isFirstRendering: true,
}
}
const nextState = rootReducer(currentState, action)
expect(nextState).toBe(currentState)
})
})

describe('with seamless immutable structure', () => {
Expand Down Expand Up @@ -194,5 +254,35 @@ describe('connectRouter', () => {
}
expect(nextState).toEqual(expectedState)
})

it('does not change state ref when receiving LOCATION_CHANGE for the first rendering', () => {
const rootReducer = combineReducers({
router: connectRouter(mockHistory)
})
const currentState = {
router: {
location: {
pathname: '/',
search: '',
hash: '',
},
action: 'POP',
},
}
const action = {
type: LOCATION_CHANGE,
payload: {
location: {
pathname: '/',
search: '',
hash: '',
},
action: 'POP',
isFirstRendering: true,
}
}
const nextState = rootReducer(currentState, action)
expect(nextState).toBe(currentState)
})
})
})
})