From 201a4f50383dfe47a61c504c61fe90262bd4c28b Mon Sep 17 00:00:00 2001 From: Alex Moon Date: Fri, 29 Mar 2019 21:55:36 +0800 Subject: [PATCH] fix(gatsby-plugin-manifest): ensure icon_options is stripped (#12907) * fix: bugs in 'icon_options' implementation * fix: make icons array take presidence over icon_options * test: write icon_options tests and fix bug * fix: small things * Update packages/gatsby-plugin-manifest/src/__tests__/gatsby-node.js Co-Authored-By: moonmeister * Update packages/gatsby-plugin-manifest/src/__tests__/gatsby-node.js Co-Authored-By: moonmeister * test: add test to validate icon_options object is removed from manifest * Update packages/gatsby-plugin-manifest/README.md Co-Authored-By: moonmeister --- packages/gatsby-plugin-manifest/README.md | 6 +- .../src/__tests__/gatsby-node.js | 82 ++++++++++++------- .../gatsby-plugin-manifest/src/gatsby-node.js | 12 ++- 3 files changed, 65 insertions(+), 35 deletions(-) diff --git a/packages/gatsby-plugin-manifest/README.md b/packages/gatsby-plugin-manifest/README.md index 989249ab7353c..5104cdf5766df 100644 --- a/packages/gatsby-plugin-manifest/README.md +++ b/packages/gatsby-plugin-manifest/README.md @@ -190,7 +190,9 @@ module.exports = { ## Custom icon options -In order to specify manifest options merged with each item of the `icons` array, `icon_options` may be used as follows: +The `icon_options` object may be used to iteratively add configuration items to the `icons` array. Any options included in this object will be merged with each object of the `icons` array (custom or default). Key value pairs already in the `icons` array will take precedence over duplicate items in the `icon_options` array. + +`icon_options` may be used as follows: ```js // in gatsby-config.js @@ -205,7 +207,7 @@ module.exports = { background_color: `#f7f0eb`, theme_color: `#a2466c`, display: `standalone`, - icon: `src/images/icon.png`, // This path is relative to the root of the site. + icon: `src/images/icon.png`, icon_options: { // For all the options available, please see: // https://developer.mozilla.org/en-US/docs/Web/Manifest diff --git a/packages/gatsby-plugin-manifest/src/__tests__/gatsby-node.js b/packages/gatsby-plugin-manifest/src/__tests__/gatsby-node.js index 718b5e5c9350e..995c92427ed43 100644 --- a/packages/gatsby-plugin-manifest/src/__tests__/gatsby-node.js +++ b/packages/gatsby-plugin-manifest/src/__tests__/gatsby-node.js @@ -60,6 +60,12 @@ const manifestOptions = { src: `icons/icon-48x48.png`, sizes: `48x48`, type: `image/png`, + purpose: `all`, + }, + { + src: `icons/icon-128x128.png`, + sizes: `128x128`, + type: `image/png`, }, ], } @@ -99,23 +105,22 @@ describe(`Test plugin manifest options`, () => { const icon = `pretend/this/exists.png` const size = 48 + const pluginSpecificOptions = { + icon: icon, + icons: [ + { + src: `icons/icon-48x48.png`, + sizes: `${size}x${size}`, + type: `image/png`, + }, + ], + } + await onPostBootstrap( { reporter }, { - name: `GatsbyJS`, - short_name: `GatsbyJS`, - start_url: `/`, - background_color: `#f7f0eb`, - theme_color: `#a2466c`, - display: `standalone`, - icon, - icons: [ - { - src: `icons/icon-48x48.png`, - sizes: `${size}x${size}`, - type: `image/png`, - }, - ], + ...manifestOptions, + ...pluginSpecificOptions, } ) @@ -126,23 +131,15 @@ describe(`Test plugin manifest options`, () => { it(`fails on non existing icon`, async () => { fs.statSync.mockReturnValueOnce({ isFile: () => false }) + const pluginSpecificOptions = { + icon: `non/existing/path`, + } + return onPostBootstrap( { reporter }, { - name: `GatsbyJS`, - short_name: `GatsbyJS`, - start_url: `/`, - background_color: `#f7f0eb`, - theme_color: `#a2466c`, - display: `standalone`, - icon: `non/existing/path`, - icons: [ - { - src: `icons/icon-48x48.png`, - sizes: `48x48`, - type: `image/png`, - }, - ], + ...manifestOptions, + ...pluginSpecificOptions, } ).catch(err => { expect(sharp).toHaveBeenCalledTimes(0) @@ -159,6 +156,7 @@ describe(`Test plugin manifest options`, () => { plugins: [], theme_color_in_head: false, cache_busting_mode: `name`, + icon_options: {}, } await onPostBootstrap( { reporter }, @@ -167,6 +165,7 @@ describe(`Test plugin manifest options`, () => { ...pluginSpecificOptions, } ) + expect(sharp).toHaveBeenCalledTimes(0) const content = JSON.parse(fs.writeFileSync.mock.calls[0][1]) expect(content).toEqual(manifestOptions) @@ -188,7 +187,7 @@ describe(`Test plugin manifest options`, () => { } ) - expect(sharp).toHaveBeenCalledTimes(2) + expect(sharp).toHaveBeenCalledTimes(3) const content = JSON.parse(fs.writeFileSync.mock.calls[0][1]) expect(content).toEqual(manifestOptions) }) @@ -209,8 +208,31 @@ describe(`Test plugin manifest options`, () => { } ) - expect(sharp).toHaveBeenCalledTimes(2) + expect(sharp).toHaveBeenCalledTimes(3) const content = JSON.parse(fs.writeFileSync.mock.calls[0][1]) expect(content).toEqual(manifestOptions) }) + + it(`icon options iterator adds options and the icon array take precedence`, async () => { + fs.statSync.mockReturnValueOnce({ isFile: () => true }) + + const pluginSpecificOptions = { + icon: `images/gatsby-logo.png`, + icon_options: { + purpose: `maskable`, + }, + } + await onPostBootstrap( + { reporter }, + { + ...manifestOptions, + ...pluginSpecificOptions, + } + ) + + expect(sharp).toHaveBeenCalledTimes(3) + const content = JSON.parse(fs.writeFileSync.mock.calls[0][1]) + expect(content.icons[0].purpose).toEqual(`all`) + expect(content.icons[1].purpose).toEqual(`maskable`) + }) }) diff --git a/packages/gatsby-plugin-manifest/src/gatsby-node.js b/packages/gatsby-plugin-manifest/src/gatsby-node.js index fa7c3efea1375..c8698726b3677 100644 --- a/packages/gatsby-plugin-manifest/src/gatsby-node.js +++ b/packages/gatsby-plugin-manifest/src/gatsby-node.js @@ -52,6 +52,7 @@ exports.onPostBootstrap = async ({ reporter }, pluginOptions) => { delete manifest.theme_color_in_head delete manifest.cache_busting_mode delete manifest.crossOrigin + delete manifest.icon_options // If icons are not manually defined, use the default icon set. if (!manifest.icons) { @@ -59,9 +60,14 @@ exports.onPostBootstrap = async ({ reporter }, pluginOptions) => { } // Specify extra options for each icon (if requested). - manifest.icons.forEach(icon => { - Object.assign(icon, pluginOptions.icon_options) - }) + if (pluginOptions.icon_options) { + manifest.icons = manifest.icons.map(icon => { + return { + ...pluginOptions.icon_options, + ...icon, + } + }) + } // Determine destination path for icons. const iconPath = path.join(`public`, path.dirname(manifest.icons[0].src))