fix(gatsby-plugin-manifest): fixes icons not getting asset pre… (#20142)

Modifies any path provided for a manifest icon to be prefixed with appropriate path or asset prefix. Co-authored-by: GatsbyJS Bot <[email protected]> Co-authored-by: Ward Peeters <[email protected]>
gatsbyjs · Jan 27, 2020 · 0e8ed07 · 0e8ed07
1 parent 32c8ec3
commit 0e8ed07
Show file tree

Hide file tree

Showing 4 changed files with 123 additions and 12 deletions.
diff --git a/packages/gatsby-plugin-manifest/src/__tests__/__snapshots__/gatsby-node.js.snap b/packages/gatsby-plugin-manifest/src/__tests__/__snapshots__/gatsby-node.js.snap
@@ -2,6 +2,8 @@
 
 exports[`Test plugin manifest options correctly works with default parameters 1`] = `"{\\"name\\":\\"GatsbyJS\\",\\"short_name\\":\\"GatsbyJS\\",\\"start_url\\":\\"/\\",\\"background_color\\":\\"#f7f0eb\\",\\"theme_color\\":\\"#a2466c\\",\\"display\\":\\"standalone\\",\\"icons\\":[{\\"src\\":\\"icons/icon-48x48.png\\",\\"sizes\\":\\"48x48\\",\\"type\\":\\"image/png\\"},{\\"src\\":\\"icons/icon-72x72.png\\",\\"sizes\\":\\"72x72\\",\\"type\\":\\"image/png\\"},{\\"src\\":\\"icons/icon-96x96.png\\",\\"sizes\\":\\"96x96\\",\\"type\\":\\"image/png\\"},{\\"src\\":\\"icons/icon-144x144.png\\",\\"sizes\\":\\"144x144\\",\\"type\\":\\"image/png\\"},{\\"src\\":\\"icons/icon-192x192.png\\",\\"sizes\\":\\"192x192\\",\\"type\\":\\"image/png\\"},{\\"src\\":\\"icons/icon-256x256.png\\",\\"sizes\\":\\"256x256\\",\\"type\\":\\"image/png\\"},{\\"src\\":\\"icons/icon-384x384.png\\",\\"sizes\\":\\"384x384\\",\\"type\\":\\"image/png\\"},{\\"src\\":\\"icons/icon-512x512.png\\",\\"sizes\\":\\"512x512\\",\\"type\\":\\"image/png\\"}]}"`;
 
+exports[`Test plugin manifest options correctly works with pathPrefix 1`] = `"{\\"name\\":\\"GatsbyJS\\",\\"short_name\\":\\"GatsbyJS\\",\\"start_url\\":\\"/blog/\\",\\"background_color\\":\\"#f7f0eb\\",\\"theme_color\\":\\"#a2466c\\",\\"display\\":\\"standalone\\",\\"icons\\":[{\\"src\\":\\"/blog/icons/icon-48x48.png\\",\\"sizes\\":\\"48x48\\",\\"type\\":\\"image/png\\"},{\\"src\\":\\"/blog/icons/icon-72x72.png\\",\\"sizes\\":\\"72x72\\",\\"type\\":\\"image/png\\"},{\\"src\\":\\"/blog/icons/icon-96x96.png\\",\\"sizes\\":\\"96x96\\",\\"type\\":\\"image/png\\"},{\\"src\\":\\"/blog/icons/icon-144x144.png\\",\\"sizes\\":\\"144x144\\",\\"type\\":\\"image/png\\"},{\\"src\\":\\"/blog/icons/icon-192x192.png\\",\\"sizes\\":\\"192x192\\",\\"type\\":\\"image/png\\"},{\\"src\\":\\"/blog/icons/icon-256x256.png\\",\\"sizes\\":\\"256x256\\",\\"type\\":\\"image/png\\"},{\\"src\\":\\"/blog/icons/icon-384x384.png\\",\\"sizes\\":\\"384x384\\",\\"type\\":\\"image/png\\"},{\\"src\\":\\"/blog/icons/icon-512x512.png\\",\\"sizes\\":\\"512x512\\",\\"type\\":\\"image/png\\"}]}"`;
+
 exports[`Test plugin manifest options does file name based cache busting 1`] = `
 [MockFunction] {
   "calls": Array [

diff --git a/packages/gatsby-plugin-manifest/src/__tests__/gatsby-node.js b/packages/gatsby-plugin-manifest/src/__tests__/gatsby-node.js
@@ -39,7 +39,9 @@ jest.mock(`sharp`, () => {
 })
 
 jest.mock(`gatsby-core-utils`, () => {
+  const originalCoreUtils = jest.requireActual(`gatsby-core-utils`)
   return {
+    slash: originalCoreUtils.slash,
     cpuCoreCount: jest.fn(() => `1`),
     createContentDigest: jest.fn(() => `contentDigest`),
   }
@@ -278,9 +280,31 @@ describe(`Test plugin manifest options`, () => {
     expect(content.icons[1].purpose).toEqual(`maskable`)
   })
 
+  it(`correctly works with pathPrefix`, async () => {
+    await onPostBootstrap(
+      { ...apiArgs, basePath: `/blog` },
+      {
+        name: `GatsbyJS`,
+        short_name: `GatsbyJS`,
+        start_url: `/`,
+        background_color: `#f7f0eb`,
+        theme_color: `#a2466c`,
+        display: `standalone`,
+      }
+    )
+    const contents = fs.writeFileSync.mock.calls[0][1]
+    expect(fs.writeFileSync).toHaveBeenCalledWith(
+      path.join(`public`, `manifest.webmanifest`),
+      expect.anything()
+    )
+    expect(sharp).toHaveBeenCalledTimes(0)
+    expect(contents).toMatchSnapshot()
+  })
+
   it(`generates all language versions`, async () => {
     fs.statSync.mockReturnValueOnce({ isFile: () => true })
     const pluginSpecificOptions = {
+      ...manifestOptions,
       localize: [
         {
           ...manifestOptions,
@@ -292,10 +316,6 @@ describe(`Test plugin manifest options`, () => {
           start_url: `/es/`,
           lang: `es`,
         },
-        {
-          ...manifestOptions,
-          start_url: `/`,
-        },
       ],
     }
     const { localize, ...manifest } = pluginSpecificOptions
@@ -318,6 +338,57 @@ describe(`Test plugin manifest options`, () => {
       JSON.stringify(expectedResults[2])
     )
   })
+  it(`generates all language versions with pathPrefix`, async () => {
+    fs.statSync.mockReturnValueOnce({ isFile: () => true })
+    const pluginSpecificOptions = {
+      ...manifestOptions,
+      localize: [
+        {
+          ...manifestOptions,
+          start_url: `/de/`,
+          lang: `de`,
+        },
+        {
+          ...manifestOptions,
+          start_url: `/es/`,
+          lang: `es`,
+        },
+      ],
+    }
+
+    const { localize, ...manifest } = pluginSpecificOptions
+    const expectedResults = [manifest].concat(localize).map(x => {
+      return {
+        ...manifest,
+        ...x,
+        start_url: path.posix.join(`/blog`, x.start_url),
+        icons: manifest.icons.map(icon => {
+          return {
+            ...icon,
+            src: path.posix.join(`/blog`, icon.src),
+          }
+        }),
+      }
+    })
+
+    await onPostBootstrap(
+      { ...apiArgs, basePath: `/blog` },
+      pluginSpecificOptions
+    )
+
+    expect(fs.writeFileSync).toHaveBeenCalledWith(
+      expect.anything(),
+      JSON.stringify(expectedResults[0])
+    )
+    expect(fs.writeFileSync).toHaveBeenCalledWith(
+      expect.anything(),
+      JSON.stringify(expectedResults[1])
+    )
+    expect(fs.writeFileSync).toHaveBeenCalledWith(
+      expect.anything(),
+      JSON.stringify(expectedResults[2])
+    )
+  })
 
   it(`merges default and language options`, async () => {
     fs.statSync.mockReturnValueOnce({ isFile: () => true })

diff --git a/packages/gatsby-plugin-manifest/src/gatsby-node.js b/packages/gatsby-plugin-manifest/src/gatsby-node.js
@@ -1,7 +1,7 @@
 import fs from "fs"
 import path from "path"
 import sharp from "./safe-sharp"
-import { createContentDigest, cpuCoreCount } from "gatsby-core-utils"
+import { createContentDigest, cpuCoreCount, slash } from "gatsby-core-utils"
 import { defaultIcons, doesIconExist, addDigestToPath } from "./common"
 
 sharp.simd(true)
@@ -58,7 +58,7 @@ async function checkCache(cache, icon, srcIcon, srcIconDigest, callback) {
 }
 
 exports.onPostBootstrap = async (
-  { reporter, parentSpan },
+  { reporter, parentSpan, basePath },
   { localize, ...manifest }
 ) => {
   const activity = reporter.activityTimer(`Build manifest and related icons`, {
@@ -68,7 +68,7 @@ exports.onPostBootstrap = async (
 
   let cache = new Map()
 
-  await makeManifest(cache, reporter, manifest)
+  await makeManifest({ cache, reporter, pluginOptions: manifest, basePath })
 
   if (Array.isArray(localize)) {
     const locales = [...localize]
@@ -84,23 +84,44 @@ exports.onPostBootstrap = async (
           cacheModeOverride = { cache_busting_mode: `name` }
         }
 
-        return makeManifest(
+        return makeManifest({
           cache,
           reporter,
-          {
+          pluginOptions: {
             ...manifest,
             ...locale,
             ...cacheModeOverride,
           },
-          true
-        )
+          shouldLocalize: true,
+          basePath,
+        })
       })
     )
   }
   activity.end()
 }
 
-const makeManifest = async (cache, reporter, pluginOptions, shouldLocalize) => {
+/**
+ * The complete Triforce, or one or more components of the Triforce.
+ * @typedef {Object} makeManifestArgs
+ * @property {Object} cache - from gatsby-node api
+ * @property {Object} reporter - from gatsby-node api
+ * @property {Object} pluginOptions - from gatsby-node api/gatsby config
+ * @property {boolean?} shouldLocalize
+ * @property {string?} basePath - string of base path frpvided by gatsby node
+ */
+
+/**
+ * Build manifest
+ * @param {makeManifestArgs}
+ */
+const makeManifest = async ({
+  cache,
+  reporter,
+  pluginOptions,
+  shouldLocalize = false,
+  basePath = ``,
+}) => {
   const { icon, ...manifest } = pluginOptions
   const suffix =
     shouldLocalize && pluginOptions.lang ? `_${pluginOptions.lang}` : ``
@@ -198,6 +219,15 @@ const makeManifest = async (cache, reporter, pluginOptions, shouldLocalize) => {
     }
   }
 
+  //Fix #18497 by prefixing paths
+  manifest.icons = manifest.icons.map(icon => {
+    return {
+      ...icon,
+      src: slash(path.join(basePath, icon.src)),
+    }
+  })
+  manifest.start_url = path.posix.join(basePath, manifest.start_url)
+
   //Write manifest
   fs.writeFileSync(
     path.join(`public`, `manifest${suffix}.webmanifest`),

diff --git a/yarn.lock b/yarn.lock
@@ -10605,6 +10605,14 @@ gather-stream@^1.0.0:
   version "1.0.0"
   resolved "https://registry.yarnpkg.com/gather-stream/-/gather-stream-1.0.0.tgz#b33994af457a8115700d410f317733cbe7a0904b"
 
+gatsby-core-utils@^1.0.26:
+  version "1.0.26"
+  resolved "https://registry.yarnpkg.com/gatsby-core-utils/-/gatsby-core-utils-1.0.26.tgz#e1cbdfad498d58d677d9d74f21a1ede661b49d6f"
+  integrity sha512-NPflmXmyTcg3x2mp6cqp/51QeAHRKepfbf0X4erDsnVlewFJuGTe+25ZJvWkkwU2g1cPAxuwzlPe0jOL92iU4A==
+  dependencies:
+    ci-info "2.0.0"
+    node-object-hash "^2.0.0"
+
 gatsby-node-helpers@^0.3.0:
   version "0.3.0"
   resolved "https://registry.yarnpkg.com/gatsby-node-helpers/-/gatsby-node-helpers-0.3.0.tgz#3bdca3b7902a702a5834fef280ad66d51099d57c"