Skip to content

Commit a47914f

Browse files
gladtocodegladtocode
committed
Rework syncer so that redux is the source of truth, driving
the router, and staying in sync with the browser. In order to make redux the source of truth, it needs to intercept react-router's pushes. The chosen implementation provides a wrapped memoryHistory object to react-router. As a result, we have a nice interception point, which as a side-effect allows us to remove the avoidRouterUpdate boolean in the action payload.
1 parent a668989 commit a47914f

File tree

4 files changed

+78
-94
lines changed

4 files changed

+78
-94
lines changed

‎examples/basic/app.js

+2-2
Original file line numberDiff line numberDiff line change
@@ -20,12 +20,12 @@ const finalCreateStore = compose(
2020
const store = finalCreateStore(reducer);
2121
const history = createHistory();
2222

23-
syncReduxAndRouter(history, store);
23+
const reduxRouterHistory = syncReduxAndRouter(history, store);
2424

2525
ReactDOM.render(
2626
<Provider store={store}>
2727
<div>
28-
<Router history={history}>
28+
<Router history={reduxRouterHistory}>
2929
<Route path="/" component={App}>
3030
<IndexRoute component={Home}/>
3131
<Route path="foo" component={Foo}/>

‎examples/basic/package.json

+3-3
Original file line numberDiff line numberDiff line change
@@ -2,13 +2,13 @@
22
"name": "rsr-basic-example",
33
"version": "0.0.0",
44
"dependencies": {
5-
"history": "^1.14.0",
5+
"history": "1.17.0",
66
"react": "^0.14.2",
77
"react-dom": "^0.14.2",
88
"react-redux": "^4.0.0",
9-
"react-router": "^1.0.0",
9+
"react-router": "1.0.3",
1010
"redux": "^3.0.4",
11-
"redux-simple-router": "0.0.8"
11+
"redux-simple-router": "0.0.8",
1212
},
1313
"devDependencies": {
1414
"babel-core": "^6.1.21",

‎src/index.js

+57-54
Original file line numberDiff line numberDiff line change
@@ -1,30 +1,28 @@
1-
import deepEqual from 'deep-equal'
1+
import { createMemoryHistory } from 'history';
22

33
// Constants
44

55
export const UPDATE_PATH = '@@router/UPDATE_PATH'
66
const SELECT_STATE = state => state.routing
77

8-
export function pushPath(path, state, { avoidRouterUpdate = false } = {}) {
8+
export function pushPath(path, state) {
99
return {
1010
type: UPDATE_PATH,
1111
payload: {
1212
path: path,
1313
state: state,
1414
replace: false,
15-
avoidRouterUpdate: !!avoidRouterUpdate
1615
}
1716
}
1817
}
1918

20-
export function replacePath(path, state, { avoidRouterUpdate = false } = {}) {
19+
export function replacePath(path, state) {
2120
return {
2221
type: UPDATE_PATH,
2322
payload: {
2423
path: path,
2524
state: state,
2625
replace: true,
27-
avoidRouterUpdate: !!avoidRouterUpdate
2826
}
2927
}
3028
}
@@ -42,7 +40,7 @@ function update(state=initialState, { type, payload }) {
4240
if(type === UPDATE_PATH) {
4341
return Object.assign({}, state, {
4442
path: payload.path,
45-
changeId: state.changeId + (payload.avoidRouterUpdate ? 0 : 1),
43+
changeId: state.changeId + 1,
4644
state: payload.state,
4745
replace: payload.replace
4846
})
@@ -52,10 +50,6 @@ function update(state=initialState, { type, payload }) {
5250

5351
// Syncing
5452

55-
function locationsAreEqual(a, b) {
56-
return a != null && b != null && a.path === b.path && deepEqual(a.state, b.state)
57-
}
58-
5953
function createPath(location) {
6054
const { pathname, search, hash } = location
6155
let result = pathname
@@ -66,32 +60,53 @@ function createPath(location) {
6660
return result
6761
}
6862

69-
export function syncReduxAndRouter(history, store, selectRouterState = SELECT_STATE) {
63+
export function syncReduxAndRouter(browserHistory, store, selectRouterState = SELECT_STATE) {
7064
const getRouterState = () => selectRouterState(store.getState())
71-
72-
// To properly handle store updates we need to track the last route.
73-
// This route contains a `changeId` which is updated on every
74-
// `pushPath` and `replacePath`. If this id changes we always
75-
// trigger a history update. However, if the id does not change, we
76-
// check if the location has changed, and if it is we trigger a
77-
// history update. It's possible for this to happen when something
78-
// reloads the entire app state such as redux devtools.
79-
let lastRoute = undefined
80-
8165
if(!getRouterState()) {
8266
throw new Error(
8367
'Cannot sync router: route state does not exist (`state.routing` by default). ' +
8468
'Did you install the routing reducer?'
8569
)
8670
}
8771

88-
const unsubscribeHistory = history.listen(location => {
72+
const memoryHistory = createMemoryHistory();
73+
74+
let lastId = null;
75+
let storeListeningToBrowser = true;
76+
let browserListeningToStore = true;
77+
78+
const unsubscribeStore = store.subscribe(() => {
79+
const routing = getRouterState();
80+
81+
if(lastId !== routing.changeId) {
82+
lastId = routing.changeId;
83+
84+
const method = routing.replace ? 'replace' : 'push'
85+
console.log("sending to memoryHistory from store subscriber");
86+
memoryHistory[method]({
87+
pathname: routing.path,
88+
state: routing.state
89+
});
90+
91+
if(browserListeningToStore) {
92+
console.log("pushing to browserHistory from store subscriber");
93+
storeListeningToBrowser = false;
94+
browserHistory[method]({
95+
pathname: routing.path,
96+
state: routing.state
97+
});
98+
storeListeningToBrowser = true;
99+
}
100+
}
101+
});
102+
103+
const unsubscribeHistory = browserHistory.listen((location) => {
89104
const route = {
90105
path: createPath(location),
91106
state: location.state
92107
}
93108

94-
if (!lastRoute) {
109+
if(!lastId) {
95110
// `initialState` *should* represent the current location when
96111
// the app loads, but we cannot get the current location when it
97112
// is defined. What happens is `history.listen` is called
@@ -108,42 +123,30 @@ export function syncReduxAndRouter(history, store, selectRouterState = SELECT_ST
108123
state: route.state,
109124
replace: false
110125
}
111-
112-
// Also set `lastRoute` so that the store subscriber doesn't
113-
// trigger an unnecessary `pushState` on load
114-
lastRoute = initialState
115-
116-
store.dispatch(pushPath(route.path, route.state, { avoidRouterUpdate: true }));
117-
} else if(!locationsAreEqual(getRouterState(), route)) {
118-
// The above check avoids dispatching an action if the store is
119-
// already up-to-date
126+
}
127+
if(storeListeningToBrowser) {
128+
console.log("got new location in browserHistory listener, sending to store")
129+
browserListeningToStore = false;
120130
const method = location.action === 'REPLACE' ? replacePath : pushPath
121-
store.dispatch(method(route.path, route.state, { avoidRouterUpdate: true }))
131+
store.dispatch(method(route.path, route.state));
132+
browserListeningToStore = true;
122133
}
123-
})
124-
125-
const unsubscribeStore = store.subscribe(() => {
126-
let routing = getRouterState()
127-
128-
// Only trigger history update if this is a new change or the
129-
// location has changed.
130-
if(lastRoute.changeId !== routing.changeId ||
131-
!locationsAreEqual(lastRoute, routing)) {
132-
133-
lastRoute = routing
134-
const method = routing.replace ? 'replace' : 'push'
135-
history[method]({
136-
pathname: routing.path,
137-
state: routing.state
138-
})
134+
});
135+
136+
const reduxRouterHistory = Object.assign({}, memoryHistory, {
137+
unsubscribe: function() {
138+
unsubscribeStore();
139+
unsubscribeHistory();
140+
},
141+
push: function(location) {
142+
console.log("Got push in reduxRouterHistory (i.e. <Link/>). sending to store");
143+
const method = location.action === 'REPLACE' ? replacePath : pushPath
144+
store.dispatch(method(createPath(location), location.state));
139145
}
146+
});
140147

141-
})
148+
return reduxRouterHistory;
142149

143-
return function unsubscribe() {
144-
unsubscribeHistory()
145-
unsubscribeStore()
146-
}
147150
}
148151

149152
export { update as routeReducer }

‎test/createTests.js

+16-35
Original file line numberDiff line numberDiff line change
@@ -31,8 +31,8 @@ function createSyncedHistoryAndStore(createHistory) {
3131
routing: routeReducer
3232
}))
3333
const history = createHistory()
34-
const unsubscribe = syncReduxAndRouter(history, store)
35-
return { history, store, unsubscribe }
34+
const reduxRouterHistory = syncReduxAndRouter(history, store)
35+
return { history, store, reduxRouterHistory }
3636
}
3737

3838
const defaultReset = () => {}
@@ -49,18 +49,16 @@ module.exports = function createTests(createHistory, name, reset = defaultReset)
4949
payload: {
5050
path: '/foo',
5151
replace: false,
52-
state: { bar: 'baz' },
53-
avoidRouterUpdate: false
52+
state: { bar: 'baz' }
5453
}
5554
})
5655

57-
expect(pushPath('/foo', undefined, { avoidRouterUpdate: true })).toEqual({
56+
expect(pushPath('/foo', undefined)).toEqual({
5857
type: UPDATE_PATH,
5958
payload: {
6059
path: '/foo',
6160
state: undefined,
6261
replace: false,
63-
avoidRouterUpdate: true
6462
}
6563
})
6664
})
@@ -74,27 +72,24 @@ module.exports = function createTests(createHistory, name, reset = defaultReset)
7472
path: '/foo',
7573
replace: true,
7674
state: { bar: 'baz' },
77-
avoidRouterUpdate: false
7875
}
7976
})
8077

81-
expect(replacePath('/foo', undefined, { avoidRouterUpdate: true })).toEqual({
78+
expect(replacePath('/foo', undefined)).toEqual({
8279
type: UPDATE_PATH,
8380
payload: {
8481
path: '/foo',
8582
state: undefined,
8683
replace: true,
87-
avoidRouterUpdate: true
8884
}
8985
})
9086

91-
expect(replacePath('/foo', undefined, { avoidRouterUpdate: false })).toEqual({
87+
expect(replacePath('/foo', undefined)).toEqual({
9288
type: UPDATE_PATH,
9389
payload: {
9490
path: '/foo',
9591
state: undefined,
9692
replace: true,
97-
avoidRouterUpdate: false
9893
}
9994
})
10095
})
@@ -127,7 +122,6 @@ module.exports = function createTests(createHistory, name, reset = defaultReset)
127122
payload: {
128123
path: '/bar',
129124
replace: true,
130-
avoidRouterUpdate: false
131125
}
132126
})).toEqual({
133127
path: '/bar',
@@ -136,22 +130,6 @@ module.exports = function createTests(createHistory, name, reset = defaultReset)
136130
changeId: 2
137131
})
138132
})
139-
140-
it('respects `avoidRouterUpdate` flag', () => {
141-
expect(routeReducer(state, {
142-
type: UPDATE_PATH,
143-
payload: {
144-
path: '/bar',
145-
replace: false,
146-
avoidRouterUpdate: true
147-
}
148-
})).toEqual({
149-
path: '/bar',
150-
replace: false,
151-
state: undefined,
152-
changeId: 1
153-
})
154-
})
155133
})
156134

157135
// To ensure that "Revert" and toggling actions work as expected in
@@ -172,7 +150,8 @@ module.exports = function createTests(createHistory, name, reset = defaultReset)
172150
// Set initial URL before syncing
173151
history.push('/foo')
174152

175-
unsubscribe = syncReduxAndRouter(history, store)
153+
const reduxRouterHistory = syncReduxAndRouter(history, store)
154+
unsubscribe = reduxRouterHistory.unsubscribe
176155
})
177156

178157
afterEach(() => {
@@ -244,7 +223,8 @@ module.exports = function createTests(createHistory, name, reset = defaultReset)
244223
let synced = createSyncedHistoryAndStore(createHistory)
245224
history = synced.history
246225
store = synced.store
247-
unsubscribe = synced.unsubscribe
226+
const reduxRouterHistory = synced.reduxRouterHistory
227+
unsubscribe = reduxRouterHistory.unsubscribe
248228
})
249229

250230
afterEach(() => {
@@ -299,9 +279,9 @@ module.exports = function createTests(createHistory, name, reset = defaultReset)
299279
state: null
300280
})
301281

302-
history.replace({
303-
state: { bar: 'baz' },
304-
pathname: '/bar?query=1'
282+
history.replace({
283+
state: { bar: 'baz' },
284+
pathname: '/bar?query=1'
305285
})
306286
expect(store).toContainRoute({
307287
path: '/bar?query=1',
@@ -310,7 +290,7 @@ module.exports = function createTests(createHistory, name, reset = defaultReset)
310290
})
311291

312292
history.replace({
313-
state: { bar: 'baz' },
293+
state: { bar: 'baz' },
314294
pathname: '/bar?query=1#hash=2'
315295
})
316296
expect(store).toContainRoute({
@@ -478,7 +458,8 @@ module.exports = function createTests(createHistory, name, reset = defaultReset)
478458
routing: routeReducer
479459
}))
480460
const history = createHistory()
481-
const unsubscribe = syncReduxAndRouter(history, store)
461+
const reduxRouterHistory = syncReduxAndRouter(history, store)
462+
const unsubscribe = reduxRouterHistory.unsubscribe;
482463

483464
history.push('/foo')
484465
expect(store).toContainRoute({

0 commit comments

Comments
 (0)