Merge pull request #40 from jlongster/experimental-sync-logic

Experimental sync logic

Author: jlongster

.babelrc

@@ -0,0 +1,3 @@
+{
+    "presets": ["es2015"]
+}

package.json

@@ -12,7 +12,8 @@
   ],
   "license": "MIT",
   "scripts": {
-    "build": "mkdir -p lib && babel ./src/index.js --plugins transform-object-assign --presets babel-preset-es2015 --out-file ./lib/index.js",
+    "build": "mkdir -p lib && babel ./src/index.js --plugins transform-object-assign --out-file ./lib/index.js",
+    "test": "mocha --compilers js:babel-core/register --recursive",
     "prepublish": "npm run build"
   },
   "tags": [
@@ -26,7 +27,12 @@
   ],
   "devDependencies": {
     "babel-cli": "^6.1.2",
+    "babel-core": "^6.2.1",
     "babel-plugin-transform-object-assign": "^6.0.14",
-    "babel-preset-es2015": "^6.1.2"
+    "babel-preset-es2015": "^6.1.2",
+    "expect": "^1.13.0",
+    "history": "^1.13.1",
+    "mocha": "^2.3.4",
+    "redux": "^3.0.4"
   }
 }

src/index.js

@@ -6,25 +6,28 @@ const SELECT_STATE = state => state.routing;
 
 // Action creator
 
-function updatePath(path, noRouterUpdate) {
+function updatePath(path, avoidRouterUpdate) {
   return {
     type: UPDATE_PATH,
     path: path,
-    noRouterUpdate: noRouterUpdate
+    avoidRouterUpdate: !!avoidRouterUpdate
   }
 }
 
 // Reducer
 
-const initialState = typeof window === 'undefined' ? {} : {
-  path: locationToString(window.location)
+const initialState = {
+  changeId: 1,
+  path: (typeof window !== 'undefined') ?
+    locationToString(window.location) :
+    '/'
 };
 
 function update(state=initialState, action) {
   if(action.type === UPDATE_PATH) {
     return Object.assign({}, state, {
       path: action.path,
-      noRouterUpdate: action.noRouterUpdate
+      changeId: state.changeId + (action.avoidRouterUpdate ? 0 : 1)
     });
   }
   return state;
@@ -37,8 +40,8 @@ function locationToString(location) {
 }
 
 function syncReduxAndRouter(history, store, selectRouterState = SELECT_STATE) {
-  let lastRoute;
   const getRouterState = () => selectRouterState(store.getState());
+  let lastChangeId = 0;
 
   if(!getRouterState()) {
     throw new Error(
@@ -48,26 +51,22 @@ function syncReduxAndRouter(history, store, selectRouterState = SELECT_STATE) {
   }
 
   const unsubscribeHistory = history.listen(location => {
-    const newLocation = locationToString(location);
-    // 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(getRouterState().path !== newLocation) {
-      lastRoute = newLocation;
-      store.dispatch(updatePath(newLocation));
+    const routePath = locationToString(location);
+
+    // Avoid dispatching an action if the store is already up-to-date
+    if(getRouterState().path !== routePath) {
+      store.dispatch(updatePath(routePath, { avoidRouterUpdate: true }));
     }
   });
 
   const unsubscribeStore = store.subscribe(() => {
     const routing = getRouterState();
 
-    // Don't update the router if the routing state hasn't changed or the new routing path
-    // is already the current location.
-    // The `noRouterUpdate` flag can be set to avoid updating altogether,
-    // which is useful for things like loading snapshots or very special
-    // edge cases.
-    if(lastRoute !== routing.path && routing.path !== locationToString(window.location) &&
-       !routing.noRouterUpdate) {
-      lastRoute = routing.path;
+    // Only update the router once per `updatePath` call. This is
+    // indicated by the `changeId` state; when that number changes, we
+    // should call `pushState`.
+    if(lastChangeId !== routing.changeId) {
+      lastChangeId = routing.changeId;
       history.pushState(null, routing.path);
     }
   });

test/index.js

@@ -0,0 +1,187 @@
+const expect = require('expect');
+const { updatePath, UPDATE_PATH, routeReducer, syncReduxAndRouter } = require('../src/index');
+const { createStore, combineReducers } = require('redux');
+const { createMemoryHistory: createHistory } = require('history');
+
+function createSyncedHistoryAndStore() {
+  const store = createStore(combineReducers({
+    routing: routeReducer
+  }));
+  const history = createHistory();
+  syncReduxAndRouter(history, store);
+  return { history, store };
+}
+
+describe('updatePath', () => {
+  it('creates actions', () => {
+    expect(updatePath('/foo')).toEqual({
+      type: UPDATE_PATH,
+      path: '/foo',
+      avoidRouterUpdate: false
+    });
+
+    expect(updatePath('/foo', { avoidRouterUpdate: true })).toEqual({
+      type: UPDATE_PATH,
+      path: '/foo',
+      avoidRouterUpdate: true
+    });
+  });
+});
+
+describe('routeReducer', () => {
+  const state = {
+    path: '/foo',
+    changeId: 1
+  };
+
+  it('updates the path', () => {
+    expect(routeReducer(state, {
+      type: UPDATE_PATH,
+      path: '/bar'
+    })).toEqual({
+      path: '/bar',
+      changeId: 2
+    });
+  });
+
+  it('respects `avoidRouterUpdate` flag', () => {
+    expect(routeReducer(state, {
+      type: UPDATE_PATH,
+      path: '/bar',
+      avoidRouterUpdate: true
+    })).toEqual({
+      path: '/bar',
+      changeId: 1
+    });
+  });
+});
+
+describe('syncReduxAndRouter', () => {
+  it('syncs router -> redux', () => {
+    const { history, store } = createSyncedHistoryAndStore();
+    expect(store.getState().routing.path).toEqual('/');
+
+    history.pushState(null, '/foo');
+    expect(store.getState().routing.path).toEqual('/foo');
+
+    history.pushState(null, '/bar');
+    expect(store.getState().routing.path).toEqual('/bar');
+
+    history.pushState(null, '/bar?query=1');
+    expect(store.getState().routing.path).toEqual('/bar?query=1');
+
+    history.pushState(null, '/bar?query=1#hash=2');
+    expect(store.getState().routing.path).toEqual('/bar?query=1#hash=2');
+  });
+
+  it('syncs redux -> router', () => {
+    const { history, store } = createSyncedHistoryAndStore();
+    expect(store.getState().routing).toEqual({
+      path: '/',
+      changeId: 1
+    });
+
+    store.dispatch(updatePath('/foo'));
+    expect(store.getState().routing).toEqual({
+      path: '/foo',
+      changeId: 2
+    });
+
+    store.dispatch(updatePath('/bar'));
+    expect(store.getState().routing).toEqual({
+      path: '/bar',
+      changeId: 3
+    });
+
+    store.dispatch(updatePath('/bar?query=1'));
+    expect(store.getState().routing).toEqual({
+      path: '/bar?query=1',
+      changeId: 4
+    });
+
+    store.dispatch(updatePath('/bar?query=1#hash=2'));
+    expect(store.getState().routing).toEqual({
+      path: '/bar?query=1#hash=2',
+      changeId: 5
+    });
+  });
+
+  it('updates the router even if path is the same', () => {
+    const { history, store } = createSyncedHistoryAndStore();
+    expect(store.getState().routing).toEqual({
+      path: '/',
+      changeId: 1
+    });
+
+    store.dispatch(updatePath('/foo'));
+    expect(store.getState().routing).toEqual({
+      path: '/foo',
+      changeId: 2
+    });
+
+    store.dispatch(updatePath('/foo'));
+    expect(store.getState().routing).toEqual({
+      path: '/foo',
+      changeId: 3
+    });
+  });
+
+  it('does not update the router for other state changes', () => {
+    const { history, store } = createSyncedHistoryAndStore();
+    store.dispatch({
+      type: 'RANDOM_ACTION',
+      value: 5
+    });
+
+    expect(store.getState().routing).toEqual({
+      path: '/',
+      changeId: 1
+    });
+  });
+
+  it('only updates the router once when dispatching from `listenBefore`', () => {
+    const { history, store } = createSyncedHistoryAndStore();
+    expect(store.getState().routing).toEqual({
+      path: '/',
+      changeId: 1
+    });
+
+    history.listenBefore(location => {
+      expect(location.pathname).toEqual('/foo');
+      store.dispatch({
+        type: 'RANDOM_ACTION',
+        value: 5
+      });
+    });
+
+    store.dispatch(updatePath('/foo'));
+    expect(store.getState().routing).toEqual({
+      path: '/foo',
+      changeId: 2
+    });
+  });
+
+  it('allows updating the route from within `listenBefore`', () => {
+    const { history, store } = createSyncedHistoryAndStore();
+    expect(store.getState().routing).toEqual({
+      path: '/',
+      changeId: 1
+    });
+
+    history.listenBefore(location => {
+      if(location.pathname === '/foo') {
+        expect(store.getState().routing).toEqual({
+          path: '/foo',
+          changeId: 2
+        });
+        store.dispatch(updatePath('/bar'));
+      }
+    });
+
+    store.dispatch(updatePath('/foo'));
+    expect(store.getState().routing).toEqual({
+      path: '/bar',
+      changeId: 3
+    });
+  })
+});