From 0e90f6cf672967d25f557ede876f1ca484db77a9 Mon Sep 17 00:00:00 2001 From: James Long Date: Fri, 18 Dec 2015 17:43:00 -0500 Subject: [PATCH] wip --- src/index.js | 88 +++++++++++++++++++++------------------------ test/createTests.js | 40 +++++---------------- 2 files changed, 49 insertions(+), 79 deletions(-) diff --git a/src/index.js b/src/index.js index a2e007c..f6bd75a 100644 --- a/src/index.js +++ b/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() { diff --git a/test/createTests.js b/test/createTests.js index 42b307d..4593c6b 100644 --- a/test/createTests.js +++ b/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