Skip to content

Commit

Permalink
Explicit error on <Route> outside <Router> (#4939)
Browse files Browse the repository at this point in the history
* Explicit error on <Route> outside <Router>

When using <Route> outside a valid <Router>, the message is not that explicit. It throws:

`Cannot read property 'route' of undefined`

Even though it's not that bad, I propose we make it even more explicit. Can't be bad to know where the error is coming from. If you made a typo when importing the router, it's not obvious otherwise.

This will throw an error rather than a warning because the library code would crash otherwise.

* Add unit test

* Fix unit test change

* Add router check before first usage on <Router />

* Switch throw to invariant for <Router> check

* Add <Router> check on <Link>, <Switch>, <Redirect>, <Prompt>
  • Loading branch information
eXon authored and timdorr committed Jul 14, 2017
1 parent e1e994d commit fabcecc
Show file tree
Hide file tree
Showing 9 changed files with 75 additions and 5 deletions.
6 changes: 6 additions & 0 deletions packages/react-router-dom/modules/Link.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import React from 'react'
import PropTypes from 'prop-types'
import invariant from 'invariant'

const isModifiedEvent = (event) =>
!!(event.metaKey || event.altKey || event.ctrlKey || event.shiftKey)
Expand Down Expand Up @@ -58,6 +59,11 @@ class Link extends React.Component {
render() {
const { replace, to, innerRef, ...props } = this.props // eslint-disable-line no-unused-vars

invariant(
this.context.router,
'You should not use <Link> outside a valid <Router>'
)

const href = this.context.router.history.createHref(
typeof to === 'string' ? { pathname: to } : to
)
Expand Down
9 changes: 9 additions & 0 deletions packages/react-router-dom/modules/__tests__/Link-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,15 @@ describe('A <Link>', () => {
expect(href).toEqual('/the/path?the=query#the-hash')
})

it('crashes explicitly with no valid <Router>', () => {
const node = document.createElement('div')

expect(() => {
ReactDOM.render((
<Link to="/">link</Link>
), node)
}).toThrow(/You should not use <Link> outside a valid <Router>/)

it('exposes its ref via an innerRef prop', done => {
const node = document.createElement('div')

Expand Down
1 change: 1 addition & 0 deletions packages/react-router-dom/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@
},
"dependencies": {
"history": "^4.5.1",
"invariant": "^2.2.2",
"loose-envify": "^1.3.1",
"prop-types": "^15.5.4",
"react-router": "^4.1.1"
Expand Down
6 changes: 6 additions & 0 deletions packages/react-router/modules/Prompt.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import React from 'react'
import PropTypes from 'prop-types'
import invariant from 'invariant'

/**
* The public API for prompting the user before navigating away
Expand Down Expand Up @@ -41,6 +42,11 @@ class Prompt extends React.Component {
}

componentWillMount() {
invariant(
this.context.router,
'You should not use <Prompt> outside a valid <Router>'
)

if (this.props.when)
this.enable(this.props.message)
}
Expand Down
6 changes: 6 additions & 0 deletions packages/react-router/modules/Redirect.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import React from 'react'
import PropTypes from 'prop-types'
import invariant from 'invariant'

/**
* The public API for updating the location programatically
Expand Down Expand Up @@ -34,6 +35,11 @@ class Redirect extends React.Component {
}

componentWillMount() {
invariant(
this.context.router,
'You should not use <Redirect> outside a valid <Router>'
)

if (this.isStatic())
this.perform()
}
Expand Down
15 changes: 11 additions & 4 deletions packages/react-router/modules/Route.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import warning from 'warning'
import invariant from 'invariant';
import React from 'react'
import PropTypes from 'prop-types'
import matchPath from './matchPath'
Expand Down Expand Up @@ -49,10 +50,16 @@ class Route extends React.Component {
match: this.computeMatch(this.props, this.context.router)
}

computeMatch({ computedMatch, location, path, strict, exact }, { route }) {
computeMatch({ computedMatch, location, path, strict, exact }, router) {
if (computedMatch)
return computedMatch // <Switch> already computed the match for us

invariant(
router,
'You should not use <Route> or withRouter() outside a valid <Router>'
)

const { route } = router
const pathname = (location || route.location).pathname

return path ? matchPath(pathname, { path, strict, exact }) : route.match
Expand All @@ -63,17 +70,17 @@ class Route extends React.Component {

warning(
!(component && render),
'You should not use <Route component> and <Route render> in the same route; <Route render> will be ignored'
'You should not use <Route component> and <Route render> in the same route; <Route render> will be ignored'
)

warning(
!(component && children),
'You should not use <Route component> and <Route children> in the same route; <Route children> will be ignored'
'You should not use <Route component> and <Route children> in the same route; <Route children> will be ignored'
)

warning(
!(render && children),
'You should not use <Route render> and <Route children> in the same route; <Route children> will be ignored'
'You should not use <Route render> and <Route children> in the same route; <Route children> will be ignored'
)
}

Expand Down
8 changes: 8 additions & 0 deletions packages/react-router/modules/Switch.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import React from 'react'
import PropTypes from 'prop-types'
import warning from 'warning'
import invariant from 'invariant'
import matchPath from './matchPath'

/**
Expand All @@ -18,6 +19,13 @@ class Switch extends React.Component {
location: PropTypes.object
}

componentWillMount() {
invariant(
this.context.router,
'You should not use <Switch> outside a valid <Router>'
)
}

componentWillReceiveProps(nextProps) {
warning(
!(nextProps.location && !this.props.location),
Expand Down
12 changes: 11 additions & 1 deletion packages/react-router/modules/__tests__/Route-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,16 @@ describe('A <Route>', () => {
push('/sushi/spicy-tuna')
expect(node.innerHTML).toContain('/sushi/spicy-tuna')
})

it('crash explicitly with no valid <Router>', () => {
const node = document.createElement('div')

expect(() => {
ReactDOM.render((
<Route path="/" render={() => null} />
), node)
}).toThrow(/You should not use <Route> or withRouter\(\) outside a valid <Router>/)
})
})

describe('A <Route> with dynamic segments in the path', () => {
Expand Down Expand Up @@ -342,7 +352,7 @@ describe('A <Route location>', () => {

expect(node.innerHTML).toContain(TEXT)
})

it('continues to use parent\'s prop location after navigation', () => {
const TEXT = 'cheddar pretzel'
const node = document.createElement('div')
Expand Down
17 changes: 17 additions & 0 deletions packages/react-router/modules/__tests__/Switch-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,23 @@ describe('A <Switch>', () => {

expect(node.innerHTML).toMatch(/one/)
})

it('crash explicitly with no valid <Router>', () => {
const node = document.createElement('div')

expect(() => {
ReactDOM.render((
<Switch>
<Route path="/one" render={() => (
<h1>one</h1>
)}/>
<Route path="/two" render={() => (
<h1>two</h1>
)}/>
</Switch>
), node)
}).toThrow(/You should not use <Switch> outside a valid <Router>/)
})
})


Expand Down

0 comments on commit fabcecc

Please sign in to comment.