fix(gatsby): don't remount matchPath pages (#10261)

* fix: don't remount page component when navigating inside matchPath

* test: add tests to ensure remounting behaviour stays consistent and avoid future regressions

* switch from snapshot testing to more descriptive assert

* abstract lifecycleCallCount, so tests are more readable

.eslintrc.json

@@ -59,7 +59,7 @@
       }
     },
     {
-      "files": ["**/cypress/integration/**/*"],
+      "files": ["**/cypress/integration/**/*", "**/cypress/support/**/*"],
       "globals": {
         "cy": false,
         "Cypress": false

e2e-tests/production-runtime/cypress/integration/lifecycle-methods.js

@@ -0,0 +1,41 @@
+describe(`Production build tests`, () => {
+  it(`should remount when navigating to different template`, () => {
+    cy.visit(`/`).waitForAPI(`onRouteUpdate`)
+
+    cy.getTestElement(`page2`)
+      .click()
+      .waitForAPI(`onRouteUpdate`)
+
+    // we expect 2 `componentDidMount` calls - 1 for initial page and 1 for second page
+    cy.lifecycleCallCount(`componentDidMount`).should(`equal`, 2)
+    cy.lifecycleCallCount(`render`).should(`equal`, 2)
+  })
+
+  it(`should remount when navigating to different page using same template`, () => {
+    cy.visit(`/`).waitForAPI(`onRouteUpdate`)
+
+    cy.getTestElement(`duplicated`)
+      .click()
+      .waitForAPI(`onRouteUpdate`)
+
+    // we expect 2 `componentDidMount` calls - 1 for initial page and 1 for duplicated page
+    cy.lifecycleCallCount(`componentDidMount`).should(`equal`, 2)
+    cy.lifecycleCallCount(`render`).should(`equal`, 2)
+  })
+
+  it(`should NOT remount when navigating within client only paths`, () => {
+    cy.visit(`/client-only-paths`).waitForAPI(`onRouteUpdate`)
+
+    cy.getTestElement(`/profile`)
+      .click()
+      .waitForAPI(`onRouteUpdate`)
+
+    cy.getTestElement(`/dashboard`)
+      .click()
+      .waitForAPI(`onRouteUpdate`)
+
+    // we expect just 1 `componentDidMount` call, when navigating inside matchPath
+    cy.lifecycleCallCount(`componentDidMount`).should(`equal`, 1)
+    cy.lifecycleCallCount(`render`).should(`equal`, 3)
+  })
+})

e2e-tests/production-runtime/cypress/support/commands.js

@@ -0,0 +1,10 @@
+Cypress.Commands.add(`lifecycleCallCount`, action =>
+  cy
+    .window()
+    .then(
+      win =>
+        win.___PageComponentLifecycleCallsLog.filter(
+          entry => entry.action === action
+        ).length
+    )
+)

e2e-tests/production-runtime/cypress/support/index.js

@@ -16,6 +16,7 @@
 // Import commands.js using ES2015 syntax:
 // TODO: import as "cypress-gatsby" once this is published to NPM
 import '../../../../packages/cypress-gatsby'
+import './commands'
 
 // Alternatively you can use CommonJS syntax:
 // require('./commands')

e2e-tests/production-runtime/gatsby-browser.js

@@ -1,7 +1,20 @@
-/**
- * Implement Gatsby's Browser APIs in this file.
- *
- * See: https://www.gatsbyjs.org/docs/browser-apis/
- */
+if (typeof window !== `undefined`) {
+  window.___PageComponentLifecycleCallsLog = []
+}
 
-// You can delete this file if you're not using it
+const addLogEntry = (action, location) => {
+  const idElement = document.querySelector(`[data-testid="dom-marker"]`)
+  window.___PageComponentLifecycleCallsLog.push({
+    action,
+    pathname: location.pathname,
+    domContent: idElement ? idElement.innerText : null,
+  })
+}
+
+exports.onPreRouteUpdate = ({ location }) => {
+  addLogEntry(`onPreRouteUpdate`, location)
+}
+
+exports.onRouteUpdate = ({ location }) => {
+  addLogEntry(`onRouteUpdate`, location)
+}

e2e-tests/production-runtime/gatsby-node.js

@@ -1,7 +1,17 @@
-/**
- * Implement Gatsby's Node APIs in this file.
- *
- * See: https://www.gatsbyjs.org/docs/node-apis/
- */
-
-// You can delete this file if you're not using it
+exports.onCreatePage = ({ page, actions }) => {
+  if (page.path === `/client-only-paths/`) {
+    // create client-only-paths
+    page.matchPath = `/client-only-paths/*`
+    actions.createPage(page)
+  } else if (page.path === `/`) {
+    // use index page as template
+    // (mimics)
+    actions.createPage({
+      ...page,
+      path: `/duplicated`,
+      context: {
+        DOMMarker: `duplicated`,
+      },
+    })
+  }
+}

e2e-tests/production-runtime/src/pages/client-only-paths.js

@@ -0,0 +1,34 @@
+import React from 'react'
+import { Router } from '@reach/router'
+import { Link } from 'gatsby'
+
+import Layout from '../components/layout'
+import InstrumentPage from '../utils/instrument-page'
+
+const Page = props => (
+  <pre data-testid="dom-marker">[client-only-path] {props.page}</pre>
+)
+
+const routes = [`/`, `/profile`, `/dashboard`]
+
+const basePath = `/client-only-paths`
+
+const ClientOnlyPathPage = props => (
+  <Layout>
+    <Router location={props.location} basepath={basePath}>
+      <Page path="/" page="index" />
+      <Page path="/:page" />
+    </Router>
+    <ul>
+      {routes.map(route => (
+        <li key={route}>
+          <Link to={`${basePath}${route}`} data-testid={route}>
+            {route}
+          </Link>
+        </li>
+      ))}
+    </ul>
+  </Layout>
+)
+
+export default InstrumentPage(ClientOnlyPathPage)

e2e-tests/production-runtime/src/pages/index.js

@@ -2,22 +2,40 @@ import React from 'react'
 import { Link } from 'gatsby'
 
 import Layout from '../components/layout'
+import InstrumentPage from '../utils/instrument-page'
 
-const IndexPage = () => (
+const IndexPage = ({ pageContext }) => (
   <Layout>
     <h1>Hi people</h1>
-    <p>Welcome to your new Gatsby site.</p>
-    <p>Now go build something great.</p>
-    <Link to="/page-2/">
-      <span data-testid="page2">Go to page 2</span>
-    </Link>
-    <Link to="/page-3/" data-testid="404">
-      To non-existent page
-    </Link>
-    <Link to="/long-page/" data-testid="long-page">
-      To long page
-    </Link>
+    <pre data-testid="dom-marker">{pageContext.DOMMarker || `index`}</pre>
+    <ul>
+      <li>
+        <Link to="/page-2/">
+          <span data-testid="page2">Go to page 2</span>
+        </Link>
+      </li>
+      <li>
+        <Link to="/page-3/" data-testid="404">
+          To non-existent page
+        </Link>
+      </li>
+      <li>
+        <Link to="/long-page/" data-testid="long-page">
+          To long page
+        </Link>
+      </li>
+      <li>
+        <Link to="/duplicated/" data-testid="duplicated">
+          Another page using Index template
+        </Link>
+      </li>
+      <li>
+        <Link to="/client-only-paths/" data-testid="client-only-paths">
+          Client only paths
+        </Link>
+      </li>
+    </ul>
   </Layout>
 )
 
-export default IndexPage
+export default InstrumentPage(IndexPage)

e2e-tests/production-runtime/src/pages/page-2.js

@@ -2,13 +2,15 @@ import React from 'react'
 import { Link } from 'gatsby'
 
 import Layout from '../components/layout'
+import InstrumentPage from '../utils/instrument-page'
 
 const SecondPage = () => (
   <Layout>
     <h1>Hi from the second page</h1>
     <p>Welcome to page 2</p>
+    <pre data-testid="dom-marker">page-2</pre>
     <Link to="/">Go back to the homepage</Link>
   </Layout>
 )
 
-export default SecondPage
+export default InstrumentPage(SecondPage)

e2e-tests/production-runtime/src/utils/instrument-page.js

@@ -0,0 +1,33 @@
+import React from 'react'
+
+export default Page =>
+  class extends React.Component {
+    addLogEntry(action) {
+      if (typeof window !== `undefined`) {
+        window.___PageComponentLifecycleCallsLog.push({
+          action,
+          pageComponent: this.props.pageResources.page.componentChunkName,
+          locationPath: this.props.location.pathname,
+          pagePath: this.props.pageResources.page.path,
+        })
+      }
+    }
+
+    constructor(props) {
+      super(props)
+      this.addLogEntry(`constructor`)
+    }
+
+    componentDidMount() {
+      this.addLogEntry(`componentDidMount`)
+    }
+
+    componentWillUnmount() {
+      this.addLogEntry(`componentWillUnmount`)
+    }
+
+    render() {
+      this.addLogEntry(`render`)
+      return <Page {...this.props} />
+    }
+  }

packages/gatsby/cache-dir/page-renderer.js

@@ -20,7 +20,7 @@ class PageRenderer extends React.Component {
       replacementElement ||
       createElement(this.props.pageResources.component, {
         ...props,
-        key: this.props.location.pathname,
+        key: this.props.pageResources.page.path,
       })
 
     const wrappedPage = apiRunner(