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

Make sure props.url is immutable #4352

Merged
merged 3 commits into from
May 12, 2018
Merged
Show file tree
Hide file tree
Changes from 2 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
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(urlResult).toBe(`{"query":{"added":"yes"},"pathname":"/nav/url-prop-change","asPath":"/nav/url-prop-change?added=yes"}`)
expect(previousUrlResult).toBe(`{"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