From fe2866800dd96ad676eef3940aafc7fa0af60cea Mon Sep 17 00:00:00 2001 From: James Long Date: Sun, 6 Dec 2015 13:58:08 -0500 Subject: [PATCH 1/3] make `initialState` consistent with currnt location and create INIT_PATH action --- src/index.js | 58 ++++++++++++++++++++++++++++++++-------------------- 1 file changed, 36 insertions(+), 22 deletions(-) diff --git a/src/index.js b/src/index.js index 14e63a9..69d5c49 100644 --- a/src/index.js +++ b/src/index.js @@ -2,10 +2,23 @@ const deepEqual = require('deep-equal'); // Constants +const INIT_PATH = "@@router/INIT_PATH"; const UPDATE_PATH = "@@router/UPDATE_PATH"; const SELECT_STATE = state => state.routing; -// Action creator +// Action creators + +function initPath(path, state) { + return { + type: INIT_PATH, + payload: { + path: path, + state: state, + replace: false, + avoidRouterUpdate: true + } + }; +} function pushPath(path, state, { avoidRouterUpdate = false } = {}) { return { @@ -33,7 +46,7 @@ function replacePath(path, state, { avoidRouterUpdate = false } = {}) { // Reducer -const initialState = { +let initialState = { changeId: 1, path: undefined, state: undefined, @@ -41,7 +54,7 @@ const initialState = { }; function update(state=initialState, { type, payload }) { - if(type === UPDATE_PATH) { + if(type === INIT_PATH || type === UPDATE_PATH) { return Object.assign({}, state, { path: payload.path, changeId: state.changeId + (payload.avoidRouterUpdate ? 0 : 1), @@ -61,15 +74,16 @@ function locationsAreEqual(a, b) { function syncReduxAndRouter(history, store, selectRouterState = SELECT_STATE) { const getRouterState = () => selectRouterState(store.getState()); - // `initialState` *sould* represent the current location when the + // `initialState` *should* represent the current location when the // app loads, but we cannot get the current location when it is // defined. What happens is `history.listen` is called immediately // when it is registered, and it updates the app state with an - // action. This causes problems with redux devtools because "revert" - // will use `initialState` and it won't revert to the original URL. - // Instead, we track the first route and hack it to load when using - // the `initialState`. - let firstRoute = undefined; + // UPDATE_PATH action. This causes problem when users are listening + // to UPDATE_PATH actions just for *changes*, and with redux + // devtools because "revert" will use `initialState` and it won't + // revert to the original URL. Instead, we specialize the first + // route notification and do different things based on it. + let firstLoad = true; // To properly handle store updates we need to track the last route. // This route contains a `changeId` which is updated on every @@ -93,14 +107,19 @@ 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)) { + if (firstLoad) { + // See comment above about `firstLoad` as to why we do this. + initialState = { + changeId: 1, + path: route.path, + state: route.state, + replace: false + }; + store.dispatch(initPath(route.path, route.state)); + firstLoad = false; + } else if(!locationsAreEqual(getRouterState(), route)) { + // The above check avoids dispatching an action if the store is + // already up-to-date const method = location.action === 'REPLACE' ? replacePath : pushPath; store.dispatch(method(route.path, route.state, { avoidRouterUpdate: true })); } @@ -109,11 +128,6 @@ function syncReduxAndRouter(history, store, selectRouterState = SELECT_STATE) { const unsubscribeStore = store.subscribe(() => { let routing = getRouterState(); - // Treat `firstRoute` as our `initialState` - if(routing === initialState) { - routing = firstRoute; - } - // Only trigger history update is this is a new change or the // location has changed. if(lastRoute === undefined || From 5c05c0143d735c75c31551d908e8407453b3f8ea Mon Sep 17 00:00:00 2001 From: James Long Date: Sun, 6 Dec 2015 14:26:40 -0500 Subject: [PATCH 2/3] refactor initialization logic and create INIT_PATH action --- src/index.js | 33 +++++++++++++++++---------------- 1 file changed, 17 insertions(+), 16 deletions(-) diff --git a/src/index.js b/src/index.js index 69d5c49..9b7b65d 100644 --- a/src/index.js +++ b/src/index.js @@ -74,17 +74,6 @@ function locationsAreEqual(a, b) { function syncReduxAndRouter(history, store, selectRouterState = SELECT_STATE) { const getRouterState = () => selectRouterState(store.getState()); - // `initialState` *should* represent the current location when the - // app loads, but we cannot get the current location when it is - // defined. What happens is `history.listen` is called immediately - // when it is registered, and it updates the app state with an - // UPDATE_PATH action. This causes problem when users are listening - // to UPDATE_PATH actions just for *changes*, and with redux - // devtools because "revert" will use `initialState` and it won't - // revert to the original URL. Instead, we specialize the first - // route notification and do different things based on it. - let firstLoad = true; - // 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 @@ -107,16 +96,29 @@ function syncReduxAndRouter(history, store, selectRouterState = SELECT_STATE) { state: location.state }; - if (firstLoad) { - // See comment above about `firstLoad` as to why we do this. + if (!lastRoute) { + // `initialState` *should* represent the current location when + // the app loads, but we cannot get the current location when it + // is defined. What happens is `history.listen` is called + // immediately when it is registered, and it updates the app + // state with an UPDATE_PATH action. This causes problem when + // users are listening to UPDATE_PATH actions just for + // *changes*, and with redux devtools because "revert" will use + // `initialState` and it won't revert to the original URL. + // Instead, we specialize the first route notification and do + // different things based on it. initialState = { changeId: 1, path: route.path, state: route.state, replace: false }; + + // Also set `lastRoute` so that the store subscriber doesn't + // trigger an unnecessary `pushState` on load + lastRoute = initialState; + store.dispatch(initPath(route.path, route.state)); - firstLoad = false; } else if(!locationsAreEqual(getRouterState(), route)) { // The above check avoids dispatching an action if the store is // already up-to-date @@ -130,8 +132,7 @@ function syncReduxAndRouter(history, store, selectRouterState = SELECT_STATE) { // Only trigger history update is this is a new change or the // location has changed. - if(lastRoute === undefined || - lastRoute.changeId !== routing.changeId || + if(lastRoute.changeId !== routing.changeId || !locationsAreEqual(lastRoute, routing)) { lastRoute = routing; From 0ef4fd3e68020eae6ad5962c220ababed7b30db0 Mon Sep 17 00:00:00 2001 From: James Long Date: Sun, 6 Dec 2015 20:16:57 -0500 Subject: [PATCH 3/3] fix small typo in comments --- src/index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/index.js b/src/index.js index 9b7b65d..fc43a1d 100644 --- a/src/index.js +++ b/src/index.js @@ -130,7 +130,7 @@ function syncReduxAndRouter(history, store, selectRouterState = SELECT_STATE) { const unsubscribeStore = store.subscribe(() => { let routing = getRouterState(); - // Only trigger history update is this is a new change or the + // Only trigger history update if this is a new change or the // location has changed. if(lastRoute.changeId !== routing.changeId || !locationsAreEqual(lastRoute, routing)) {