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

Issue #5114 warn about history prop in Router-derived components #5151

Merged
merged 7 commits into from
Jun 21, 2017
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
7 changes: 7 additions & 0 deletions packages/react-router-dom/modules/BrowserRouter.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import warning from 'warning'
import React from 'react'
import PropTypes from 'prop-types'
import createHistory from 'history/createBrowserHistory'
Expand All @@ -18,6 +19,12 @@ class BrowserRouter extends React.Component {
history = createHistory(this.props)

render() {
warning(
!this.props.history,
'<BrowserRouter> ignores the history prop. To use a custom history, ' +
'make sure you are using `import { Router }` and not `import { BrowserRouter as Router }`.'
)

return <Router history={this.history} children={this.props.children}/>
}
}
Expand Down
7 changes: 7 additions & 0 deletions packages/react-router-dom/modules/HashRouter.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import warning from 'warning'
import React from 'react'
import PropTypes from 'prop-types'
import createHistory from 'history/createHashHistory'
Expand All @@ -17,6 +18,12 @@ class HashRouter extends React.Component {
history = createHistory(this.props)

render() {
warning(
!this.props.history,
'<HashRouter> ignores the history prop. To use a custom history, ' +
'make sure you are using `import { Router }` and not `import { HashRouter as Router }`.'
)

return <Router history={this.history} children={this.props.children}/>
}
}
Expand Down
15 changes: 15 additions & 0 deletions packages/react-router-dom/modules/__tests__/BrowserRouter-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,4 +26,19 @@ describe('A <BrowserRouter>', () => {

expect(history).toBeAn('object')
})

it('warns when passed a history prop', () => {
const history = {}
const node = document.createElement('div')
expect.spyOn(console, 'error')

ReactDOM.render((
<BrowserRouter history={history} />
), node)

expect(console.error.calls.length).toBe(1)
expect(console.error.calls[0].arguments[0]).toMatch(
/<BrowserRouter.*import \{ Router }/)
expect.restoreSpies()
})
})
15 changes: 15 additions & 0 deletions packages/react-router-dom/modules/__tests__/HashRouter-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,4 +26,19 @@ describe('A <HashRouter>', () => {

expect(history).toBeAn('object')
})

it('warns when passed a history prop', () => {
const history = {}
const node = document.createElement('div')
expect.spyOn(console, 'error')

ReactDOM.render((
<HashRouter history={history} />
), node)

expect(console.error.calls.length).toBe(1)
expect(console.error.calls[0].arguments[0]).toMatch(
/<HashRouter.*import \{ Router }/)
expect.restoreSpies()
})
})
7 changes: 7 additions & 0 deletions packages/react-router/modules/MemoryRouter.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import warning from 'warning'
import React from 'react'
import PropTypes from 'prop-types'
import createHistory from 'history/createMemoryHistory'
Expand All @@ -18,6 +19,12 @@ class MemoryRouter extends React.Component {
history = createHistory(this.props)

render() {
warning(
!this.props.history,
'<MemoryRouter> ignores the history prop. To use a custom history, ' +
'make sure you are using `import { Router }` and not `import { MemoryRouter as Router }`.'
)

return <Router history={this.history} children={this.props.children}/>
}
}
Expand Down
7 changes: 7 additions & 0 deletions packages/react-router/modules/StaticRouter.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import warning from 'warning'
import invariant from 'invariant'
import React from 'react'
import PropTypes from 'prop-types'
Expand Down Expand Up @@ -127,6 +128,12 @@ class StaticRouter extends React.Component {
block: this.handleBlock
}

warning(
!this.props.history,
'<StaticRouter> ignores the history prop. To use a custom history, ' +
'make sure you are using `import { Router }` and not `import { StaticRouter as Router }`.'
)

return <Router {...props} history={history}/>
}
}
Expand Down
15 changes: 15 additions & 0 deletions packages/react-router/modules/__tests__/MemoryRouter-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,4 +26,19 @@ describe('A <MemoryRouter>', () => {

expect(history).toBeAn('object')
})

it('warns when passed a history prop', () => {
const history = {}
const node = document.createElement('div')
expect.spyOn(console, 'error')

ReactDOM.render((
<MemoryRouter history={history} />
), node)

expect(console.error.calls.length).toBe(1)
expect(console.error.calls[0].arguments[0]).toMatch(
/<MemoryRouter.*import \{ Router }/)
expect.restoreSpies()
})
})
17 changes: 17 additions & 0 deletions packages/react-router/modules/__tests__/StaticRouter-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,22 @@ describe('A <StaticRouter>', () => {
)
})

it('warns when passed a history prop', () => {
const context = {}
const history = {}
const node = document.createElement('div')
expect.spyOn(console, 'error')

ReactDOM.render((
<StaticRouter context={context} history={history} />
), node)

expect(console.error.calls.length).toBe(1)
expect(console.error.calls[0].arguments[0]).toMatch(
/<StaticRouter.*import \{ Router }/)
expect.restoreSpies()
})

it('reports PUSH actions on the context object', () => {
const context = {}

Expand Down Expand Up @@ -211,4 +227,5 @@ describe('A <StaticRouter>', () => {
}).toNotThrow()
})
})

})