From 151154f27859257d6ebc615b007143524e7588cc Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Wed, 3 Feb 2016 04:53:44 +0000 Subject: [PATCH] Tweak the API to handle initial state correctly --- README.md | 18 +++-- examples/basic/app.js | 18 +++-- examples/basic/package.json | 4 +- package.json | 2 +- src/index.js | 103 ++++++++++++++++------------ test/createTests.js | 133 +++++++++++++++++++++++++++++++----- 6 files changed, 202 insertions(+), 76 deletions(-) diff --git a/README.md b/README.md index 98bf28a..0b0781a 100644 --- a/README.md +++ b/README.md @@ -61,14 +61,18 @@ const reducer = combineReducers(Object.assign({}, reducers, { routing: routeReducer })) -// Sync dispatched route actions to the history +// specify the history to listen to const reduxRouterMiddleware = syncHistory(browserHistory) -const createStoreWithMiddleware = applyMiddleware(reduxRouterMiddleware)(createStore) - -const store = createStoreWithMiddleware(reducer) +const store = createStore( + reducer, + applyMiddleware(reduxRouterMiddleware) +) -// Required for replaying actions from devtools to work -reduxRouterMiddleware.listenForReplays(store) +// begin syncing +reduxRouterMiddleware.syncWith(store, { + urlToState: true, // route changes will appear in state + stateToUrl: false // set to true for time travel in DevTools +}) ReactDOM.render( @@ -140,7 +144,7 @@ Examples from the community: _Have an example to add? Send us a PR!_ -### API +### API (TODO) #### `syncHistory(history: History) => ReduxMiddleware` diff --git a/examples/basic/app.js b/examples/basic/app.js index 00f5b3d..894f352 100644 --- a/examples/basic/app.js +++ b/examples/basic/app.js @@ -27,12 +27,18 @@ const DevTools = createDevTools( ) -const finalCreateStore = compose( - applyMiddleware(middleware), - DevTools.instrument() -)(createStore) -const store = finalCreateStore(reducer) -middleware.listenForReplays(store) +const store = createStore( + reducer, + compose( + applyMiddleware(middleware), + DevTools.instrument() + ) +) + +middleware.syncWith(store, { + urlToState: true, + stateToUrl: true +}) ReactDOM.render( diff --git a/examples/basic/package.json b/examples/basic/package.json index bbebb67..e807752 100644 --- a/examples/basic/package.json +++ b/examples/basic/package.json @@ -9,8 +9,8 @@ "react-dom": "^0.14.2", "react-redux": "^4.0.0", "react-router": "^1.0.0", - "redux": "^3.0.4", - "react-router-redux": "^2.1.0" + "react-router-redux": "^2.1.0", + "redux": "^3.2.1" }, "devDependencies": { "babel-core": "^6.1.21", diff --git a/package.json b/package.json index a4ebdbf..925b20a 100644 --- a/package.json +++ b/package.json @@ -62,7 +62,7 @@ "karma-webpack": "^1.7.0", "mocha": "^2.3.4", "react": "^0.14.3", - "redux": "^3.0.4", + "redux": "^3.2.1", "redux-devtools": "^3.0.0", "redux-devtools-dock-monitor": "^1.0.1", "redux-devtools-log-monitor": "^1.0.1", diff --git a/src/index.js b/src/index.js index 8c62ec8..01dafad 100644 --- a/src/index.js +++ b/src/index.js @@ -49,19 +49,7 @@ export function syncHistory(history) { history.listen(location => { initialState.location = location })() - function middleware(store) { - unsubscribeHistory = history.listen(location => { - currentKey = location.key - if (syncing) { - // Don't dispatch a new action if we're replaying location. - return - } - - store.dispatch(updateLocation(location)) - }) - - connected = true - + function middleware() { return next => action => { if (action.type !== TRANSITION || !connected) { return next(action) @@ -72,41 +60,72 @@ export function syncHistory(history) { } } - middleware.listenForReplays = - (store, selectLocationState = SELECT_LOCATION) => { - const getLocationState = () => selectLocationState(store.getState()) - const initialLocation = getLocationState() - - unsubscribeStore = store.subscribe(() => { - const location = getLocationState() + middleware.syncWith = + (store, { + urlToState = false, + stateToUrl = false, + selectLocationState = SELECT_LOCATION + } = {}) => { + if (!urlToState && !stateToUrl) { + throw new Error( + 'At least one of "urlToState" and "stateToUrl" options must be true.' + ) + } - // 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.key === initialLocation.key) { - history.replace(initialLocation) - return + if (stateToUrl) { + const getLocationState = () => selectLocationState(store.getState()) + const initialLocation = getLocationState() + + const reconcileLocationWithState = () => { + const location = getLocationState() + + // 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.key === initialLocation.key) { + history.replace(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) { + syncing = true + history.transitionTo(location) + syncing = false + } } - // 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) { - syncing = true - history.transitionTo(location) - syncing = false + unsubscribeStore = store.subscribe(reconcileLocationWithState) + reconcileLocationWithState() + } + + if (urlToState) { + unsubscribeHistory = history.listen(location => { + currentKey = location.key + if (syncing) { + // Don't dispatch a new action if we're replaying location. + return + } + + store.dispatch(updateLocation(location)) + }) + + connected = true + } + + return () => { + if (stateToUrl) { + unsubscribeStore() } - }) - } - middleware.unsubscribe = () => { - unsubscribeHistory() - if (unsubscribeStore) { - unsubscribeStore() + if (urlToState) { + unsubscribeHistory() + connected = false + } + } } - connected = false - } - return middleware } diff --git a/test/createTests.js b/test/createTests.js index dcfbfad..77b73f9 100644 --- a/test/createTests.js +++ b/test/createTests.js @@ -28,15 +28,18 @@ expect.extend({ } }) -function createSyncedHistoryAndStore(createHistory) { +function createSyncedHistoryAndStore(createHistory, syncOptions, initialState) { const history = createHistory() const middleware = syncHistory(history) - const { unsubscribe } = middleware - - const createStoreWithMiddleware = applyMiddleware(middleware)(createStore) - const store = createStoreWithMiddleware(combineReducers({ + const reducer = combineReducers({ routing: routeReducer - })) + }) + const store = createStore( + reducer, + initialState, + applyMiddleware(middleware) + ) + const unsubscribe = middleware.syncWith(store, syncOptions) return { history, store, unsubscribe } } @@ -197,18 +200,17 @@ module.exports = function createTests(createHistory, name, reset = defaultReset) history.push('/foo') const middleware = syncHistory(history) - unsubscribe = middleware.unsubscribe - - const finalCreateStore = compose( + store = createStore(combineReducers({ + routing: routeReducer + }), compose( applyMiddleware(middleware), instrument() - )(createStore) - store = finalCreateStore(combineReducers({ - routing: routeReducer - })) + )) devToolsStore = store.liftedStore - - middleware.listenForReplays(store) + unsubscribe = middleware.syncWith(store, { + stateToUrl: true, + urlToState: true + }) }) afterEach(() => { @@ -273,11 +275,103 @@ module.exports = function createTests(createHistory, name, reset = defaultReset) }) }) + describe('initialState', () => { + it('does not respect initialState when syncing url to state', () => { + let synced = createSyncedHistoryAndStore(createHistory, { + urlToState: true + }, { + routing: { + location: { + pathname: '/init', + search: '', + hash: '', + state: null, + action: 'PUSH', + key: 'abcde' + } + } + }) + + let history = synced.history + let unsubscribe = synced.unsubscribe + + let currentPath + const historyUnsubscribe = history.listen(location => { + currentPath = location.pathname + }) + + expect(currentPath).toEqual('/') + historyUnsubscribe() + unsubscribe() + }) + + it('respects initialState when syncing state to url', () => { + let synced = createSyncedHistoryAndStore(createHistory, { + stateToUrl: true + }, { + routing: { + location: { + pathname: '/init', + search: '', + hash: '', + state: null, + action: 'PUSH', + key: 'abcde' + } + } + }) + + let history = synced.history + let unsubscribe = synced.unsubscribe + + let currentPath + const historyUnsubscribe = history.listen(location => { + currentPath = location.pathname + }) + + expect(currentPath).toEqual('/init') + historyUnsubscribe() + unsubscribe() + }) + + it('respects initialState when syncing both ways', () => { + let synced = createSyncedHistoryAndStore(createHistory, { + stateToUrl: true, + urlToState: true + }, { + routing: { + location: { + pathname: '/init', + search: '', + hash: '', + state: null, + action: 'PUSH', + key: 'abcde' + } + } + }) + + let history = synced.history + let unsubscribe = synced.unsubscribe + + let currentPath + const historyUnsubscribe = history.listen(location => { + currentPath = location.pathname + }) + + expect(currentPath).toEqual('/init') + historyUnsubscribe() + unsubscribe() + }) + }) + describe('syncReduxAndRouter', () => { let history, store, unsubscribe beforeEach(() => { - let synced = createSyncedHistoryAndStore(createHistory) + let synced = createSyncedHistoryAndStore(createHistory, { + urlToState: true + }) history = synced.history store = synced.store unsubscribe = synced.unsubscribe @@ -545,7 +639,9 @@ module.exports = function createTests(createHistory, name, reset = defaultReset) let history, store, unsubscribe beforeEach(() => { - const synced = createSyncedHistoryAndStore(useQueries(createHistory)) + const synced = createSyncedHistoryAndStore(useQueries(createHistory), { + urlToState: true + }) history = synced.history store = synced.store unsubscribe = synced.unsubscribe @@ -583,7 +679,8 @@ module.exports = function createTests(createHistory, name, reset = defaultReset) beforeEach(() => { const synced = createSyncedHistoryAndStore( - () => useBasename(createHistory)({ basename: '/foobar' }) + () => useBasename(createHistory)({ basename: '/foobar' }), + { urlToState: true } ) history = synced.history store = synced.store