Support history v3 (#3647)

Author: taion

modules/Router.js

@@ -8,12 +8,6 @@ import { createRoutes } from './RouteUtils'
 import { createRouterObject, assignRouterState } from './RouterUtils'
 import warning from './routerWarning'
 
-/* istanbul ignore next: sanity check */
-function isUnsupportedHistory(history) {
-  // v3 histories expose getCurrentLocation, but aren't currently supported.
-  return history && history.getCurrentLocation
-}
-
 const { func, object } = React.PropTypes
 
 /**
@@ -82,10 +76,10 @@ const Router = React.createClass({
     const { routes, children } = this.props
 
     invariant(
-      !isUnsupportedHistory(history),
-      'You have provided a history object created with history v3.x. ' +
-      'This version of React Router is not compatible with v3 history ' +
-      'objects. Please use history v2.x instead.'
+      history.getCurrentLocation,
+      'You have provided a history object created with history v2.x or ' +
+      'earlier. This version of React Router is only compatible with v3 ' +
+      'history objects. Please upgrade to history v3.x.'
     )
 
     return createTransitionManager(

modules/__tests__/serverRendering-test.js

@@ -183,13 +183,13 @@ describe('server rendering', function () {
     })
   })
 
-  describe('server/client consistency', function () {
+  describe('server/client consistency', () => {
     // Just render to static markup here to avoid having to normalize markup.
 
-    it('should match for synchronous route', function () {
+    it('should match for synchronous route', () => {
       let serverString
 
-      match({ routes, location: '/dashboard' }, function (error, redirectLocation, renderProps) {
+      match({ routes, location: '/dashboard' }, (error, redirectLocation, renderProps) => {
         serverString = renderToStaticMarkup(
           <RouterContext {...renderProps} />
         )
@@ -202,13 +202,13 @@ describe('server rendering', function () {
       expect(browserString).toEqual(serverString)
     })
 
-    it('should match for asynchronous route', function (done) {
-      match({ routes, location: '/async' }, function (error, redirectLocation, renderProps) {
+    it('should match for asynchronous route', done => {
+      match({ routes, location: '/async' }, (error, redirectLocation, renderProps) => {
         const serverString = renderToStaticMarkup(
           <RouterContext {...renderProps} />
         )
 
-        match({ history: createMemoryHistory('/async'), routes }, function (error, redirectLocation, renderProps) {
+        match({ history: createMemoryHistory('/async'), routes }, (error, redirectLocation, renderProps) => {
           const browserString = renderToStaticMarkup(
             <Router {...renderProps} />
           )

modules/__tests__/useRouterHistory-test.js

@@ -8,8 +8,8 @@ import Redirect from '../Redirect'
 import Router from '../Router'
 import Route from '../Route'
 
-describe('useRouterHistory', function () {
-  it('passes along options, especially query parsing', function (done) {
+describe('useRouterHistory', () => {
+  it('passes along options, especially query parsing', done => {
     const history = useRouterHistory(createHistory)({
       stringifyQuery() {
         assert(true)
@@ -20,40 +20,46 @@ describe('useRouterHistory', function () {
     history.push({ pathname: '/', query: { test: true } })
   })
 
-  describe('when using basename', function () {
+  describe('when using basename', () => {
 
     let node
-    beforeEach(function () {
+    beforeEach(() => {
       node = document.createElement('div')
     })
 
-    afterEach(function () {
+    afterEach(() => {
       unmountComponentAtNode(node)
     })
 
-    it('should regard basename', function (done) {
-      const pathnames = []
-      const basenames = []
+    it('should regard basename', () => {
       const history = useRouterHistory(createHistory)({
         entries: '/foo/notes/5',
         basename: '/foo'
       })
-      history.listen(function (location) {
+
+      const pathnames = []
+      const basenames = []
+
+      const currentLocation = history.getCurrentLocation()
+      pathnames.push(currentLocation.pathname)
+      basenames.push(currentLocation.basename)
+
+      history.listen(location => {
         pathnames.push(location.pathname)
         basenames.push(location.basename)
       })
-      render((
+
+      const instance = render((
         <Router history={history}>
           <Route path="/messages/:id" />
           <Redirect from="/notes/:id" to="/messages/:id" />
         </Router>
-      ), node, function () {
-        expect(pathnames).toEqual([ '/notes/5', '/messages/5' ])
-        expect(basenames).toEqual([ '/foo', '/foo' ])
-        expect(this.state.location.pathname).toEqual('/messages/5')
-        expect(this.state.location.basename).toEqual('/foo')
-        done()
-      })
+      ), node)
+
+      expect(pathnames).toEqual([ '/notes/5', '/messages/5' ])
+      expect(basenames).toEqual([ '/foo', '/foo' ])
+      expect(instance.state.location.pathname).toEqual('/messages/5')
+      expect(instance.state.location.basename).toEqual('/foo')
     })
   })
 })

modules/createTransitionManager.js

@@ -216,9 +216,7 @@ export default function createTransitionManager(history, routes) {
    * gracefully handle errors and redirects.
    */
   function listen(listener) {
-    // TODO: Only use a single history listener. Otherwise we'll
-    // end up with multiple concurrent calls to match.
-    return history.listen(function (location) {
+    function historyListener(location) {
       if (state.location === location) {
         listener(null, state)
       } else {
@@ -238,7 +236,22 @@ export default function createTransitionManager(history, routes) {
           }
         })
       }
-    })
+    }
+
+    // TODO: Only use a single history listener. Otherwise we'll end up with
+    // multiple concurrent calls to match.
+
+    // Set up the history listener first in case the initial match redirects.
+    const unsubscribe = history.listen(historyListener)
+
+    if (state.location) {
+      // Picking up on a matchContext.
+      listener(null, state)
+    } else {
+      historyListener(history.getCurrentLocation())
+    }
+
+    return unsubscribe
   }
 
   return {

modules/match.js

@@ -26,20 +26,14 @@ function match({ history, routes, location, ...options }, callback) {
     createRoutes(routes)
   )
 
-  let unlisten
-
   if (location) {
     // Allow match({ location: '/the/path', ... })
     location = history.createLocation(location)
   } else {
-    // Pick up the location from the history via synchronous history.listen
-    // call if needed.
-    unlisten = history.listen(historyLocation => {
-      location = historyLocation
-    })
+    location = history.getCurrentLocation()
   }
 
-  transitionManager.match(location, function (error, redirectLocation, nextState) {
+  transitionManager.match(location, (error, redirectLocation, nextState) => {
     let renderProps
 
     if (nextState) {
@@ -52,13 +46,6 @@ function match({ history, routes, location, ...options }, callback) {
     }
 
     callback(error, redirectLocation, renderProps)
-
-    // Defer removing the listener to here to prevent DOM histories from having
-    // to unwind DOM event listeners unnecessarily, in case callback renders a
-    // <Router> and attaches another history listener.
-    if (unlisten) {
-      unlisten()
-    }
   })
 }
 

package.json

@@ -32,7 +32,7 @@
   ],
   "license": "MIT",
   "dependencies": {
-    "history": "^2.1.2",
+    "history": "^3.0.0",
     "hoist-non-react-statics": "^1.2.0",
     "invariant": "^2.2.1",
     "warning": "^3.0.0",