diff --git a/README.md b/README.md index f01b108..47bdc5e 100644 --- a/README.md +++ b/README.md @@ -72,10 +72,12 @@ it, and also change it with an action. Here's some code: ```js +import React from 'react' +import ReactDOM from 'react-dom' import { createStore, combineReducers } from 'redux' import { Provider } from 'react-redux' import { Router, Route } from 'react-router' -import createBrowserHistory from 'history/lib/createBrowserHistory' +import { createHistory } from 'history' import { syncReduxAndRouter, routeReducer } from 'redux-simple-router' import reducers from '/reducers' @@ -83,11 +85,11 @@ const reducer = combineReducers(Object.assign({}, reducers, { routing: routeReducer })) const store = createStore(reducer) -const history = createBrowserHistory() +const history = createHistory() syncReduxAndRouter(history, store) -React.render( +ReactDOM.render( diff --git a/examples/basic/app.js b/examples/basic/app.js index 0182eb8..a6213f8 100644 --- a/examples/basic/app.js +++ b/examples/basic/app.js @@ -1,10 +1,12 @@ const React = require('react'); const ReactDOM = require('react-dom'); -const { createStore, combineReducers } = require('redux'); +const { compose, createStore, combineReducers } = require('redux'); const { Provider } = require('react-redux'); const { Router, Route, IndexRoute } = require('react-router'); const createHistory = require('history/lib/createHashHistory'); const { syncReduxAndRouter, routeReducer } = require('redux-simple-router'); +import { devTools } from 'redux-devtools'; +const { DevTools, DebugPanel, LogMonitor } = require('redux-devtools/lib/react'); const reducers = require('./reducers'); const { App, Home, Foo, Bar } = require('./components'); @@ -12,20 +14,28 @@ const { App, Home, Foo, Bar } = require('./components'); const reducer = combineReducers(Object.assign({}, reducers, { routing: routeReducer })); -const store = createStore(reducer); +const finalCreateStore = compose( + devTools() +)(createStore); +const store = finalCreateStore(reducer); const history = createHistory(); syncReduxAndRouter(history, store); ReactDOM.render( - - - - - - - +
+ + + + + + + + + + +
, document.getElementById('mount') ); diff --git a/examples/basic/components/App.js b/examples/basic/components/App.js index cc87e2e..dbfa5b8 100644 --- a/examples/basic/components/App.js +++ b/examples/basic/components/App.js @@ -1,9 +1,9 @@ const React = require('react'); const { Link } = require('react-router'); const { connect } = require('react-redux'); -const { updatePath } = require('redux-simple-router'); +const { pushPath } = require('redux-simple-router'); -function App({ updatePath, children }) { +function App({ pushPath, children }) { return (
@@ -16,7 +16,7 @@ function App({ updatePath, children }) { Bar
- +
{children}
@@ -25,5 +25,5 @@ function App({ updatePath, children }) { module.exports = connect( null, - { updatePath } + { pushPath } )(App); diff --git a/examples/basic/package.json b/examples/basic/package.json index e9fac6c..4f74fb8 100644 --- a/examples/basic/package.json +++ b/examples/basic/package.json @@ -15,6 +15,10 @@ "babel-loader": "^6.2.0", "babel-preset-es2015": "^6.1.18", "babel-preset-react": "^6.1.18", + "redux-devtools": "^2.1.5", "webpack": "^1.12.6" + }, + "scripts": { + "start": "webpack --watch" } } diff --git a/examples/basic/webpack.config.js b/examples/basic/webpack.config.js index 78d1e45..6a42878 100644 --- a/examples/basic/webpack.config.js +++ b/examples/basic/webpack.config.js @@ -26,4 +26,9 @@ var fs = require('fs') if (fs.existsSync(src)) { // Use the latest src module.exports.resolve = { alias: { 'redux-simple-router': src } } + module.exports.module.loaders.push({ + test: /\.js$/, + loaders: ['babel'], + include: src + }); } diff --git a/package.json b/package.json index aae8dc0..0e88f44 100644 --- a/package.json +++ b/package.json @@ -3,6 +3,7 @@ "version": "0.0.10", "description": "Ruthlessly simple bindings to keep react-router and redux in sync", "main": "lib/index", + "jsnext:main": "src/index", "repository": { "type": "git", "url": "https://github.com/jlongster/redux-simple-router.git" @@ -53,6 +54,7 @@ "karma-webpack": "^1.7.0", "mocha": "^2.3.4", "redux": "^3.0.4", + "redux-devtools": "^2.1.5", "webpack": "^1.12.9" }, "dependencies": { diff --git a/src/index.js b/src/index.js index 8d62e53..1cf003b 100644 --- a/src/index.js +++ b/src/index.js @@ -1,13 +1,26 @@ -const deepEqual = require('deep-equal'); +import deepEqual from 'deep-equal'; // Constants -const UPDATE_PATH = "@@router/UPDATE_PATH"; +const INIT_PATH = "@@router/INIT_PATH"; +export const UPDATE_PATH = "@@router/UPDATE_PATH"; const SELECT_STATE = state => state.routing; -// Action creator +// Action creators -function pushPath(path, state, { avoidRouterUpdate = false } = {}) { +function initPath(path, state) { + return { + type: INIT_PATH, + payload: { + path: path, + state: state, + replace: false, + avoidRouterUpdate: true + } + }; +} + +export function pushPath(path, state, { avoidRouterUpdate = false } = {}) { return { type: UPDATE_PATH, payload: { @@ -19,7 +32,7 @@ function pushPath(path, state, { avoidRouterUpdate = false } = {}) { }; } -function replacePath(path, state, { avoidRouterUpdate = false } = {}) { +export function replacePath(path, state, { avoidRouterUpdate = false } = {}) { return { type: UPDATE_PATH, payload: { @@ -33,7 +46,7 @@ function replacePath(path, state, { avoidRouterUpdate = false } = {}) { // Reducer -const initialState = { +let initialState = { changeId: 1, path: undefined, state: undefined, @@ -41,7 +54,7 @@ const initialState = { }; function update(state=initialState, { type, payload }) { - if(type === UPDATE_PATH) { + if(type === INIT_PATH || type === UPDATE_PATH) { return Object.assign({}, state, { path: payload.path, changeId: state.changeId + (payload.avoidRouterUpdate ? 0 : 1), @@ -55,12 +68,20 @@ function update(state=initialState, { type, payload }) { // Syncing function locationsAreEqual(a, b) { - return a.path === b.path && deepEqual(a.state, b.state); + return a != null && b != null && a.path === b.path && deepEqual(a.state, b.state); } -function syncReduxAndRouter(history, store, selectRouterState = SELECT_STATE) { +export function syncReduxAndRouter(history, store, selectRouterState = SELECT_STATE) { const getRouterState = () => selectRouterState(store.getState()); - let lastChangeId = 0; + + // 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( @@ -75,30 +96,50 @@ function syncReduxAndRouter(history, store, selectRouterState = SELECT_STATE) { state: location.state }; - // 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(locationsAreEqual(getRouterState(), route)) return; - - const updatePath = location.action === 'REPLACE' - ? replacePath - : pushPath; - - store.dispatch(updatePath(route.path, route.state, { avoidRouterUpdate: true })); + 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)) { + // 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(() => { - const routing = getRouterState(); - - // Only update the router once per `pushPath` call. This is - // indicated by the `changeId` state; when that number changes, we - // should update the history. - if(lastChangeId === routing.changeId) return; + let routing = getRouterState(); - lastChangeId = routing.changeId; + // Only trigger history update if this is a new change or the + // location has changed. + if(lastRoute.changeId !== routing.changeId || + !locationsAreEqual(lastRoute, routing)) { - const method = routing.replace ? 'replaceState' : 'pushState'; + lastRoute = routing; + const method = routing.replace ? 'replaceState' : 'pushState'; + history[method](routing.state, routing.path); + } - history[method](routing.state, routing.path); }); return function unsubscribe() { @@ -107,10 +148,4 @@ function syncReduxAndRouter(history, store, selectRouterState = SELECT_STATE) { }; } -module.exports = { - UPDATE_PATH, - pushPath, - replacePath, - syncReduxAndRouter, - routeReducer: update -}; +export { update as routeReducer }; diff --git a/test/createTests.js b/test/createTests.js index e05b9ad..f52d4a6 100644 --- a/test/createTests.js +++ b/test/createTests.js @@ -1,6 +1,27 @@ const expect = require('expect'); const { pushPath, replacePath, UPDATE_PATH, routeReducer, syncReduxAndRouter } = require('../src/index'); -const { createStore, combineReducers } = require('redux'); +const { createStore, combineReducers, compose } = require('redux'); +const { devTools } = require('redux-devtools'); +const { ActionCreators } = require('redux-devtools/lib/devTools'); + +expect.extend({ + toContainRoute({ + path, + state = undefined, + replace = false, + changeId = undefined + }) { + 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({ @@ -22,22 +43,22 @@ module.exports = function createTests(createHistory, name, reset = defaultReset) it('creates actions', () => { expect(pushPath('/foo', { bar: 'baz' })).toEqual({ type: UPDATE_PATH, - payload: { - path: '/foo', - replace: false, - state: { bar: 'baz' }, - avoidRouterUpdate: false + payload: { + path: '/foo', + replace: false, + state: { bar: 'baz' }, + avoidRouterUpdate: false } }); expect(pushPath('/foo', undefined, { avoidRouterUpdate: true })).toEqual({ type: UPDATE_PATH, payload: { - path: '/foo', - state: undefined, - replace: false, - avoidRouterUpdate: true - } + path: '/foo', + state: undefined, + replace: false, + avoidRouterUpdate: true + } }); }); }); @@ -47,31 +68,31 @@ module.exports = function createTests(createHistory, name, reset = defaultReset) expect(replacePath('/foo', { bar: 'baz' })).toEqual({ type: UPDATE_PATH, payload: { - path: '/foo', - replace: true, - state: { bar: 'baz' }, - avoidRouterUpdate: false - } + path: '/foo', + replace: true, + state: { bar: 'baz' }, + avoidRouterUpdate: false + } }); expect(replacePath('/foo', undefined, { avoidRouterUpdate: true })).toEqual({ type: UPDATE_PATH, payload: { - path: '/foo', - state: undefined, - replace: true, - avoidRouterUpdate: true - } + path: '/foo', + state: undefined, + replace: true, + avoidRouterUpdate: true + } }); expect(replacePath('/foo', undefined, { avoidRouterUpdate: false })).toEqual({ type: UPDATE_PATH, payload: { - path: '/foo', - state: undefined, - replace: true, - avoidRouterUpdate: false - } + path: '/foo', + state: undefined, + replace: true, + avoidRouterUpdate: false + } }); }); }); @@ -86,9 +107,9 @@ module.exports = function createTests(createHistory, name, reset = defaultReset) expect(routeReducer(state, { type: UPDATE_PATH, payload: { - path: '/bar', - replace: false - } + path: '/bar', + replace: false + } })).toEqual({ path: '/bar', replace: false, @@ -101,10 +122,10 @@ module.exports = function createTests(createHistory, name, reset = defaultReset) expect(routeReducer(state, { type: UPDATE_PATH, payload: { - path: '/bar', - replace: true, - avoidRouterUpdate: false - } + path: '/bar', + replace: true, + avoidRouterUpdate: false + } })).toEqual({ path: '/bar', replace: true, @@ -117,10 +138,10 @@ module.exports = function createTests(createHistory, name, reset = defaultReset) expect(routeReducer(state, { type: UPDATE_PATH, payload: { - path: '/bar', - replace: false, - avoidRouterUpdate: true - } + path: '/bar', + replace: false, + avoidRouterUpdate: true + } })).toEqual({ path: '/bar', replace: false, @@ -130,6 +151,89 @@ module.exports = function createTests(createHistory, name, reset = defaultReset) }); }); + // To ensure that "Revert" and toggling actions work as expected in + // Redux DevTools we need a couple of tests for it. In these tests we + // rely directly on the DevTools, as they implement these actions as + // middleware, and we don't want to implement this ourselves. + describe('devtools', () => { + let history, store, devToolsStore, unsubscribe; + + beforeEach(() => { + history = createHistory(); + const finalCreateStore = compose(devTools())(createStore); + store = finalCreateStore(combineReducers({ + routing: routeReducer + })); + devToolsStore = store.devToolsStore; + + // Set initial URL before syncing + history.pushState(null, '/foo'); + + unsubscribe = syncReduxAndRouter(history, store); + }); + + afterEach(() => { + unsubscribe(); + }); + + it('resets to the initial url', () => { + let currentPath; + const historyUnsubscribe = history.listen(location => { + currentPath = location.pathname; + }); + + history.pushState(null, '/bar'); + store.dispatch(pushPath('/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(currentPath).toEqual('/foo'); + + historyUnsubscribe(); + }); + + it('handles toggle after store change', () => { + let currentPath; + const historyUnsubscribe = history.listen(location => { + currentPath = location.pathname; + }); + + // DevTools action #2 + history.pushState(null, '/foo2'); + // DevTools action #3 + history.pushState(null, '/foo3'); + + // When we toggle an action, the devtools will revert the action + // and we therefore expect the history to update to the previous path + devToolsStore.dispatch(ActionCreators.toggleAction(3)); + expect(currentPath).toEqual('/foo2'); + + historyUnsubscribe(); + }); + + it('handles toggle after store change', () => { + let currentPath; + const historyUnsubscribe = history.listen(location => { + currentPath = location.pathname; + }); + + // DevTools action #2 + store.dispatch(pushPath('/foo2')); + // DevTools action #3 + store.dispatch(pushPath('/foo3')); + + // When we toggle an action, the devtools will revert the action + // and we therefore expect the history to update to the previous path + devToolsStore.dispatch(ActionCreators.toggleAction(3)); + expect(currentPath).toEqual('/foo2'); + + historyUnsubscribe(); + }); + }); + describe('syncReduxAndRouter', () => { let history, store, unsubscribe; @@ -145,101 +249,112 @@ module.exports = function createTests(createHistory, name, reset = defaultReset) }); it('syncs router -> redux', () => { - expect(store.getState().routing.path).toEqual('/'); + expect(store).toContainRoute({ + path: '/' + }); history.pushState(null, '/foo'); - expect(store.getState().routing.path).toEqual('/foo'); - expect(store.getState().routing.state).toBe(null); - expect(store.getState().routing.replace).toBe(false); + expect(store).toContainRoute({ + path: '/foo', + replace: false, + state: null + }); history.pushState({ bar: 'baz' }, '/foo'); - expect(store.getState().routing.path).toEqual('/foo'); - expect(store.getState().routing.state).toEqual({ bar: 'baz' }); - expect(store.getState().routing.replace).toBe(true); + expect(store).toContainRoute({ + path: '/foo', + replace: true, + state: { bar: 'baz' } + }); history.replaceState(null, '/bar'); - expect(store.getState().routing.path).toEqual('/bar'); - expect(store.getState().routing.state).toBe(null); - expect(store.getState().routing.replace).toBe(true); + expect(store).toContainRoute({ + path: '/bar', + replace: true, + state: null + }); history.pushState(null, '/bar'); - expect(store.getState().routing.path).toEqual('/bar'); - expect(store.getState().routing.state).toBe(null); - expect(store.getState().routing.replace).toBe(true); + expect(store).toContainRoute({ + path: '/bar', + replace: true, + state: null + }); history.pushState(null, '/bar?query=1'); - expect(store.getState().routing.path).toEqual('/bar?query=1'); - expect(store.getState().routing.replace).toBe(false); + expect(store).toContainRoute({ + path: '/bar?query=1', + replace: false, + state: null + }); history.replaceState({ bar: 'baz' }, '/bar?query=1'); - expect(store.getState().routing.path).toEqual('/bar?query=1'); - expect(store.getState().routing.state).toEqual({ bar: 'baz' }); - expect(store.getState().routing.replace).toBe(true); + expect(store).toContainRoute({ + path: '/bar?query=1', + replace: true, + state: { bar: 'baz' } + }); history.pushState({ bar: 'baz' }, '/bar?query=1#hash=2'); - expect(store.getState().routing.path).toEqual('/bar?query=1#hash=2'); - expect(store.getState().routing.replace).toBe(true); + expect(store).toContainRoute({ + path: '/bar?query=1#hash=2', + replace: true, + state: { bar: 'baz' } + }); }); it('syncs redux -> router', () => { - expect(store.getState().routing).toEqual({ + expect(store).toContainRoute({ path: '/', - changeId: 1, replace: false, state: undefined }); store.dispatch(pushPath('/foo')); - expect(store.getState().routing).toEqual({ + expect(store).toContainRoute({ path: '/foo', - changeId: 2, replace: false, state: undefined }); store.dispatch(pushPath('/foo', { bar: 'baz' })); - expect(store.getState().routing).toEqual({ + expect(store).toContainRoute({ path: '/foo', - changeId: 3, replace: false, state: { bar: 'baz' } }); store.dispatch(replacePath('/bar', { bar: 'foo' })); - expect(store.getState().routing).toEqual({ + expect(store).toContainRoute({ path: '/bar', - changeId: 4, replace: true, state: { bar: 'foo' } }); store.dispatch(pushPath('/bar')); - expect(store.getState().routing).toEqual({ + expect(store).toContainRoute({ path: '/bar', - changeId: 5, replace: false, state: undefined }); store.dispatch(pushPath('/bar?query=1')); - expect(store.getState().routing).toEqual({ + expect(store).toContainRoute({ path: '/bar?query=1', - changeId: 6, replace: false, state: undefined }); store.dispatch(pushPath('/bar?query=1#hash=2')); - expect(store.getState().routing).toEqual({ + expect(store).toContainRoute({ path: '/bar?query=1#hash=2', - changeId: 7, replace: false, state: undefined }); }); it('updates the router even if path is the same', () => { - expect(store.getState().routing).toEqual({ + expect(store).toContainRoute({ path: '/', changeId: 1, replace: false, @@ -247,7 +362,7 @@ module.exports = function createTests(createHistory, name, reset = defaultReset) }); store.dispatch(pushPath('/foo')); - expect(store.getState().routing).toEqual({ + expect(store).toContainRoute({ path: '/foo', changeId: 2, replace: false, @@ -255,7 +370,7 @@ module.exports = function createTests(createHistory, name, reset = defaultReset) }); store.dispatch(pushPath('/foo')); - expect(store.getState().routing).toEqual({ + expect(store).toContainRoute({ path: '/foo', changeId: 3, replace: false, @@ -263,7 +378,7 @@ module.exports = function createTests(createHistory, name, reset = defaultReset) }); store.dispatch(replacePath('/foo')); - expect(store.getState().routing).toEqual({ + expect(store).toContainRoute({ path: '/foo', changeId: 4, replace: true, @@ -281,7 +396,7 @@ module.exports = function createTests(createHistory, name, reset = defaultReset) } }); - expect(store.getState().routing).toEqual({ + expect(store).toContainRoute({ path: '/', changeId: 1, replace: false, @@ -290,7 +405,7 @@ module.exports = function createTests(createHistory, name, reset = defaultReset) }); it('only updates the router once when dispatching from `listenBefore`', () => { - expect(store.getState().routing).toEqual({ + expect(store).toContainRoute({ path: '/', changeId: 1, replace: false, @@ -310,7 +425,7 @@ module.exports = function createTests(createHistory, name, reset = defaultReset) }); store.dispatch(pushPath('/foo')); - expect(store.getState().routing).toEqual({ + expect(store).toContainRoute({ path: '/foo', changeId: 2, replace: false, @@ -328,12 +443,13 @@ module.exports = function createTests(createHistory, name, reset = defaultReset) store.dispatch(pushPath('/foo')); store.dispatch(pushPath('/foo')); store.dispatch(pushPath('/foo', { bar: 'baz' })); + history.pushState({ foo: 'bar' }, '/foo'); store.dispatch(replacePath('/bar')); store.dispatch(replacePath('/bar', { bar: 'foo' })); unsubscribeFromStore(); - expect(updates.length).toBe(5); + expect(updates.length).toBe(6); expect(updates).toEqual([ { routing: { @@ -359,6 +475,14 @@ module.exports = function createTests(createHistory, name, reset = defaultReset) replace: false } }, + { + routing: { + changeId: 4, + path: '/foo', + state: { foo: 'bar' }, + replace: true + } + }, { routing: { changeId: 5, @@ -379,16 +503,13 @@ module.exports = function createTests(createHistory, name, reset = defaultReset) }); it('allows updating the route from within `listenBefore`', () => { - expect(store.getState().routing).toEqual({ - path: '/', - changeId: 1, - replace: false, - state: undefined + expect(store).toContainRoute({ + path: '/' }); history.listenBefore(location => { if(location.pathname === '/foo') { - expect(store.getState().routing).toEqual({ + expect(store).toContainRoute({ path: '/foo', changeId: 2, replace: false, @@ -397,7 +518,7 @@ module.exports = function createTests(createHistory, name, reset = defaultReset) store.dispatch(pushPath('/bar')); } else if(location.pathname === '/replace') { - expect(store.getState().routing).toEqual({ + expect(store).toContainRoute({ path: '/replace', changeId: 4, replace: false, @@ -408,7 +529,7 @@ module.exports = function createTests(createHistory, name, reset = defaultReset) }); store.dispatch(pushPath('/foo')); - expect(store.getState().routing).toEqual({ + expect(store).toContainRoute({ path: '/bar', changeId: 3, replace: false, @@ -416,7 +537,7 @@ module.exports = function createTests(createHistory, name, reset = defaultReset) }); store.dispatch(pushPath('/replace', { bar: 'baz' })); - expect(store.getState().routing).toEqual({ + expect(store).toContainRoute({ path: '/baz', changeId: 5, replace: true, @@ -452,10 +573,12 @@ module.exports = function createTests(createHistory, name, reset = defaultReset) const unsubscribe = syncReduxAndRouter(history, store) history.pushState(null, '/foo'); - expect(store.getState().routing.path).toEqual('/foo'); + expect(store).toContainRoute({ + path: '/foo' + }); store.dispatch(pushPath('/bar')); - expect(store.getState().routing).toEqual({ + expect(store).toContainRoute({ path: '/bar', changeId: 2, replace: false, @@ -465,7 +588,9 @@ module.exports = function createTests(createHistory, name, reset = defaultReset) unsubscribe(); history.pushState(null, '/foo'); - expect(store.getState().routing.path).toEqual('/bar'); + expect(store).toContainRoute({ + path: '/bar' + }); history.listenBefore(location => { throw new Error()