From 9a2b0264ac0b3fd2637594545ee08b278e31e295 Mon Sep 17 00:00:00 2001 From: Jimmy Jia Date: Tue, 19 Jul 2016 11:07:00 -0400 Subject: [PATCH] Support history v3 (#3647) --- modules/Router.js | 14 +++----- modules/__tests__/serverRendering-test.js | 12 +++---- modules/__tests__/useRouterHistory-test.js | 40 +++++++++++++--------- modules/createTransitionManager.js | 21 +++++++++--- modules/match.js | 17 ++------- package.json | 2 +- 6 files changed, 53 insertions(+), 53 deletions(-) diff --git a/modules/Router.js b/modules/Router.js index ff91dc4fd2..8825ffa42f 100644 --- a/modules/Router.js +++ b/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( diff --git a/modules/__tests__/serverRendering-test.js b/modules/__tests__/serverRendering-test.js index 0b326bab4f..a8c2f29bf5 100644 --- a/modules/__tests__/serverRendering-test.js +++ b/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( ) @@ -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( ) - match({ history: createMemoryHistory('/async'), routes }, function (error, redirectLocation, renderProps) { + match({ history: createMemoryHistory('/async'), routes }, (error, redirectLocation, renderProps) => { const browserString = renderToStaticMarkup( ) diff --git a/modules/__tests__/useRouterHistory-test.js b/modules/__tests__/useRouterHistory-test.js index c2d3899115..2d5323d355 100644 --- a/modules/__tests__/useRouterHistory-test.js +++ b/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(( - ), 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') }) }) }) diff --git a/modules/createTransitionManager.js b/modules/createTransitionManager.js index 65ba4addb6..559c7cb3e6 100644 --- a/modules/createTransitionManager.js +++ b/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 { diff --git a/modules/match.js b/modules/match.js index 75ade46dda..8e89be4e14 100644 --- a/modules/match.js +++ b/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 - // and attaches another history listener. - if (unlisten) { - unlisten() - } }) } diff --git a/package.json b/package.json index b1e17d9ec5..506c1bab2b 100644 --- a/package.json +++ b/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",