From 6fc6ab7a5fd29cf8cb7ab0f39a528a10cdc30d5f Mon Sep 17 00:00:00 2001 From: Jimmy Jia Date: Sun, 27 Dec 2015 17:25:26 -0500 Subject: [PATCH 1/3] Use middleware to synchronize store to history --- .babelrc | 3 +- package.json | 5 +- src/index.js | 166 +++++++++++++++++------------------------ test/createTests.js | 176 +++++++++++++++++++------------------------- 4 files changed, 147 insertions(+), 203 deletions(-) diff --git a/.babelrc b/.babelrc index 3c3b968..2d4d503 100644 --- a/.babelrc +++ b/.babelrc @@ -1,4 +1,3 @@ { - "presets": ["es2015"], - "plugins": ["transform-object-assign"] + "presets": ["es2015", "stage-2"] } diff --git a/package.json b/package.json index bbfda95..b2672ef 100644 --- a/package.json +++ b/package.json @@ -42,8 +42,8 @@ "babel-core": "^6.2.1", "babel-eslint": "^4.1.6", "babel-loader": "^6.2.0", - "babel-plugin-transform-object-assign": "^6.0.14", "babel-preset-es2015": "^6.1.2", + "babel-preset-stage-2": "^6.3.13", "eslint": "^1.10.3", "eslint-config-rackt": "^1.1.1", "expect": "^1.13.0", @@ -64,8 +64,5 @@ "redux": "^3.0.4", "redux-devtools": "^2.1.5", "webpack": "^1.12.9" - }, - "dependencies": { - "deep-equal": "^1.0.1" } } diff --git a/src/index.js b/src/index.js index 91b14bd..06cc5ca 100644 --- a/src/index.js +++ b/src/index.js @@ -1,61 +1,40 @@ -import deepEqual from 'deep-equal' - // Constants export const UPDATE_PATH = '@@router/UPDATE_PATH' const SELECT_STATE = state => state.routing -export function pushPath(path, state, { avoidRouterUpdate = false } = {}) { +export function pushPath(path, state, key) { return { type: UPDATE_PATH, - payload: { - path: path, - state: state, - replace: false, - avoidRouterUpdate: !!avoidRouterUpdate - } + payload: { path, state, key, replace: false } } } -export function replacePath(path, state, { avoidRouterUpdate = false } = {}) { +export function replacePath(path, state, key) { return { type: UPDATE_PATH, - payload: { - path: path, - state: state, - replace: true, - avoidRouterUpdate: !!avoidRouterUpdate - } + payload: { path, state, key, replace: true } } } // Reducer let initialState = { - changeId: 1, path: undefined, state: undefined, - replace: false + replace: false, + key: undefined } -function update(state=initialState, { type, payload }) { +export function routeReducer(state=initialState, { type, payload }) { if(type === UPDATE_PATH) { - return Object.assign({}, state, { - path: payload.path, - changeId: state.changeId + (payload.avoidRouterUpdate ? 0 : 1), - state: payload.state, - replace: payload.replace - }) + return payload } + return state } // Syncing - -function locationsAreEqual(a, b) { - return a != null && b != null && a.path === b.path && deepEqual(a.state, b.state) -} - function createPath(location) { const { pathname, search, hash } = location let result = pathname @@ -66,84 +45,75 @@ function createPath(location) { return result } -export function syncReduxAndRouter(history, store, selectRouterState = SELECT_STATE) { - const getRouterState = () => selectRouterState(store.getState()) - - // 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. It's possible for this to happen when something - // reloads the entire app state such as redux devtools. - let lastRoute = undefined - - if(!getRouterState()) { - throw new Error( - 'Cannot sync router: route state does not exist (`state.routing` by default). ' + - 'Did you install the routing reducer?' - ) - } +export function syncHistory(history) { + let unsubscribeHistory, currentKey, unsubscribeStore + let connected = false - const unsubscribeHistory = history.listen(location => { - const route = { - path: createPath(location), - state: location.state - } + function middleware(store) { + unsubscribeHistory = history.listen(location => { + const path = createPath(location) + const { state, key } = location + currentKey = key - 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 - } + const method = location.action === 'REPLACE' ? replacePath : pushPath + store.dispatch(method(path, state, key)) + }) - // Also set `lastRoute` so that the store subscriber doesn't - // trigger an unnecessary `pushState` on load - lastRoute = initialState + connected = true - store.dispatch(pushPath(route.path, route.state, { avoidRouterUpdate: true })); - } 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 })) - } - }) + return next => action => { + if (action.type !== UPDATE_PATH) { + next(action) + return + } - const unsubscribeStore = store.subscribe(() => { - let routing = getRouterState() + const { payload } = action + if (payload.key || !connected) { + // Either this came from the history, or else we're not forwarding + // location actions to history. + next(action) + return + } - // Only trigger history update if this is a new change or the - // location has changed. - if(lastRoute.changeId !== routing.changeId || - !locationsAreEqual(lastRoute, routing)) { + const { replace, state, path } = payload + // FIXME: ???! `path` and `pathname` are _not_ synonymous. + const method = replace ? 'replaceState' : 'pushState' - lastRoute = routing - const method = routing.replace ? 'replace' : 'push' - history[method]({ - pathname: routing.path, - state: routing.state - }) + history[method](state, path) } + } - }) + middleware.syncHistoryToStore = + (store, selectRouterState = SELECT_STATE) => { + const getRouterState = () => selectRouterState(store.getState()) + const { + key: initialKey, state: initialState, path: initialPath + } = getRouterState() + + unsubscribeStore = store.subscribe(() => { + let { key, state, path } = getRouterState() + + // If we're resetting to the beginning, use the saved values. + if (key === undefined) { + key = initialKey + state = initialState + path = initialPath + } + + if (key !== currentKey) { + history.pushState(state, path) + } + }) + } - return function unsubscribe() { + middleware.unsubscribe = () => { unsubscribeHistory() - unsubscribeStore() + if (unsubscribeStore) { + unsubscribeStore() + } + + connected = false } -} -export { update as routeReducer } + return middleware +} diff --git a/test/createTests.js b/test/createTests.js index ccc04f5..c6b24b8 100644 --- a/test/createTests.js +++ b/test/createTests.js @@ -1,8 +1,8 @@ /*eslint-env mocha */ import expect from 'expect' -import { pushPath, replacePath, UPDATE_PATH, routeReducer, syncReduxAndRouter } from '../src/index' -import { createStore, combineReducers, compose } from 'redux' +import { pushPath, replacePath, UPDATE_PATH, routeReducer, syncHistory } from '../src/index' +import { applyMiddleware, createStore, combineReducers, compose } from 'redux' import { devTools } from 'redux-devtools' import { ActionCreators } from 'redux-devtools/lib/devTools' import { useBasename } from 'history' @@ -11,27 +11,26 @@ expect.extend({ toContainRoute({ path, state = undefined, - replace = false, - changeId = undefined + replace = false }) { const routing = this.actual.getState().routing expect(routing.path).toEqual(path) expect(routing.state).toEqual(state) expect(routing.replace).toEqual(replace) - - if (changeId !== undefined) { - expect(routing.changeId).toEqual(changeId) - } } }) function createSyncedHistoryAndStore(createHistory) { - const store = createStore(combineReducers({ + const history = createHistory() + const middleware = syncHistory(history) + const { unsubscribe } = middleware + + const createStoreWithMiddleware = applyMiddleware(middleware)(createStore) + const store = createStoreWithMiddleware(combineReducers({ routing: routeReducer })) - const history = createHistory() - const unsubscribe = syncReduxAndRouter(history, store) + return { history, store, unsubscribe } } @@ -50,17 +49,17 @@ module.exports = function createTests(createHistory, name, reset = defaultReset) path: '/foo', replace: false, state: { bar: 'baz' }, - avoidRouterUpdate: false + key: undefined } }) - expect(pushPath('/foo', undefined, { avoidRouterUpdate: true })).toEqual({ + expect(pushPath('/foo', undefined, 'foo')).toEqual({ type: UPDATE_PATH, payload: { path: '/foo', state: undefined, replace: false, - avoidRouterUpdate: true + key: 'foo' } }) }) @@ -74,27 +73,27 @@ module.exports = function createTests(createHistory, name, reset = defaultReset) path: '/foo', replace: true, state: { bar: 'baz' }, - avoidRouterUpdate: false + key: undefined } }) - expect(replacePath('/foo', undefined, { avoidRouterUpdate: true })).toEqual({ + expect(replacePath('/foo', undefined, 'foo')).toEqual({ type: UPDATE_PATH, payload: { path: '/foo', state: undefined, replace: true, - avoidRouterUpdate: true + key: 'foo' } }) - expect(replacePath('/foo', undefined, { avoidRouterUpdate: false })).toEqual({ + expect(replacePath('/foo', undefined, undefined)).toEqual({ type: UPDATE_PATH, payload: { path: '/foo', state: undefined, replace: true, - avoidRouterUpdate: false + key: undefined } }) }) @@ -102,8 +101,7 @@ module.exports = function createTests(createHistory, name, reset = defaultReset) describe('routeReducer', () => { const state = { - path: '/foo', - changeId: 1 + path: '/foo' } it('updates the path', () => { @@ -115,9 +113,7 @@ module.exports = function createTests(createHistory, name, reset = defaultReset) } })).toEqual({ path: '/bar', - replace: false, - state: undefined, - changeId: 2 + replace: false }) }) @@ -126,30 +122,11 @@ module.exports = function createTests(createHistory, name, reset = defaultReset) type: UPDATE_PATH, payload: { path: '/bar', - replace: true, - avoidRouterUpdate: false - } - })).toEqual({ - path: '/bar', - replace: true, - state: undefined, - changeId: 2 - }) - }) - - it('respects `avoidRouterUpdate` flag', () => { - expect(routeReducer(state, { - type: UPDATE_PATH, - payload: { - path: '/bar', - replace: false, - avoidRouterUpdate: true + replace: true } })).toEqual({ path: '/bar', - replace: false, - state: undefined, - changeId: 1 + replace: true }) }) }) @@ -163,16 +140,23 @@ module.exports = function createTests(createHistory, name, reset = defaultReset) beforeEach(() => { history = createHistory() - const finalCreateStore = compose(devTools())(createStore) + + // Set initial URL before syncing + history.push('/foo') + + const middleware = syncHistory(history) + unsubscribe = middleware.unsubscribe + + const finalCreateStore = compose( + applyMiddleware(middleware), + devTools() + )(createStore) store = finalCreateStore(combineReducers({ routing: routeReducer })) devToolsStore = store.devToolsStore - // Set initial URL before syncing - history.push('/foo') - - unsubscribe = syncReduxAndRouter(history, store) + middleware.syncHistoryToStore(store) }) afterEach(() => { @@ -198,7 +182,7 @@ module.exports = function createTests(createHistory, name, reset = defaultReset) historyUnsubscribe() }) - it('handles toggle after store change', () => { + it('handles toggle after history change', () => { let currentPath const historyUnsubscribe = history.listen(location => { currentPath = location.pathname @@ -331,13 +315,14 @@ module.exports = function createTests(createHistory, name, reset = defaultReset) expect(store).toContainRoute({ path: '/foo', replace: false, - state: undefined + state: null }) + // history changes this from PUSH to REPLACE store.dispatch(pushPath('/foo', { bar: 'baz' })) expect(store).toContainRoute({ path: '/foo', - replace: false, + replace: true, state: { bar: 'baz' } }) @@ -348,25 +333,26 @@ module.exports = function createTests(createHistory, name, reset = defaultReset) state: { bar: 'foo' } }) + // history changes this from PUSH to REPLACE store.dispatch(pushPath('/bar')) expect(store).toContainRoute({ path: '/bar', - replace: false, - state: undefined + replace: true, + state: null }) store.dispatch(pushPath('/bar?query=1')) expect(store).toContainRoute({ path: '/bar?query=1', replace: false, - state: undefined + state: null }) store.dispatch(pushPath('/bar?query=1#hash=2')) expect(store).toContainRoute({ path: '/bar?query=1#hash=2', replace: false, - state: undefined + state: null }) }) @@ -440,7 +426,8 @@ module.exports = function createTests(createHistory, name, reset = defaultReset) store.dispatch(pushPath('/foo')) expect(store).toContainRoute({ - path: '/bar' + path: '/bar', + state: null }) store.dispatch(pushPath('/replace', { bar: 'baz' })) @@ -453,33 +440,7 @@ module.exports = function createTests(createHistory, name, reset = defaultReset) expect(updates).toEqual([ '/', '/bar', '/baz' ]) }) - it('throws if "routing" key is missing with default selectRouteState', () => { - const store = createStore(combineReducers({ - notRouting: routeReducer - })) - const history = createHistory() - expect( - () => syncReduxAndRouter(history, store) - ).toThrow(/Cannot sync router: route state does not exist/) - }) - - it('accepts custom selectRouterState', () => { - const store = createStore(combineReducers({ - notRouting: routeReducer - })) - const history = createHistory() - syncReduxAndRouter(history, store, state => state.notRouting) - history.push('/bar') - expect(store.getState().notRouting.path).toEqual('/bar') - }) - it('returns unsubscribe to stop listening to history and store', () => { - const store = createStore(combineReducers({ - routing: routeReducer - })) - const history = createHistory() - const unsubscribe = syncReduxAndRouter(history, store) - history.push('/foo') expect(store).toContainRoute({ path: '/foo', @@ -488,14 +449,19 @@ module.exports = function createTests(createHistory, name, reset = defaultReset) store.dispatch(pushPath('/bar')) expect(store).toContainRoute({ - path: '/bar' + path: '/bar', + state: null }) unsubscribe() + // Make the teardown a no-op. + unsubscribe = () => {} + history.push('/foo') expect(store).toContainRoute({ - path: '/bar' + path: '/bar', + state: null }) history.listenBefore(() => { @@ -534,22 +500,34 @@ module.exports = function createTests(createHistory, name, reset = defaultReset) }) }) - it('handles basename history option', () => { - const store = createStore(combineReducers({ - routing: routeReducer - })) - const history = useBasename(createHistory)({ basename: '/foobar' }) - syncReduxAndRouter(history, store) + describe('basename support', () => { + let history, store, unsubscribe - store.dispatch(pushPath('/bar')) - expect(store).toContainRoute({ - path: '/bar' + beforeEach(() => { + let synced = createSyncedHistoryAndStore( + () => useBasename(createHistory)({ basename: '/foobar' }) + ) + history = synced.history + store = synced.store + unsubscribe = synced.unsubscribe }) - history.push('/baz') - expect(store).toContainRoute({ - path: '/baz', - state: null + afterEach(() => { + unsubscribe() + }) + + it('handles basename history option', () => { + store.dispatch(pushPath('/bar')) + expect(store).toContainRoute({ + path: '/bar', + state: null + }) + + history.push('/baz') + expect(store).toContainRoute({ + path: '/baz', + state: null + }) }) }) }) From 243235eb7c1c9c5c79ab22e2e1eddc88123357d2 Mon Sep 17 00:00:00 2001 From: Jimmy Jia Date: Mon, 28 Dec 2015 16:51:10 -0500 Subject: [PATCH 2/3] Work with location descriptors --- src/index.js | 104 ++++++------ test/createTests.js | 390 +++++++++++++++++++++++--------------------- 2 files changed, 250 insertions(+), 244 deletions(-) diff --git a/src/index.js b/src/index.js index 06cc5ca..c1b8024 100644 --- a/src/index.js +++ b/src/index.js @@ -1,107 +1,95 @@ // Constants -export const UPDATE_PATH = '@@router/UPDATE_PATH' +export const TRANSITION = '@@router/TRANSITION' +export const UPDATE_LOCATION = '@@router/UPDATE_LOCATION' + const SELECT_STATE = state => state.routing -export function pushPath(path, state, key) { - return { - type: UPDATE_PATH, - payload: { path, state, key, replace: false } - } +function transition(method) { + return arg => ({ + type: TRANSITION, + method, arg + }) } -export function replacePath(path, state, key) { +export const push = transition('push') +export const replace = transition('replace') + +// TODO: Add go, goBack, goForward. + +function updateLocation(location) { return { - type: UPDATE_PATH, - payload: { path, state, key, replace: true } + type: UPDATE_LOCATION, + location } } // Reducer -let initialState = { - path: undefined, - state: undefined, - replace: false, - key: undefined +const initialState = { + location: undefined } -export function routeReducer(state=initialState, { type, payload }) { - if(type === UPDATE_PATH) { - return payload +export function routeReducer(state = initialState, { type, location }) { + if (type !== UPDATE_LOCATION) { + return state } - return state + return { location } } // Syncing -function createPath(location) { - const { pathname, search, hash } = location - let result = pathname - if (search) - result += search - if (hash) - result += hash - return result -} export function syncHistory(history) { let unsubscribeHistory, currentKey, unsubscribeStore - let connected = false + let connected = false, syncing = false function middleware(store) { unsubscribeHistory = history.listen(location => { - const path = createPath(location) - const { state, key } = location - currentKey = key + currentKey = location.key + if (syncing) { + // Don't dispatch a new action if we're replaying location. + return + } - const method = location.action === 'REPLACE' ? replacePath : pushPath - store.dispatch(method(path, state, key)) + store.dispatch(updateLocation(location)) }) connected = true return next => action => { - if (action.type !== UPDATE_PATH) { + if (action.type !== TRANSITION || !connected) { next(action) return } - const { payload } = action - if (payload.key || !connected) { - // Either this came from the history, or else we're not forwarding - // location actions to history. - next(action) - return - } - - const { replace, state, path } = payload - // FIXME: ???! `path` and `pathname` are _not_ synonymous. - const method = replace ? 'replaceState' : 'pushState' + // FIXME: Is it correct to swallow the TRANSITION action here and replace + // it with UPDATE_LOCATION instead? We could also use the same type in + // both places instead and just set the location on the action. - history[method](state, path) + const { method, arg } = action + history[method](arg) } } middleware.syncHistoryToStore = (store, selectRouterState = SELECT_STATE) => { const getRouterState = () => selectRouterState(store.getState()) - const { - key: initialKey, state: initialState, path: initialPath - } = getRouterState() + const { location: initialLocation } = getRouterState() unsubscribeStore = store.subscribe(() => { - let { key, state, path } = getRouterState() - // If we're resetting to the beginning, use the saved values. - if (key === undefined) { - key = initialKey - state = initialState - path = initialPath - } + const storeLocation = getRouterState().location + const location = storeLocation || initialLocation + + if (location.key !== currentKey) { + // If there already is a location in the store, then don't dispatch + // an action from the history listener; otherwise do so in order to + // populate the store. + syncing = !!storeLocation - if (key !== currentKey) { - history.pushState(state, path) + history.transitionTo(location) + syncing = false } }) } diff --git a/test/createTests.js b/test/createTests.js index c6b24b8..b8ad8bd 100644 --- a/test/createTests.js +++ b/test/createTests.js @@ -1,23 +1,30 @@ /*eslint-env mocha */ import expect from 'expect' -import { pushPath, replacePath, UPDATE_PATH, routeReducer, syncHistory } from '../src/index' +import { + push, replace, TRANSITION, UPDATE_LOCATION, routeReducer, syncHistory +} from '../src/index' import { applyMiddleware, createStore, combineReducers, compose } from 'redux' import { devTools } from 'redux-devtools' import { ActionCreators } from 'redux-devtools/lib/devTools' -import { useBasename } from 'history' +import { useBasename, useQueries } from 'history' expect.extend({ - toContainRoute({ - path, - state = undefined, - replace = false + toContainLocation({ + pathname, + search = '', + hash = '', + state = null, + query, + action = 'PUSH' }) { - const routing = this.actual.getState().routing + const { location } = this.actual.getState().routing - expect(routing.path).toEqual(path) - expect(routing.state).toEqual(state) - expect(routing.replace).toEqual(replace) + expect(location.pathname).toEqual(pathname) + expect(location.search).toEqual(search) + expect(location.state).toEqual(state) + expect(location.query).toEqual(query) + expect(location.action).toEqual(action) } }) @@ -41,59 +48,39 @@ module.exports = function createTests(createHistory, name, reset = defaultReset) beforeEach(reset) - describe('pushPath', () => { + describe('push', () => { it('creates actions', () => { - expect(pushPath('/foo', { bar: 'baz' })).toEqual({ - type: UPDATE_PATH, - payload: { - path: '/foo', - replace: false, - state: { bar: 'baz' }, - key: undefined - } - }) - - expect(pushPath('/foo', undefined, 'foo')).toEqual({ - type: UPDATE_PATH, - payload: { - path: '/foo', - state: undefined, - replace: false, - key: 'foo' + expect(push('/foo')).toEqual({ + type: TRANSITION, + method: 'push', + arg: '/foo' + }) + + expect(push({ pathname: '/foo', state: { the: 'state' } })).toEqual({ + type: TRANSITION, + method: 'push', + arg: { + pathname: '/foo', + state: { the: 'state' } } }) }) }) - describe('replacePath', () => { + describe('replace', () => { it('creates actions', () => { - expect(replacePath('/foo', { bar: 'baz' })).toEqual({ - type: UPDATE_PATH, - payload: { - path: '/foo', - replace: true, - state: { bar: 'baz' }, - key: undefined - } - }) - - expect(replacePath('/foo', undefined, 'foo')).toEqual({ - type: UPDATE_PATH, - payload: { - path: '/foo', - state: undefined, - replace: true, - key: 'foo' - } - }) - - expect(replacePath('/foo', undefined, undefined)).toEqual({ - type: UPDATE_PATH, - payload: { - path: '/foo', - state: undefined, - replace: true, - key: undefined + expect(replace('/foo')).toEqual({ + type: TRANSITION, + method: 'replace', + arg: '/foo' + }) + + expect(replace({ pathname: '/foo', state: { the: 'state' } })).toEqual({ + type: TRANSITION, + method: 'replace', + arg: { + pathname: '/foo', + state: { the: 'state' } } }) }) @@ -101,32 +88,39 @@ module.exports = function createTests(createHistory, name, reset = defaultReset) describe('routeReducer', () => { const state = { - path: '/foo' + location: { + pathname: '/foo', + action: 'POP' + } } it('updates the path', () => { expect(routeReducer(state, { - type: UPDATE_PATH, - payload: { + type: UPDATE_LOCATION, + location: { path: '/bar', - replace: false + action: 'PUSH' } })).toEqual({ - path: '/bar', - replace: false + location: { + path: '/bar', + action: 'PUSH' + } }) }) it('respects replace', () => { expect(routeReducer(state, { - type: UPDATE_PATH, - payload: { + type: UPDATE_LOCATION, + location: { path: '/bar', - replace: true + action: 'REPLACE' } })).toEqual({ - path: '/bar', - replace: true + location: { + path: '/bar', + action: 'REPLACE' + } }) }) }) @@ -170,13 +164,13 @@ module.exports = function createTests(createHistory, name, reset = defaultReset) }) history.push('/bar') - store.dispatch(pushPath('/baz')) + store.dispatch(push('/baz')) // By calling reset we expect DevTools to re-play the initial state // and the history to update to the initial path devToolsStore.dispatch(ActionCreators.reset()) - expect(store.getState().routing.path).toEqual('/foo') + expect(store.getState().routing.location.pathname).toEqual('/foo') expect(currentPath).toEqual('/foo') historyUnsubscribe() @@ -208,9 +202,9 @@ module.exports = function createTests(createHistory, name, reset = defaultReset) }) // DevTools action #2 - store.dispatch(pushPath('/foo2')) + store.dispatch(push('/foo2')) // DevTools action #3 - store.dispatch(pushPath('/foo3')) + store.dispatch(push('/foo3')) // When we toggle an action, the devtools will revert the action // and we therefore expect the history to update to the previous path @@ -236,123 +230,116 @@ module.exports = function createTests(createHistory, name, reset = defaultReset) }) it('syncs router -> redux', () => { - expect(store).toContainRoute({ - path: '/', - state: null + expect(store).toContainLocation({ + pathname: '/', + action: 'POP' }) history.push('/foo') - expect(store).toContainRoute({ - path: '/foo', - replace: false, - state: null + expect(store).toContainLocation({ + pathname: '/foo' }) history.push({ state: { bar: 'baz' }, pathname: '/foo' }) - expect(store).toContainRoute({ - path: '/foo', - replace: true, - state: { bar: 'baz' } + expect(store).toContainLocation({ + pathname: '/foo', + state: { bar: 'baz' }, + action: 'REPLACE' // Converted by history. }) history.replace('/bar') - expect(store).toContainRoute({ - path: '/bar', - replace: true, - state: null + expect(store).toContainLocation({ + pathname: '/bar', + action: 'REPLACE' }) history.push('/bar') - expect(store).toContainRoute({ - path: '/bar', - replace: true, - state: null + expect(store).toContainLocation({ + pathname: '/bar', + action: 'REPLACE' // Converted by history. }) history.push('/bar?query=1') - expect(store).toContainRoute({ - path: '/bar?query=1', - replace: false, - state: null + expect(store).toContainLocation({ + pathname: '/bar', + search: '?query=1' }) history.push('/bar#baz') - expect(store).toContainRoute({ - path: '/bar#baz', - replace: false, - state: null + expect(store).toContainLocation({ + pathname: '/bar', + hash: '#baz' }) - history.replace({ - state: { bar: 'baz' }, - pathname: '/bar?query=1' - }) - expect(store).toContainRoute({ - path: '/bar?query=1', - replace: true, + history.replace({ + pathname: '/bar', + search: '?query=1', state: { bar: 'baz' } }) + expect(store).toContainLocation({ + pathname: '/bar', + search: '?query=1', + state: { bar: 'baz' }, + action: 'REPLACE' + }) history.replace({ - state: { bar: 'baz' }, - pathname: '/bar?query=1#hash=2' - }) - expect(store).toContainRoute({ - path: '/bar?query=1#hash=2', - replace: true, + pathname: '/bar', + search: '?query=1', + hash: '#hash=2', state: { bar: 'baz' } }) + expect(store).toContainLocation({ + pathname: '/bar', + search: '?query=1', + hash: '#hash=2', + state: { bar: 'baz' }, + action: 'REPLACE' + }) }) it('syncs redux -> router', () => { - expect(store).toContainRoute({ - path: '/', - replace: false, - state: null + expect(store).toContainLocation({ + pathname: '/', + action: 'POP' }) - store.dispatch(pushPath('/foo')) - expect(store).toContainRoute({ - path: '/foo', - replace: false, - state: null + store.dispatch(push('/foo')) + expect(store).toContainLocation({ + pathname: '/foo' }) - // history changes this from PUSH to REPLACE - store.dispatch(pushPath('/foo', { bar: 'baz' })) - expect(store).toContainRoute({ - path: '/foo', - replace: true, - state: { bar: 'baz' } + store.dispatch(push({ pathname: '/foo', state: { bar: 'baz' } })) + expect(store).toContainLocation({ + pathname: '/foo', + state: { bar: 'baz' }, + action: 'REPLACE' // Converted by history. }) - store.dispatch(replacePath('/bar', { bar: 'foo' })) - expect(store).toContainRoute({ - path: '/bar', - replace: true, - state: { bar: 'foo' } + store.dispatch(replace({ pathname: '/bar', state: { bar: 'foo' } })) + expect(store).toContainLocation({ + pathname: '/bar', + state: { bar: 'foo' }, + action: 'REPLACE' }) - // history changes this from PUSH to REPLACE - store.dispatch(pushPath('/bar')) - expect(store).toContainRoute({ - path: '/bar', - replace: true, - state: null + store.dispatch(push('/bar')) + expect(store).toContainLocation({ + pathname: '/bar', + action: 'REPLACE' // Converted by history. }) - store.dispatch(pushPath('/bar?query=1')) - expect(store).toContainRoute({ - path: '/bar?query=1', - replace: false, - state: null + store.dispatch(push('/bar?query=1')) + expect(store).toContainLocation({ + pathname: '/bar', + search: '?query=1' }) - store.dispatch(pushPath('/bar?query=1#hash=2')) - expect(store).toContainRoute({ - path: '/bar?query=1#hash=2', - replace: false, - state: null + store.dispatch(push('/bar?query=1#hash=2')) + expect(store).toContainLocation({ + pathname: '/bar', + search: '?query=1', + hash: '#hash=2' }) }) @@ -362,9 +349,9 @@ module.exports = function createTests(createHistory, name, reset = defaultReset) updates.push(location.pathname) }) - store.dispatch(pushPath('/foo')) - store.dispatch(pushPath('/foo')) - store.dispatch(replacePath('/foo')) + store.dispatch(push('/foo')) + store.dispatch(push('/foo')) + store.dispatch(replace('/foo')) expect(updates).toEqual([ '/', '/foo', '/foo', '/foo' ]) @@ -404,7 +391,7 @@ module.exports = function createTests(createHistory, name, reset = defaultReset) updates.push(location.pathname) }) - store.dispatch(pushPath('/foo')) + store.dispatch(push('/foo')) expect(updates).toEqual([ '/', '/foo' ]) }) @@ -412,10 +399,10 @@ module.exports = function createTests(createHistory, name, reset = defaultReset) it('allows updating the route from within `listenBefore`', () => { history.listenBefore(location => { if(location.pathname === '/foo') { - store.dispatch(pushPath('/bar')) + store.dispatch(push('/bar')) } else if(location.pathname === '/replace') { - store.dispatch(replacePath('/baz', { foo: 'bar' })) + store.dispatch(replace({ pathname: '/baz', state: { foo: 'bar' } })) } }) @@ -424,17 +411,16 @@ module.exports = function createTests(createHistory, name, reset = defaultReset) updates.push(location.pathname) }) - store.dispatch(pushPath('/foo')) - expect(store).toContainRoute({ - path: '/bar', - state: null + store.dispatch(push('/foo')) + expect(store).toContainLocation({ + pathname: '/bar' }) - store.dispatch(pushPath('/replace', { bar: 'baz' })) - expect(store).toContainRoute({ - path: '/baz', + store.dispatch(push({ pathname: '/replace', state: { bar: 'baz' } })) + expect(store).toContainLocation({ + pathname: '/baz', state: { foo: 'bar' }, - replace: true + action: 'REPLACE' }) expect(updates).toEqual([ '/', '/bar', '/baz' ]) @@ -442,15 +428,13 @@ module.exports = function createTests(createHistory, name, reset = defaultReset) it('returns unsubscribe to stop listening to history and store', () => { history.push('/foo') - expect(store).toContainRoute({ - path: '/foo', - state: null + expect(store).toContainLocation({ + pathname: '/foo' }) - store.dispatch(pushPath('/bar')) - expect(store).toContainRoute({ - path: '/bar', - state: null + store.dispatch(push('/bar')) + expect(store).toContainLocation({ + pathname: '/bar' }) unsubscribe() @@ -459,16 +443,15 @@ module.exports = function createTests(createHistory, name, reset = defaultReset) unsubscribe = () => {} history.push('/foo') - expect(store).toContainRoute({ - path: '/bar', - state: null + expect(store).toContainLocation({ + pathname: '/bar' }) history.listenBefore(() => { throw new Error() }) expect( - () => store.dispatch(pushPath('/foo')) + () => store.dispatch(push('/foo')) ).toNotThrow() }) @@ -478,8 +461,8 @@ module.exports = function createTests(createHistory, name, reset = defaultReset) updates.push(location.pathname) }) - store.dispatch(pushPath('/bar')) - store.dispatch(pushPath('/baz')) + store.dispatch(push('/bar')) + store.dispatch(push('/baz')) expect(updates).toEqual([ '/', '/bar', '/baz' ]) historyUnsubscribe() @@ -488,23 +471,60 @@ module.exports = function createTests(createHistory, name, reset = defaultReset) it('only triggers store once when updating path via store', () => { const updates = [] const storeUnsubscribe = store.subscribe(() => { - updates.push(store.getState().routing.path) + updates.push(store.getState().routing.location.pathname) }) - store.dispatch(pushPath('/bar')) - store.dispatch(pushPath('/baz')) - store.dispatch(replacePath('/foo')) + store.dispatch(push('/bar')) + store.dispatch(push('/baz')) + store.dispatch(replace('/foo')) expect(updates).toEqual([ '/bar', '/baz', '/foo' ]) storeUnsubscribe() }) }) + describe('query support', () => { + let history, store, unsubscribe + + beforeEach(() => { + const synced = createSyncedHistoryAndStore(useQueries(createHistory)) + history = synced.history + store = synced.store + unsubscribe = synced.unsubscribe + }) + + afterEach(() => { + unsubscribe() + }) + + it('handles location queries', () => { + store.dispatch(push({ pathname: '/bar', query: { the: 'query' } })) + expect(store).toContainLocation({ + pathname: '/bar', + query: { the: 'query' }, + search: '?the=query' + }) + + history.push({ pathname: '/baz', query: { other: 'query' } }) + expect(store).toContainLocation({ + pathname: '/baz', + query: { other: 'query' }, + search: '?other=query' + }) + + store.dispatch(push('/foo')) + expect(store).toContainLocation({ + pathname: '/foo', + query: {} + }) + }) + }) + describe('basename support', () => { let history, store, unsubscribe beforeEach(() => { - let synced = createSyncedHistoryAndStore( + const synced = createSyncedHistoryAndStore( () => useBasename(createHistory)({ basename: '/foobar' }) ) history = synced.history @@ -517,16 +537,14 @@ module.exports = function createTests(createHistory, name, reset = defaultReset) }) it('handles basename history option', () => { - store.dispatch(pushPath('/bar')) - expect(store).toContainRoute({ - path: '/bar', - state: null + store.dispatch(push('/bar')) + expect(store).toContainLocation({ + pathname: '/bar' }) history.push('/baz') - expect(store).toContainRoute({ - path: '/baz', - state: null + expect(store).toContainLocation({ + pathname: '/baz' }) }) }) From 30b1187797947f0e7fca3d3d3e3e014851fadaeb Mon Sep 17 00:00:00 2001 From: Jimmy Jia Date: Tue, 29 Dec 2015 13:23:37 -0500 Subject: [PATCH 3/3] Fix resetting to initial location --- src/index.js | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/src/index.js b/src/index.js index c1b8024..31b682b 100644 --- a/src/index.js +++ b/src/index.js @@ -78,16 +78,21 @@ export function syncHistory(history) { const { location: initialLocation } = getRouterState() unsubscribeStore = store.subscribe(() => { - // If we're resetting to the beginning, use the saved values. - const storeLocation = getRouterState().location - const location = storeLocation || initialLocation + const { location } = getRouterState() + + // If we're resetting to the beginning, use the saved initial value. We + // need to dispatch a new action at this point to populate the store + // appropriately. + if (!location) { + history.transitionTo(initialLocation) + return + } + // Otherwise, if we need to update the history location, do so without + // dispatching a new action, as we're just bringing history in sync + // with the store. if (location.key !== currentKey) { - // If there already is a location in the store, then don't dispatch - // an action from the history listener; otherwise do so in order to - // populate the store. - syncing = !!storeLocation - + syncing = true history.transitionTo(location) syncing = false }