An extension of #109 for stopping cycles

src/index.js

@@ -14,32 +14,29 @@ function initPath(path, state) {
     payload: {
       path: path,
       state: state,
-      replace: false,
-      avoidRouterUpdate: true
+      replace: false
     }
   }
 }
 
-export function pushPath(path, state, { avoidRouterUpdate = false } = {}) {
+export function pushPath(path, state) {
   return {
     type: UPDATE_PATH,
     payload: {
       path: path,
       state: state,
-      replace: false,
-      avoidRouterUpdate: !!avoidRouterUpdate
+      replace: false
     }
   }
 }
 
-export function replacePath(path, state, { avoidRouterUpdate = false } = {}) {
+export function replacePath(path, state) {
   return {
     type: UPDATE_PATH,
     payload: {
       path: path,
       state: state,
-      replace: true,
-      avoidRouterUpdate: !!avoidRouterUpdate
+      replace: true
     }
   }
 }
@@ -57,7 +54,7 @@ function update(state=initialState, { type, payload }) {
   if(type === INIT_PATH || type === UPDATE_PATH) {
     return Object.assign({}, state, {
       path: payload.path,
-      changeId: state.changeId + (payload.avoidRouterUpdate ? 0 : 1),
+      changeId: state.changeId + 1,
       state: payload.state,
       replace: payload.replace
     })
@@ -92,6 +89,7 @@ export function syncReduxAndRouter(history, store, selectRouterState = SELECT_ST
   // history update. It's possible for this to happen when something
   // reloads the entire app state such as redux devtools.
   let lastRoute = undefined
+  let avoidRouterUpdate = false;
 
   if(!getRouterState()) {
     throw new Error(
@@ -100,56 +98,50 @@ export function syncReduxAndRouter(history, store, selectRouterState = SELECT_ST
     )
   }
 
+  const unsubscribeStore = store.subscribe(() => {
+    let routing = getRouterState()
+
+    // Only trigger history update if this is a new change or the
+    // location has changed.
+    if(lastRoute !== routing && !avoidRouterUpdate) {
+      lastRoute = routing;
+      const method = routing.replace ? 'replaceState' : 'pushState'
+      history[method](routing.state, routing.path)
+    }
+
+    avoidRouterUpdate = false;
+  })
+
+
   const unsubscribeHistory = history.listen(location => {
     const route = {
       path: createPath(location),
       state: location.state
     }
 
-    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))
-    } else if(!locationsAreEqual(getRouterState(), route)) {
+    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 }))
-    }
-  })
-
-  const unsubscribeStore = store.subscribe(() => {
-    let routing = getRouterState()
 
-    // Only trigger history update if this is a new change or the
-    // location has changed.
-    if(lastRoute.changeId !== routing.changeId ||
-       !locationsAreEqual(lastRoute, routing)) {
+      if(!lastRoute) {
+        initialState = {
+          changeId: 1,
+          path: route.path,
+          state: route.state,
+          replace: false
+        }
+
+        // TODO: temporary hack only so that we don't set the
+        // initialState again. This is not needed for anything else.
+        // We should re-think in generate how to solve the initial
+        // state problem.
+        lastRoute = route
+      }
 
-      lastRoute = routing
-      const method = routing.replace ? 'replaceState' : 'pushState'
-      history[method](routing.state, routing.path)
+      avoidRouterUpdate = true;
+      const method = location.action === 'REPLACE' ? replacePath : pushPath
+      store.dispatch(method(route.path, route.state))
     }
-
   })
 
   return function unsubscribe() {

test/createTests.js

@@ -49,18 +49,16 @@ module.exports = function createTests(createHistory, name, reset = defaultReset)
           payload: {
             path: '/foo',
             replace: false,
-            state: { bar: 'baz' },
-            avoidRouterUpdate: false
+            state: { bar: 'baz' }
           }
         })
 
-        expect(pushPath('/foo', undefined, { avoidRouterUpdate: true })).toEqual({
+        expect(pushPath('/foo', undefined)).toEqual({
           type: UPDATE_PATH,
           payload: {
             path: '/foo',
             state: undefined,
-            replace: false,
-            avoidRouterUpdate: true
+            replace: false
           }
         })
       })
@@ -73,28 +71,25 @@ module.exports = function createTests(createHistory, name, reset = defaultReset)
           payload: {
             path: '/foo',
             replace: true,
-            state: { bar: 'baz' },
-            avoidRouterUpdate: false
+            state: { bar: 'baz' }
           }
         })
 
-        expect(replacePath('/foo', undefined, { avoidRouterUpdate: true })).toEqual({
+        expect(replacePath('/foo', undefined)).toEqual({
           type: UPDATE_PATH,
           payload: {
             path: '/foo',
             state: undefined,
-            replace: true,
-            avoidRouterUpdate: true
+            replace: true
           }
         })
 
-        expect(replacePath('/foo', undefined, { avoidRouterUpdate: false })).toEqual({
+        expect(replacePath('/foo', undefined)).toEqual({
           type: UPDATE_PATH,
           payload: {
             path: '/foo',
             state: undefined,
-            replace: true,
-            avoidRouterUpdate: false
+            replace: true
           }
         })
       })
@@ -126,8 +121,7 @@ module.exports = function createTests(createHistory, name, reset = defaultReset)
           type: UPDATE_PATH,
           payload: {
             path: '/bar',
-            replace: true,
-            avoidRouterUpdate: false
+            replace: true
           }
         })).toEqual({
           path: '/bar',
@@ -136,22 +130,6 @@ module.exports = function createTests(createHistory, name, reset = defaultReset)
           changeId: 2
         })
       })
-
-      it('respects `avoidRouterUpdate` flag', () => {
-        expect(routeReducer(state, {
-          type: UPDATE_PATH,
-          payload: {
-            path: '/bar',
-            replace: false,
-            avoidRouterUpdate: true
-          }
-        })).toEqual({
-          path: '/bar',
-          replace: false,
-          state: undefined,
-          changeId: 1
-        })
-      })
     })
 
     // To ensure that "Revert" and toggling actions work as expected in