From f52bbacd13b1d91df15e7932bdb0425a320d1809 Mon Sep 17 00:00:00 2001
From: Anthony Marcar <Anthony.Marcar@gmail.com>
Date: Tue, 25 Jun 2019 00:20:14 +1000
Subject: [PATCH] fix(gatsby): Fix client side routing match paths regression
 (#15010)

* export loader.findMatchPath

* reorganize production-app explicit navigate if statement

* strip BASE_PATH prefix before finding matchPath

* Add end to end tests

* Remove screenshots

* Update tests

* Add .gitignore

* Move tests to development runtime

* Hard code path in tests

* Remove serve e2e tests

* Add tests in production runtime
---
 .../integration/functionality/paths.js        | 11 +++++++++
 e2e-tests/development-runtime/gatsby-node.js  | 10 ++++++++
 .../development-runtime/src/pages/paths.js    | 15 ++++++++++++
 .../cypress/integration/paths.js              | 11 +++++++++
 e2e-tests/production-runtime/gatsby-node.js   |  4 ++++
 .../production-runtime/src/pages/paths.js     | 15 ++++++++++++
 packages/gatsby/cache-dir/loader.js           |  6 +++--
 packages/gatsby/cache-dir/production-app.js   | 23 ++++++++++++-------
 8 files changed, 85 insertions(+), 10 deletions(-)
 create mode 100644 e2e-tests/development-runtime/cypress/integration/functionality/paths.js
 create mode 100644 e2e-tests/development-runtime/src/pages/paths.js
 create mode 100644 e2e-tests/production-runtime/cypress/integration/paths.js
 create mode 100644 e2e-tests/production-runtime/src/pages/paths.js

diff --git a/e2e-tests/development-runtime/cypress/integration/functionality/paths.js b/e2e-tests/development-runtime/cypress/integration/functionality/paths.js
new file mode 100644
index 0000000000000..d480e1c5a8805
--- /dev/null
+++ b/e2e-tests/development-runtime/cypress/integration/functionality/paths.js
@@ -0,0 +1,11 @@
+describe(`develop`, () => {
+  it(`works with the client side path`, () => {
+    cy.visit(`/paths/app`).waitForRouteChange()
+    cy.get(`h1`).contains(`App Page`)
+  })
+
+  it(`works with a child of the client side path`, () => {
+    cy.visit(`/paths/app/12345`).waitForRouteChange()
+    cy.get(`h1`).contains(`Not Found in App`)
+  })
+})
diff --git a/e2e-tests/development-runtime/gatsby-node.js b/e2e-tests/development-runtime/gatsby-node.js
index ba52e858e9783..6883584018835 100644
--- a/e2e-tests/development-runtime/gatsby-node.js
+++ b/e2e-tests/development-runtime/gatsby-node.js
@@ -78,3 +78,13 @@ exports.createPages = async function createPages({
     })
   })
 }
+
+exports.onCreatePage = async ({ page, actions }) => {
+  const { createPage } = actions
+
+  if (page.path.match(/^\/paths/)) {
+    page.matchPath = `/paths/*`
+  }
+
+  createPage(page)
+}
diff --git a/e2e-tests/development-runtime/src/pages/paths.js b/e2e-tests/development-runtime/src/pages/paths.js
new file mode 100644
index 0000000000000..5b55a47068a57
--- /dev/null
+++ b/e2e-tests/development-runtime/src/pages/paths.js
@@ -0,0 +1,15 @@
+import { Router } from "@reach/router"
+import React from "react"
+
+const App = () => <h1>App Page</h1>
+
+const NotFound = () => <h1>Not Found in App</h1>
+
+const AppPage = () => (
+  <Router>
+    <App path="/paths/app" />
+    <NotFound default />
+  </Router>
+)
+
+export default AppPage
diff --git a/e2e-tests/production-runtime/cypress/integration/paths.js b/e2e-tests/production-runtime/cypress/integration/paths.js
new file mode 100644
index 0000000000000..96c021869b0df
--- /dev/null
+++ b/e2e-tests/production-runtime/cypress/integration/paths.js
@@ -0,0 +1,11 @@
+describe(`serve`, () => {
+  it(`works with the client side path`, () => {
+    cy.visit(`/paths/app`).waitForRouteChange()
+    cy.get(`h1`).contains(`App Page`)
+  })
+
+  it(`works with a child of the client side path`, () => {
+    cy.visit(`/paths/app/12345`).waitForRouteChange()
+    cy.get(`h1`).contains(`Not Found in App`)
+  })
+})
diff --git a/e2e-tests/production-runtime/gatsby-node.js b/e2e-tests/production-runtime/gatsby-node.js
index 465407b7f593f..24c6f49585e4f 100644
--- a/e2e-tests/production-runtime/gatsby-node.js
+++ b/e2e-tests/production-runtime/gatsby-node.js
@@ -27,4 +27,8 @@ exports.onCreatePage = ({ page, actions }) => {
       })
       break
   }
+  if (page.path.match(/^\/paths/)) {
+    page.matchPath = `/paths/*`
+    actions.createPage(page)
+  }
 }
diff --git a/e2e-tests/production-runtime/src/pages/paths.js b/e2e-tests/production-runtime/src/pages/paths.js
new file mode 100644
index 0000000000000..88c722215ae70
--- /dev/null
+++ b/e2e-tests/production-runtime/src/pages/paths.js
@@ -0,0 +1,15 @@
+import { Router } from '@reach/router'
+import React from 'react'
+
+const App = () => <h1>App Page</h1>
+
+const NotFound = () => <h1>Not Found in App</h1>
+
+const AppPage = () => (
+  <Router>
+    <App path="/paths/app" />
+    <NotFound default />
+  </Router>
+)
+
+export default AppPage
diff --git a/packages/gatsby/cache-dir/loader.js b/packages/gatsby/cache-dir/loader.js
index 21cc1888b0ded..c5d76162816c0 100644
--- a/packages/gatsby/cache-dir/loader.js
+++ b/packages/gatsby/cache-dir/loader.js
@@ -26,7 +26,7 @@ if (process.env.NODE_ENV !== `production`) {
 // Cache for `cleanAndFindPath()`. In case `match-paths.json` is large
 const cleanAndFindPathCache = {}
 
-const findMatchPath = (matchPaths, trimmedPathname) => {
+const findMatchPath = trimmedPathname => {
   for (const { matchPath, path } of matchPaths) {
     if (match(matchPath, trimmedPathname)) {
       return path
@@ -66,7 +66,7 @@ const cleanAndFindPath = rawPathname => {
     return cleanAndFindPathCache[trimmedPathname]
   }
 
-  let foundPath = findMatchPath(matchPaths, trimmedPathname)
+  let foundPath = findMatchPath(trimmedPathname)
   if (!foundPath) {
     if (trimmedPathname === `/index.html`) {
       foundPath = `/`
@@ -371,6 +371,8 @@ const queue = {
 
   doesPageHtmlExistSync: rawPath =>
     pageHtmlExistsResults[cleanAndFindPath(rawPath)],
+
+  findMatchPath,
 }
 
 export const postInitialRenderWork = () => {
diff --git a/packages/gatsby/cache-dir/production-app.js b/packages/gatsby/cache-dir/production-app.js
index 13fdc28915d5b..17d4166b3cd8e 100644
--- a/packages/gatsby/cache-dir/production-app.js
+++ b/packages/gatsby/cache-dir/production-app.js
@@ -15,6 +15,7 @@ import asyncRequires from "./async-requires"
 import matchPaths from "./match-paths.json"
 import loader, { setApiRunnerForLoader } from "./loader"
 import EnsureResources from "./ensure-resources"
+import stripPrefix from "./strip-prefix"
 
 window.asyncRequires = asyncRequires
 window.___emitter = emitter
@@ -62,17 +63,23 @@ apiRunnerAsync(`onClientEntry`).then(() => {
   }
 
   const { pagePath, location: browserLoc } = window
+
+  // Explicitly call navigate if the canonical path (window.pagePath)
+  // is different to the browser path (window.location.pathname). But
+  // only if NONE of the following conditions hold:
+  //
+  // - The url matches a client side route (page.matchPath)
+  // - it's a 404 page
+  // - it's the offline plugin shell (/offline-plugin-app-shell-fallback/)
   if (
-    // Make sure the window.page object is defined
     pagePath &&
-    // The canonical path doesn't match the actual path (i.e. the address bar)
     __BASE_PATH__ + pagePath !== browserLoc.pathname &&
-    // Ignore 404 pages, since we want to keep the same URL
-    pagePath !== `/404.html` &&
-    !pagePath.match(/^\/404\/?$/) &&
-    // Also ignore the offline shell (since when using the offline plugin, all
-    // pages have this canonical path)
-    !pagePath.match(/^\/offline-plugin-app-shell-fallback\/?$/)
+    !(
+      loader.findMatchPath(stripPrefix(browserLoc.pathname, __BASE_PATH__)) ||
+      pagePath === `/404.html` ||
+      pagePath.match(/^\/404\/?$/) ||
+      pagePath.match(/^\/offline-plugin-app-shell-fallback\/?$/)
+    )
   ) {
     navigate(__BASE_PATH__ + pagePath + browserLoc.search + browserLoc.hash, {
       replace: true,