1
0
Fork 0
mirror of https://github.com/terribleplan/next.js.git synced 2024-01-19 02:48:18 +00:00

should not change method to replaceState unless asPath is the same (#6033)

original code in `/lib/router/router.js`
```
  urlIsNew (pathname, query) {
    return this.pathname !== pathname || !shallowEquals(query, this.query)
  }
```
the urlIsNew compare `this.pathname` to an argument `pathname`
the invokers:
```
    // If asked to change the current URL we should reload the current page
    // (not location.reload() but reload getInitialProps and other Next.js stuffs)
    // We also need to set the method = replaceState always
    // as this should not go into the history (That's how browsers work)
    if (!this.urlIsNew(asPathname, asQuery)) {
      method = 'replaceState'
    }
```
the parameter here is `asPathname` destructured from `asPath`

so here is a problem when we reuse a single page rendered in two asPaths

pages/a.js
```
<>
  <Link href='/a'><a>goto a</a></Link>
  <Link href='/a' as='/b'><a>goto b</a></Link>
</>
```
If we navigate to page /a, then click 'goto b', actually the history is replaced, not pushed.
It is expected that history could be correctly pushed and popped as long as the browser url is changed.
This commit is contained in:
tangye 2019-01-12 01:29:59 +08:00 committed by Connor Davis
parent 0607638e8f
commit ad5431b4ae
No known key found for this signature in database
GPG key ID: 9E3CD9875570DB83
3 changed files with 21 additions and 5 deletions

View file

@ -188,14 +188,14 @@ export default class Router {
return true
}
const { pathname: asPathname, query: asQuery } = parse(as, true)
const { pathname, query } = parse(url, true)
// If asked to change the current URL we should reload the current page
// (not location.reload() but reload getInitialProps and other Next.js stuffs)
// We also need to set the method = replaceState always
// as this should not go into the history (That's how browsers work)
if (!this.urlIsNew(asPathname, asQuery)) {
// We should compare the new asPath to the current asPath, not the url
if (!this.urlIsNew(as)) {
method = 'replaceState'
}
@ -366,8 +366,10 @@ export default class Router {
}
}
urlIsNew (pathname, query) {
return this.pathname !== pathname || !shallowEquals(query, this.query)
urlIsNew (asPath) {
const { pathname, query } = parse(asPath, true)
const { pathname: curPathname } = parse(this.asPath, true)
return curPathname !== pathname || !shallowEquals(query, this.query)
}
isShallowRoutingPossible (route) {

View file

@ -9,7 +9,16 @@ export default withRouter(({router: {asPath, query}}) => {
<a id='hello'>hello</a>
</Link>
</div>
<div>
<Link href='/nav/as-path-pushstate' as='/something/else'>
<a id='else'>else</a>
</Link>
</div>
<div>
<Link href='/nav/as-path-pushstate' as='/nav/as-path-pushstate'>
<a id='hello2'>normal hello</a>
</Link>
</div>
{query.something === 'hello' && <Link href='/nav/as-path-pushstate?something=hello' as='/something/same-query'>
<a id='same-query'>same query</a>
</Link>}

View file

@ -648,6 +648,11 @@ export default (context) => {
await browser.back().waitForElementByCss('#something-hello')
const queryThree = JSON.parse(await browser.elementByCss('#router-query').text())
expect(queryThree.something).toBe('hello')
await browser.elementByCss('#else').click().waitForElementByCss('#something-else')
await browser.elementByCss('#hello2').click().waitForElementByCss('#nav-as-path-pushstate')
await browser.back().waitForElementByCss('#something-else')
const queryFour = JSON.parse(await browser.elementByCss('#router-query').text())
expect(queryFour.something).toBe(undefined)
} finally {
if (browser) {
browser.close()