Skip to content

Commit

Permalink
Make sure props.url is immutable (#4352)
Browse files Browse the repository at this point in the history
* Make sure props.url is immutable

* Add test for immutable url

* Match object instead of string
  • Loading branch information
timneutkens authored May 12, 2018
1 parent 2c3f406 commit 449f38d
Show file tree
Hide file tree
Showing 4 changed files with 57 additions and 3 deletions.
8 changes: 5 additions & 3 deletions lib/app.js
Original file line number Diff line number Diff line change
Expand Up @@ -93,18 +93,20 @@ const warnUrl = execOnce(() => {
})

export function createUrl (router) {
// This is to make sure we don't references the router object at call time
const {pathname, asPath, query} = router
return {
get query () {
warnUrl()
return router.query
return query
},
get pathname () {
warnUrl()
return router.pathname
return pathname
},
get asPath () {
warnUrl()
return router.asPath
return asPath
},
back: () => {
warnUrl()
Expand Down
37 changes: 37 additions & 0 deletions test/integration/basic/pages/nav/url-prop-change.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
import React from 'react'
import Link from 'next/link'

export default class UrlPropChange extends React.Component {
constructor (props) {
super(props)
this.state = {
previousUrl: {},
url: props.url
}
}

componentWillReceiveProps (nextProps) {
this.setState(() => {
return {
previousUrl: this.props.url,
url: nextProps.url
}
})
}

render () {
const {previousUrl, url} = this.state
return <div>
Current:
<div id='url-result'>
{JSON.stringify(url)}
</div>
<br /><br />
Previous:
<div id='previous-url-result'>
{JSON.stringify(previousUrl)}
</div>
<Link href='/nav/url-prop-change?added=yes'><a id='add-query'>Add querystring</a></Link>
</div>
}
}
14 changes: 14 additions & 0 deletions test/integration/basic/test/client-navigation.js
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,20 @@ export default (context, render) => {
})
})

describe('With url property', () => {
it('Should keep immutable pathname, asPath and query', async () => {
const browser = await webdriver(context.appPort, '/nav/url-prop-change')
await browser.elementByCss('#add-query').click()
const urlResult = await browser.elementByCss('#url-result').text()
const previousUrlResult = await browser.elementByCss('#previous-url-result').text()

expect(JSON.parse(urlResult)).toMatchObject({'query': {'added': 'yes'}, 'pathname': '/nav/url-prop-change', 'asPath': '/nav/url-prop-change?added=yes'})
expect(JSON.parse(previousUrlResult)).toMatchObject({'query': {}, 'pathname': '/nav/url-prop-change', 'asPath': '/nav/url-prop-change'})

browser.close()
})
})

describe('with <a/> tag inside the <Link />', () => {
it('should navigate the page', async () => {
const browser = await webdriver(context.appPort, '/nav/about')
Expand Down
1 change: 1 addition & 0 deletions test/integration/basic/test/index.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ describe('Basic Features', () => {
renderViaHTTP(context.appPort, '/nav/redirect'),
renderViaHTTP(context.appPort, '/nav/as-path'),
renderViaHTTP(context.appPort, '/nav/as-path-using-router'),
renderViaHTTP(context.appPort, '/nav/url-prop-change'),

renderViaHTTP(context.appPort, '/nested-cdm/index'),

Expand Down

0 comments on commit 449f38d

Please sign in to comment.