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.
gatsbyjs · Oct 24, 2019 · b8aa333 · b8aa333
1 parent f5d3317
commit b8aa333
Show file tree

Hide file tree

Showing 3 changed files with 140 additions and 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
@@ -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
@@ -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
@@ -27,15 +27,19 @@ 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
     }
 
     return {
       ...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) {