Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support history v3 #3647

Merged
merged 1 commit into from
Jul 19, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 4 additions & 10 deletions modules/Router.js
Original file line number Diff line number Diff line change
Expand Up @@ -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

/**
Expand Down Expand Up @@ -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(
Expand Down
12 changes: 6 additions & 6 deletions modules/__tests__/serverRendering-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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} />
)
Expand All @@ -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} />
)
Expand Down
40 changes: 23 additions & 17 deletions modules/__tests__/useRouterHistory-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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')
})
})
})
21 changes: 17 additions & 4 deletions modules/createTransitionManager.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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 {
Expand Down
17 changes: 2 additions & 15 deletions modules/match.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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()
}
})
}

Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down