Skip to content

Commit

Permalink
Redirect: Support props changes on Redirect to handle edge cases
Browse files Browse the repository at this point in the history
  • Loading branch information
alexilyaev committed Jul 17, 2017
1 parent ce52721 commit 2d6e1f6
Show file tree
Hide file tree
Showing 2 changed files with 122 additions and 2 deletions.
24 changes: 22 additions & 2 deletions packages/react-router/modules/Redirect.js
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
import React from 'react'
import PropTypes from 'prop-types'
import warning from 'warning'
import invariant from 'invariant'
import { locationsAreEqual } from 'history'

/**
* The public API for updating the location programatically
* The public API for updating the location programmatically
* with a component.
*/
class Redirect extends React.Component {
Expand All @@ -13,7 +15,7 @@ class Redirect extends React.Component {
to: PropTypes.oneOfType([
PropTypes.string,
PropTypes.object
])
]).isRequired
}

static defaultProps = {
Expand Down Expand Up @@ -49,6 +51,24 @@ class Redirect extends React.Component {
this.perform()
}

componentDidUpdate(prevProps) {
let prevTo = prevProps.to
let nextTo = this.props.to

prevTo = typeof prevTo === 'string' ? { pathname: prevTo } : prevTo
nextTo = typeof nextTo === 'string' ? { pathname: nextTo } : nextTo

if (locationsAreEqual(prevTo, nextTo))
return warning(
false,
`You tried to redirect to the same route you're currently on: "%s%s"`,
nextTo.pathname,
nextTo.search || ''
)

this.perform()
}

perform() {
const { history } = this.context.router
const { push, to } = this.props
Expand Down
100 changes: 100 additions & 0 deletions packages/react-router/modules/__tests__/Switch-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,106 @@ describe('A <Switch>', () => {
expect(node.innerHTML).toContain('bub')
})

it('handles subsequent redirects', () => {
const node = document.createElement('div')

ReactDOM.render((
<MemoryRouter initialEntries={[ '/one' ]}>
<Switch>
<Redirect exact from="/one" to="/two"/>
<Redirect exact from="/two" to="/three"/>

<Route path="/three" render={() => <div>three</div>}/>
</Switch>
</MemoryRouter>
), node)

expect(node.innerHTML).toContain('three')
})

it('warns when redirecting to same route, both strings', () => {
const node = document.createElement('div')
let redirected = false

expect.spyOn(console, 'error')

ReactDOM.render((
<MemoryRouter initialEntries={[ '/one' ]}>
<Switch>
<Route path="/one" render={() => {
if (!redirected) {
return <Redirect to="/one"/>
}

return <Redirect to='/one'/>
}}/>
</Switch>
</MemoryRouter>
), node)

expect(console.error.calls.length).toBe(1)
expect(console.error.calls[0].arguments[0]).toMatch(/Warning:.*"\/one"/)
expect.restoreSpies()
})

it('warns when redirecting to same route, mixed types', () => {
const node = document.createElement('div')
let redirected = false

expect.spyOn(console, 'error')

ReactDOM.render((
<MemoryRouter initialEntries={[ '/one' ]}>
<Switch>
<Route path="/one" render={() => {
if (!redirected) {
redirected = true
return <Redirect to="/one"/>
}

return <Redirect to={{pathname: '/one'}}/>
}}/>
</Switch>
</MemoryRouter>
), node)

expect(console.error.calls.length).toBe(1)
expect(console.error.calls[0].arguments[0]).toMatch(/Warning:.*"\/one"/)
expect.restoreSpies()
})

it('does NOT warn when redirecting to same route with different `search`', () => {
const node = document.createElement('div')
let redirected = false
let done = false

expect.spyOn(console, 'error')

ReactDOM.render((
<MemoryRouter initialEntries={[ '/one' ]}>
<Switch>
<Route path="/one" render={() => {
if (done)
return <h1>done</h1>

if (!redirected) {
redirected = true
return <Redirect to={{pathname: '/one', search: '?utm=1'}}/>
}

done = true

return <Redirect to={{pathname: '/one', search: '?utm=2'}}/>
}}/>
</Switch>
</MemoryRouter>
), node)

expect(node.innerHTML).toContain('done')
expect(console.error.calls.length).toBe(0)
expect.restoreSpies()
})

it('handles comments', () => {
const node = document.createElement('div')

Expand Down

0 comments on commit 2d6e1f6

Please sign in to comment.