Skip to content

Commit

Permalink
Fix <Link to="string">
Browse files Browse the repository at this point in the history
New implementation will parse the string to create an actual location object. This also means that a to string with no pathname will resolve using the current location.

The "href" value is also now stored in state, only recalculating it when the "to" prop changes.
  • Loading branch information
pshrmn committed Sep 1, 2017
1 parent 7585419 commit 86ef389
Show file tree
Hide file tree
Showing 2 changed files with 68 additions and 10 deletions.
36 changes: 26 additions & 10 deletions packages/react-router-dom/modules/Link.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 invariant from 'invariant'
import { createLocation } from 'history'

const isModifiedEvent = (event) =>
!!(event.metaKey || event.altKey || event.ctrlKey || event.shiftKey)
Expand Down Expand Up @@ -37,6 +38,30 @@ class Link extends React.Component {
}).isRequired
}

state = {}

componentWillMount() {
this.setHref(this.props.to)
}

componentWillReceiveProps(nextProps) {
if (this.props.to !== nextProps.to) {
this.setHref(nextProps.to)
}
}

setHref(to) {
invariant(
this.context.router,
'You should not use <Link> outside a <Router>'
)
const { history } = this.context.router
const location = typeof to === 'string'
? createLocation(to, null, null, history.location)
: to
this.setState({ href: history.createHref(location) })
}

handleClick = (event) => {
if (this.props.onClick)
this.props.onClick(event)
Expand All @@ -62,16 +87,7 @@ 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 <Router>'
)

const href = this.context.router.history.createHref(
typeof to === 'string' ? { pathname: to } : to
)

const { href } = this.state
return <a {...props} onClick={this.handleClick} href={href} ref={innerRef}/>
}
}
Expand Down
42 changes: 42 additions & 0 deletions packages/react-router-dom/modules/__tests__/Link-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,47 @@ describe('A <Link>', () => {
expect(href).toEqual('/the/path?the=query#the-hash')
})

it('re-renders with correct href when to prop changes', () => {
const location = { pathname: '/the/path' }
const newLocation = { pathname: '/another/path' }

const node = document.createElement('div')

ReactDOM.render((
<MemoryRouter>
<Link to={location}>link</Link>
</MemoryRouter>
), node)

const anchor = node.querySelector('a')
let href = anchor.getAttribute('href')
expect(href).toEqual('/the/path')

ReactDOM.render((
<MemoryRouter>
<Link to={newLocation}>link</Link>
</MemoryRouter>
), node)
href = anchor.getAttribute('href')
expect(href).toEqual('/another/path')
})

describe('to as a string', () => {
it('resolves to with no pathname using current location', () => {
const node = document.createElement('div')

ReactDOM.render((
<MemoryRouter initialEntries={[ '/somewhere' ]}>
<Link to='?rendersWithPathname=true'>link</Link>
</MemoryRouter>
), node)

const href = node.querySelector('a').getAttribute('href')

expect(href).toEqual('/somewhere?rendersWithPathname=true')
})
})

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

Expand Down Expand Up @@ -73,6 +114,7 @@ describe('A <Link> underneath a <HashRouter>', () => {

afterEach(() => {
ReactDOM.unmountComponentAtNode(node)
window.history.replaceState(null, '', '#')
})

const createLinkNode = (hashType, to) => {
Expand Down

0 comments on commit 86ef389

Please sign in to comment.