Fix re-rendering on client after navigating back from external (#6652)

* Fix re-rendering on client after navigating
back from external site

* Clean up tagging before unload

* Add check for history.state 1/2
Co-Authored-By: Nijjk <22380829+ijjk@users.noreply.github.com>

* Add check for history.state 2/2
Co-Authored-By: Nijjk <22380829+ijjk@users.noreply.github.com>

* Add check for options
Co-Authored-By: Nijjk <22380829+ijjk@users.noreply.github.com>

* Add test for navigating to external site and back
also added testing in safari and firefox

* Add test for query in url

.circleci/config.yml

@@ -44,6 +44,32 @@ jobs:
           environment:
             BROWSERSTACK: 'true'
             BROWSER_NAME: 'ie'
+  test-safari:
+    docker:
+      - image: circleci/node:8-browsers
+    working_directory: ~/repo
+    steps:
+      - attach_workspace:
+          at: .
+      - run:
+          name: Test in Safari
+          command: 'if [[ ! -z $BROWSERSTACK_USERNAME ]]; then yarn testall test/integration/production/; else echo "Not running for PR"; fi'
+          environment:
+            BROWSERSTACK: 'true'
+            BROWSER_NAME: 'safari'
+  test-firefox:
+    docker:
+      - image: circleci/node:8-browsers
+    working_directory: ~/repo
+    steps:
+      - attach_workspace:
+          at: .
+      - run:
+          name: Test in Firefox
+          command: 'if [[ ! -z $BROWSERSTACK_USERNAME ]]; then yarn testall test/integration/production/; else echo "Not running for PR"; fi'
+          environment:
+            BROWSERSTACK: 'true'
+            BROWSER_NAME: 'firefox'
   deploy:
     docker:
       - image: circleci/node:8-browsers
@@ -76,6 +102,27 @@ workflows:
               only:
                 - master
                 - canary
+      - test-safari:
+          requires:
+            - build
+            - test
+            - test-ie11
+          filters:
+            branches:
+              only:
+                - master
+                - canary
+      - test-firefox:
+          requires:
+            - build
+            - test
+            - test-ie11
+            - test-safari
+          filters:
+            branches:
+              only:
+                - master
+                - canary
       - deploy:
           requires:
             - test

packages/next-server/lib/router/router.ts

@@ -75,6 +75,14 @@ export default class Router implements IRouterInterface {
       this.changeState('replaceState', formatWithValidation({ pathname, query }), as)
 
       window.addEventListener('popstate', this.onPopState)
+      window.addEventListener('unload', () => {
+        // Workaround for popstate firing on initial page load when
+        // navigating back from an external site
+        if (history.state) {
+          const { url, as, options }: any = history.state
+          this.changeState('replaceState', url, as, { ...options, fromExternal: true })
+        }
+      })
     }
   }
 
@@ -106,6 +114,12 @@ export default class Router implements IRouterInterface {
       return
     }
 
+    // Make sure we don't re-render on initial load,
+    // can be caused by navigating back from an external site
+    if (e.state.options && e.state.options.fromExternal) {
+      return
+    }
+
     // If the downstream application returns falsy, return.
     // They will then be responsible for handling the event.
     if (this._bps && !this._bps(e.state)) {

test/integration/production/pages/external-and-back.js

+const Page = ({ from }) => (
+  <div>
+    <p>{from}</p>
+    <a href='https://google.com'>External link</a>
+  </div>
+)
+
+Page.getInitialProps = () => {
+  return { from: typeof window === 'undefined' ? 'server' : 'client' }
+}
+
+export default Page

test/integration/production/pages/query.js

+import { withRouter } from 'next/router'
+
+const Page = ({ router: { query } }) => (
+  <>
+    <p id={'q' + query.id}>{query.id}</p>
+    <a id='first' href='/query?id=1'>Go to ?id=1</a>
+    <a id='second' href='/query?id=2'>Go to ?id=2</a>
+  </>
+)
+
+export default withRouter(Page)

test/integration/production/test/index.test.js

@@ -164,6 +164,45 @@ describe('Production Usage', () => {
     })
   })
 
+  it('should navigate to external site and back', async () => {
+    const browser = await webdriver(appPort, '/external-and-back')
+    const initialText = await browser.elementByCss('p').text()
+    expect(initialText).toBe('server')
+
+    await browser
+      .elementByCss('a')
+      .click()
+      .waitForElementByCss('input')
+      .back()
+      .waitForElementByCss('p')
+
+    await waitFor(1000)
+    const newText = await browser.elementByCss('p').text()
+    expect(newText).toBe('server')
+  })
+
+  it('should change query correctly', async () => {
+    const browser = await webdriver(appPort, '/query?id=0')
+    let id = await browser.elementByCss('#q0').text()
+    expect(id).toBe('0')
+
+    await browser
+      .elementByCss('#first')
+      .click()
+      .waitForElementByCss('#q1')
+
+    id = await browser.elementByCss('#q1').text()
+    expect(id).toBe('1')
+
+    await browser
+      .elementByCss('#second')
+      .click()
+      .waitForElementByCss('#q2')
+
+    id = await browser.elementByCss('#q2').text()
+    expect(id).toBe('2')
+  })
+
   describe('Runtime errors', () => {
     it('should render a server side error on the client side', async () => {
       const browser = await webdriver(appPort, '/error-in-ssr-render')