From fb1508c1f003ac8c4bad29e8a22a83317fbbff48 Mon Sep 17 00:00:00 2001 From: Kim Joar Bekkelund Date: Fri, 4 Dec 2015 06:54:44 +0100 Subject: [PATCH 1/3] Get devtools working --- src/index.js | 38 ++++++++++++++++++++++++++++++-------- 1 file changed, 30 insertions(+), 8 deletions(-) diff --git a/src/index.js b/src/index.js index 8d62e53..b8816cf 100644 --- a/src/index.js +++ b/src/index.js @@ -55,12 +55,26 @@ function update(state=initialState, { type, payload }) { // Syncing function locationsAreEqual(a, b) { - return a.path === b.path && deepEqual(a.state, b.state); + return a != null && b != null && a.path === b.path && deepEqual(a.state, b.state); } function syncReduxAndRouter(history, store, selectRouterState = SELECT_STATE) { const getRouterState = () => selectRouterState(store.getState()); - let lastChangeId = 0; + + // Because we're not able to set the initial path in `initialState` we need a + // "hack" to get "Revert" in Redux DevTools to work. We solve this by keeping + // the first route so we can revert to this route when the initial state is + // replayed to reset the state. Basically, we treat the first route as our + // initial state. + let firstRoute = undefined; + + // To properly handle store updates we need to track the last route. This + // route contains a `changeId` which is updated on every `pushPath` and + // `replacePath`. If this id changes we always trigger a history update. + // However, if the id does not change, we check if the location has changed, + // and if it is we trigger a history update. (If these are out of sync it's + // likely because of React DevTools.) + let lastRoute = undefined; if(!getRouterState()) { throw new Error( @@ -75,6 +89,10 @@ function syncReduxAndRouter(history, store, selectRouterState = SELECT_STATE) { state: location.state }; + if (firstRoute === undefined) { + firstRoute = route; + } + // Avoid dispatching an action if the store is already up-to-date, // even if `history` wouldn't do anything if the location is the same if(locationsAreEqual(getRouterState(), route)) return; @@ -87,14 +105,18 @@ function syncReduxAndRouter(history, store, selectRouterState = SELECT_STATE) { }); const unsubscribeStore = store.subscribe(() => { - const routing = getRouterState(); + let routing = getRouterState(); - // Only update the router once per `pushPath` call. This is - // indicated by the `changeId` state; when that number changes, we - // should update the history. - if(lastChangeId === routing.changeId) return; + // Treat `firstRoute` as our `initialState` + if(routing === initialState) { + routing = firstRoute; + } + + if(lastRoute !== undefined && lastRoute.changeId === routing.changeId) { + if(locationsAreEqual(routing, lastRoute)) return; + } - lastChangeId = routing.changeId; + lastRoute = routing; const method = routing.replace ? 'replaceState' : 'pushState'; From 4091b1e43f9a0b83243a09592aab2b5f22e239a7 Mon Sep 17 00:00:00 2001 From: Kim Joar Bekkelund Date: Sat, 5 Dec 2015 18:22:52 +0100 Subject: [PATCH 2/3] Add tests for devtools fix --- package.json | 1 + test/createTests.js | 75 ++++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 75 insertions(+), 1 deletion(-) diff --git a/package.json b/package.json index aae8dc0..93bbd07 100644 --- a/package.json +++ b/package.json @@ -53,6 +53,7 @@ "karma-webpack": "^1.7.0", "mocha": "^2.3.4", "redux": "^3.0.4", + "redux-devtools": "^2.1.5", "webpack": "^1.12.9" }, "dependencies": { diff --git a/test/createTests.js b/test/createTests.js index e05b9ad..14c49a6 100644 --- a/test/createTests.js +++ b/test/createTests.js @@ -1,6 +1,8 @@ const expect = require('expect'); const { pushPath, replacePath, UPDATE_PATH, routeReducer, syncReduxAndRouter } = require('../src/index'); -const { createStore, combineReducers } = require('redux'); +const { createStore, combineReducers, compose } = require('redux'); +const { devTools } = require('redux-devtools'); +const { ActionCreators } = require('redux-devtools/lib/devTools'); function createSyncedHistoryAndStore(createHistory) { const store = createStore(combineReducers({ @@ -130,6 +132,77 @@ module.exports = function createTests(createHistory, name, reset = defaultReset) }); }); + describe('devtools', () => { + let history, store, devToolsStore, unsubscribe; + + beforeEach(() => { + history = createHistory(); + const finalCreateStore = compose(devTools())(createStore); + store = finalCreateStore(combineReducers({ + routing: routeReducer + })); + devToolsStore = store.devToolsStore; + + // Set initial URL before syncing + history.pushState(null, '/foo'); + + unsubscribe = syncReduxAndRouter(history, store); + }); + + afterEach(() => { + unsubscribe(); + }); + + it('resets to the initial url', () => { + let lastPath; + const historyUnsubscribe = history.listen(location => { + lastPath = location.pathname; + }); + + history.pushState(null, '/bar'); + store.dispatch(pushPath('/baz')); + + devToolsStore.dispatch(ActionCreators.reset()); + + expect(store.getState().routing.path).toEqual('/foo'); + expect(lastPath).toEqual('/foo'); + }); + + it('handles toggle after store change', () => { + let lastPath; + const historyUnsubscribe = history.listen(location => { + lastPath = location.pathname; + }); + + // action 2 + history.pushState(null, '/foo2'); + // action 3 + history.pushState(null, '/foo3'); + + devToolsStore.dispatch(ActionCreators.toggleAction(3)); + expect(lastPath).toEqual('/foo2'); + + historyUnsubscribe(); + }); + + it('handles toggle after store change', () => { + let lastPath; + const historyUnsubscribe = history.listen(location => { + lastPath = location.pathname; + }); + + // action 2 + store.dispatch(pushPath('/foo2')); + // action 3 + store.dispatch(pushPath('/foo3')); + + devToolsStore.dispatch(ActionCreators.toggleAction(3)); + expect(lastPath).toEqual('/foo2'); + + historyUnsubscribe(); + }); + }); + describe('syncReduxAndRouter', () => { let history, store, unsubscribe; From ab7b481006176dee754951976887ad9cc59ea690 Mon Sep 17 00:00:00 2001 From: Kim Joar Bekkelund Date: Sat, 5 Dec 2015 19:18:20 +0100 Subject: [PATCH 3/3] Clean up location check --- src/index.js | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/index.js b/src/index.js index b8816cf..0b33e7b 100644 --- a/src/index.js +++ b/src/index.js @@ -112,8 +112,12 @@ function syncReduxAndRouter(history, store, selectRouterState = SELECT_STATE) { routing = firstRoute; } - if(lastRoute !== undefined && lastRoute.changeId === routing.changeId) { - if(locationsAreEqual(routing, lastRoute)) return; + // Only trigger history update is this is a new change or the location + // has changed. + if(lastRoute !== undefined && + lastRoute.changeId === routing.changeId && + locationsAreEqual(lastRoute, routing)) { + return; } lastRoute = routing;