From b8aa33352ce5f0fc9f6f88795409f0b8c73e99a5 Mon Sep 17 00:00:00 2001
From: Bailey Swartz <53988640+sever1an@users.noreply.github.com>
Date: Thu, 24 Oct 2019 15:36:13 -0700
Subject: [PATCH] fix(gatsby): Fix matchpath ordering (#18478)

* Add a test that fails when a * matchpath shadows an index page.

* Trailing * is removed during matchPath page weight scoring, this places glob paths at an equal level to index paths. Test for glob path prescedence now uses `toEqual` for deep object comparison.

* Place all static paths before matchPaths using wildcard sort field before score.

* Test case is more difficult with pages nested more deeply in matchPaths.
---
 .../__snapshots__/requires-writer.js.snap     | 54 +++++++++++++
 .../bootstrap/__tests__/requires-writer.js    | 75 +++++++++++++++++++
 .../gatsby/src/bootstrap/requires-writer.js   | 12 ++-
 3 files changed, 140 insertions(+), 1 deletion(-)

diff --git a/packages/gatsby/src/bootstrap/__tests__/__snapshots__/requires-writer.js.snap b/packages/gatsby/src/bootstrap/__tests__/__snapshots__/requires-writer.js.snap
index 3a73ee00ef44a..81721a350f1e1 100644
--- a/packages/gatsby/src/bootstrap/__tests__/__snapshots__/requires-writer.js.snap
+++ b/packages/gatsby/src/bootstrap/__tests__/__snapshots__/requires-writer.js.snap
@@ -1,5 +1,46 @@
 // Jest Snapshot v1, https://goo.gl/fbAQLP
 
+exports[`requires-writer matchPath have static pages first and prefer more specific matchPaths 1`] = `
+Array [
+  Object {
+    "matchPath": "/mp1/mp2/hello",
+    "path": "/mp1/mp2/hello",
+  },
+  Object {
+    "matchPath": "/mp1/mp2",
+    "path": "/mp1/mp2",
+  },
+  Object {
+    "matchPath": "/some-page",
+    "path": "/some-page",
+  },
+  Object {
+    "matchPath": "/",
+    "path": "/",
+  },
+  Object {
+    "matchPath": "/mp1/mp2/mp3/mp4/*",
+    "path": "/mp4",
+  },
+  Object {
+    "matchPath": "/mp1/mp2/mp3/*",
+    "path": "/mp3",
+  },
+  Object {
+    "matchPath": "/mp1/mp2/*",
+    "path": "/mp2",
+  },
+  Object {
+    "matchPath": "/mp1/*",
+    "path": "/mp1",
+  },
+  Object {
+    "matchPath": "/*",
+    "path": "/custom-404",
+  },
+]
+`;
+
 exports[`requires-writer matchPath should be sorted by specificity 1`] = `
 Array [
   Object {
@@ -21,6 +62,19 @@ Array [
 ]
 `;
 
+exports[`requires-writer matchPath should have index pages with higher priority than matchPaths 1`] = `
+Array [
+  Object {
+    "matchPath": "/",
+    "path": "/",
+  },
+  Object {
+    "matchPath": "/*",
+    "path": "/custom-404",
+  },
+]
+`;
+
 exports[`requires-writer matchPath should have static pages that live inside a matchPath 1`] = `
 Array [
   Object {
diff --git a/packages/gatsby/src/bootstrap/__tests__/requires-writer.js b/packages/gatsby/src/bootstrap/__tests__/requires-writer.js
index 53bc4758fe27e..a1059cc10c97f 100644
--- a/packages/gatsby/src/bootstrap/__tests__/requires-writer.js
+++ b/packages/gatsby/src/bootstrap/__tests__/requires-writer.js
@@ -145,5 +145,80 @@ describe(`requires-writer`, () => {
       expect(matchPaths[0].path).toBe(pages.get(`/app/clients/static`).path)
       expect(matchPaths).toMatchSnapshot()
     })
+
+    it(`should have index pages with higher priority than matchPaths`, async () => {
+      const pages = generatePagesState([
+        {
+          path: `/`,
+        },
+        {
+          path: `/custom-404`,
+          matchPath: `/*`,
+        },
+      ])
+
+      await requiresWriter.writeAll({
+        pages,
+        program,
+      })
+
+      expect(matchPaths[0].path).toBe(pages.get(`/`).path)
+      expect(matchPaths).toMatchSnapshot()
+    })
+
+    it(`have static pages first and prefer more specific matchPaths`, async () => {
+      const pages = generatePagesState([
+        {
+          path: `/`,
+        },
+        {
+          path: `/custom-404`,
+          matchPath: `/*`,
+        },
+        {
+          path: `/mp4`,
+          matchPath: `/mp1/mp2/mp3/mp4/*`,
+        },
+        {
+          path: `/some-page`,
+        },
+        {
+          path: `/mp1/mp2`,
+        },
+        {
+          path: `/mp1/mp2/hello`,
+        },
+        {
+          path: `/mp1`,
+          matchPath: `/mp1/*`,
+        },
+        {
+          path: `/mp2`,
+          matchPath: `/mp1/mp2/*`,
+        },
+        {
+          path: `/mp3`,
+          matchPath: `/mp1/mp2/mp3/*`,
+        },
+      ])
+
+      await requiresWriter.writeAll({
+        pages,
+        program,
+      })
+
+      expect(matchPaths.map(p => p.path)).toEqual([
+        `/mp1/mp2/hello`,
+        `/mp1/mp2`,
+        `/some-page`,
+        `/`,
+        `/mp4`,
+        `/mp3`,
+        `/mp2`,
+        `/mp1`,
+        `/custom-404`,
+      ])
+      expect(matchPaths).toMatchSnapshot()
+    })
   })
 })
diff --git a/packages/gatsby/src/bootstrap/requires-writer.js b/packages/gatsby/src/bootstrap/requires-writer.js
index 9484d47655176..997fee990707a 100644
--- a/packages/gatsby/src/bootstrap/requires-writer.js
+++ b/packages/gatsby/src/bootstrap/requires-writer.js
@@ -27,8 +27,11 @@ const getComponents = pages =>
  */
 const getMatchPaths = pages => {
   const createMatchPathEntry = (page, index) => {
-    let score = page.matchPath.replace(/\/$/, ``).split(`/`).length
+    let score = page.matchPath.replace(/[/][*]?$/, ``).split(`/`).length
+    let wildcard = 0
+
     if (!page.matchPath.includes(`*`)) {
+      wildcard = 1
       score += 1
     }
 
@@ -36,6 +39,7 @@ const getMatchPaths = pages => {
       ...page,
       index,
       score,
+      wildcard,
     }
   }
 
@@ -74,6 +78,12 @@ const getMatchPaths = pages => {
 
   return matchPathPages
     .sort((a, b) => {
+      // Paths with wildcards should appear after those without.
+      const wildcardOrder = b.wildcard - a.wildcard
+      if (wildcardOrder !== 0) {
+        return wildcardOrder
+      }
+
       // The higher the score, the higher the specificity of our matchPath
       const order = b.score - a.score
       if (order !== 0) {