From ad5431b4ae952d16e7a038c57e47055f6446cfcd Mon Sep 17 00:00:00 2001
From: tangye <447084+tangye1234@users.noreply.github.com>
Date: Sat, 12 Jan 2019 01:29:59 +0800
Subject: [PATCH] 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.
---
 packages/next-server/lib/router/router.js             | 10 ++++++----
 test/integration/basic/pages/nav/as-path-pushstate.js | 11 ++++++++++-
 test/integration/basic/test/client-navigation.js      |  5 +++++
 3 files changed, 21 insertions(+), 5 deletions(-)

diff --git a/packages/next-server/lib/router/router.js b/packages/next-server/lib/router/router.js
index d4f4c1d5..cca88738 100644
--- a/packages/next-server/lib/router/router.js
+++ b/packages/next-server/lib/router/router.js
@@ -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) {
diff --git a/test/integration/basic/pages/nav/as-path-pushstate.js b/test/integration/basic/pages/nav/as-path-pushstate.js
index a5c1482b..a85e0ac2 100644
--- a/test/integration/basic/pages/nav/as-path-pushstate.js
+++ b/test/integration/basic/pages/nav/as-path-pushstate.js
@@ -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>}
diff --git a/test/integration/basic/test/client-navigation.js b/test/integration/basic/test/client-navigation.js
index fbf5fdc4..f51411dc 100644
--- a/test/integration/basic/test/client-navigation.js
+++ b/test/integration/basic/test/client-navigation.js
@@ -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()