Skip to content

Commit

Permalink
[gatsby-plugin-manifest] Generate a 32x32 favicon instead of generati…
Browse files Browse the repository at this point in the history 
…ng from first manifest icon (#23077)

* Decouple favicons from manifest icons

* Update packages/gatsby-plugin-manifest/README.md

* Replace include_favicon undefined check with nullish coalescing operator

Co-Authored-By: Alex Moon <moonmeister@users.noreply.github.com>

* Assert gatsby-plugin-manifest generates favicon PNG (unless include_favicon option is false)

* Don't mock createContentDigest in gatsby-plugin-manifest's gatsby-ssr tests (to be consistent with gatsby-node tests)

* Tweak a comment

Co-authored-by: LB <laurie@gatsbyjs.com>
Co-authored-by: Alex Moon <moonmeister@users.noreply.github.com>
  • Loading branch information
@moonmeister
3 people committed Apr 21, 2020
1 parent 3c123c7 commit 8cfe41b
Show file tree
Hide file tree
Showing 8 changed files with 166 additions and 82 deletions.
4 changes: 3 additions & 1 deletion packages/gatsby-plugin-manifest/README.md
View file
Expand Up @@ -238,7 +238,9 @@ module.exports = {

#### Disable favicon

Excludes `<link rel="icon" href="/favicon.png" />` link tag to html output. You can set `include_favicon` plugin option to `false` to opt-out of this behavior.
A favicon is generated by default in automatic and hybrid modes (a 32x32 PNG, included via a `<link rel="icon" />` tag in the document head).

You can set the `include_favicon` plugin option to `false` to opt-out of this behavior.

```js
// in gatsby-config.js
Expand Down
2 changes: 1 addition & 1 deletion packages/gatsby-plugin-manifest/src/__tests__/__snapshots__/gatsby-node.js.snap
View file
Expand Up @@ -9,7 +9,7 @@ exports[`Test plugin manifest options does file name based cache busting 1`] = `
"calls": Array [
Array [
"public/manifest.webmanifest",
"{\\"name\\":\\"GatsbyJS\\",\\"short_name\\":\\"GatsbyJS\\",\\"start_url\\":\\"/\\",\\"background_color\\":\\"#f7f0eb\\",\\"theme_color\\":\\"#a2466c\\",\\"display\\":\\"standalone\\",\\"icons\\":[{\\"src\\":\\"icons/icon-48x48-contentDigest.png\\",\\"sizes\\":\\"48x48\\",\\"type\\":\\"image/png\\",\\"purpose\\":\\"all\\"},{\\"src\\":\\"icons/icon-128x128-contentDigest.png\\",\\"sizes\\":\\"128x128\\",\\"type\\":\\"image/png\\"}]}",
"{\\"name\\":\\"GatsbyJS\\",\\"short_name\\":\\"GatsbyJS\\",\\"start_url\\":\\"/\\",\\"background_color\\":\\"#f7f0eb\\",\\"theme_color\\":\\"#a2466c\\",\\"display\\":\\"standalone\\",\\"icons\\":[{\\"src\\":\\"icons/icon-48x48-00913339321ee5a854812aea11f8a5d4.png\\",\\"sizes\\":\\"48x48\\",\\"type\\":\\"image/png\\",\\"purpose\\":\\"all\\"},{\\"src\\":\\"icons/icon-128x128-00913339321ee5a854812aea11f8a5d4.png\\",\\"sizes\\":\\"128x128\\",\\"type\\":\\"image/png\\"}]}",
],
],
"results": Array [
Expand Down
76 changes: 38 additions & 38 deletions packages/gatsby-plugin-manifest/src/__tests__/__snapshots__/gatsby-ssr.js.snap
View file
Expand Up @@ -32,50 +32,50 @@ Array [
exports[`gatsby-plugin-manifest Cache Busting Does file name cache busting if "cache_busting_mode" option is set to name 1`] = `
Array [
<link
href="/icons/icon-48x48-contentDigest.png"
href="/favicon-32x32-00913339321ee5a854812aea11f8a5d4.png"
rel="icon"
/>,
<link
href="/manifest.webmanifest"
rel="manifest"
/>,
<link
href="/icons/icon-48x48-contentDigest.png"
href="/icons/icon-48x48-00913339321ee5a854812aea11f8a5d4.png"
rel="apple-touch-icon"
sizes="48x48"
/>,
<link
href="/icons/icon-72x72-contentDigest.png"
href="/icons/icon-72x72-00913339321ee5a854812aea11f8a5d4.png"
rel="apple-touch-icon"
sizes="72x72"
/>,
<link
href="/icons/icon-96x96-contentDigest.png"
href="/icons/icon-96x96-00913339321ee5a854812aea11f8a5d4.png"
rel="apple-touch-icon"
sizes="96x96"
/>,
<link
href="/icons/icon-144x144-contentDigest.png"
href="/icons/icon-144x144-00913339321ee5a854812aea11f8a5d4.png"
rel="apple-touch-icon"
sizes="144x144"
/>,
<link
href="/icons/icon-192x192-contentDigest.png"
href="/icons/icon-192x192-00913339321ee5a854812aea11f8a5d4.png"
rel="apple-touch-icon"
sizes="192x192"
/>,
<link
href="/icons/icon-256x256-contentDigest.png"
href="/icons/icon-256x256-00913339321ee5a854812aea11f8a5d4.png"
rel="apple-touch-icon"
sizes="256x256"
/>,
<link
href="/icons/icon-384x384-contentDigest.png"
href="/icons/icon-384x384-00913339321ee5a854812aea11f8a5d4.png"
rel="apple-touch-icon"
sizes="384x384"
/>,
<link
href="/icons/icon-512x512-contentDigest.png"
href="/icons/icon-512x512-00913339321ee5a854812aea11f8a5d4.png"
rel="apple-touch-icon"
sizes="512x512"
/>,
Expand All @@ -85,50 +85,50 @@ Array [
exports[`gatsby-plugin-manifest Cache Busting Does query cache busting if "cache_busting_mode" option is set to query 1`] = `
Array [
<link
href="/icons/icon-48x48.png?v=contentDigest"
href="/favicon-32x32.png?v=00913339321ee5a854812aea11f8a5d4"
rel="icon"
/>,
<link
href="/manifest.webmanifest"
rel="manifest"
/>,
<link
href="/icons/icon-48x48.png?v=contentDigest"
href="/icons/icon-48x48.png?v=00913339321ee5a854812aea11f8a5d4"
rel="apple-touch-icon"
sizes="48x48"
/>,
<link
href="/icons/icon-72x72.png?v=contentDigest"
href="/icons/icon-72x72.png?v=00913339321ee5a854812aea11f8a5d4"
rel="apple-touch-icon"
sizes="72x72"
/>,
<link
href="/icons/icon-96x96.png?v=contentDigest"
href="/icons/icon-96x96.png?v=00913339321ee5a854812aea11f8a5d4"
rel="apple-touch-icon"
sizes="96x96"
/>,
<link
href="/icons/icon-144x144.png?v=contentDigest"
href="/icons/icon-144x144.png?v=00913339321ee5a854812aea11f8a5d4"
rel="apple-touch-icon"
sizes="144x144"
/>,
<link
href="/icons/icon-192x192.png?v=contentDigest"
href="/icons/icon-192x192.png?v=00913339321ee5a854812aea11f8a5d4"
rel="apple-touch-icon"
sizes="192x192"
/>,
<link
href="/icons/icon-256x256.png?v=contentDigest"
href="/icons/icon-256x256.png?v=00913339321ee5a854812aea11f8a5d4"
rel="apple-touch-icon"
sizes="256x256"
/>,
<link
href="/icons/icon-384x384.png?v=contentDigest"
href="/icons/icon-384x384.png?v=00913339321ee5a854812aea11f8a5d4"
rel="apple-touch-icon"
sizes="384x384"
/>,
<link
href="/icons/icon-512x512.png?v=contentDigest"
href="/icons/icon-512x512.png?v=00913339321ee5a854812aea11f8a5d4"
rel="apple-touch-icon"
sizes="512x512"
/>,
Expand All @@ -138,50 +138,50 @@ Array [
exports[`gatsby-plugin-manifest Cache Busting Does query cache busting if "cache_busting_mode" option is set to undefined 1`] = `
Array [
<link
href="/icons/icon-48x48.png?v=contentDigest"
href="/favicon-32x32.png?v=00913339321ee5a854812aea11f8a5d4"
rel="icon"
/>,
<link
href="/manifest.webmanifest"
rel="manifest"
/>,
<link
href="/icons/icon-48x48.png?v=contentDigest"
href="/icons/icon-48x48.png?v=00913339321ee5a854812aea11f8a5d4"
rel="apple-touch-icon"
sizes="48x48"
/>,
<link
href="/icons/icon-72x72.png?v=contentDigest"
href="/icons/icon-72x72.png?v=00913339321ee5a854812aea11f8a5d4"
rel="apple-touch-icon"
sizes="72x72"
/>,
<link
href="/icons/icon-96x96.png?v=contentDigest"
href="/icons/icon-96x96.png?v=00913339321ee5a854812aea11f8a5d4"
rel="apple-touch-icon"
sizes="96x96"
/>,
<link
href="/icons/icon-144x144.png?v=contentDigest"
href="/icons/icon-144x144.png?v=00913339321ee5a854812aea11f8a5d4"
rel="apple-touch-icon"
sizes="144x144"
/>,
<link
href="/icons/icon-192x192.png?v=contentDigest"
href="/icons/icon-192x192.png?v=00913339321ee5a854812aea11f8a5d4"
rel="apple-touch-icon"
sizes="192x192"
/>,
<link
href="/icons/icon-256x256.png?v=contentDigest"
href="/icons/icon-256x256.png?v=00913339321ee5a854812aea11f8a5d4"
rel="apple-touch-icon"
sizes="256x256"
/>,
<link
href="/icons/icon-384x384.png?v=contentDigest"
href="/icons/icon-384x384.png?v=00913339321ee5a854812aea11f8a5d4"
rel="apple-touch-icon"
sizes="384x384"
/>,
<link
href="/icons/icon-512x512.png?v=contentDigest"
href="/icons/icon-512x512.png?v=00913339321ee5a854812aea11f8a5d4"
rel="apple-touch-icon"
sizes="512x512"
/>,
Expand All @@ -191,7 +191,7 @@ Array [
exports[`gatsby-plugin-manifest Cache Busting doesn't add cache busting if "cache_busting_mode" option is set to none 1`] = `
Array [
<link
href="/icons/icon-48x48.png"
href="/favicon-32x32.png"
rel="icon"
/>,
<link
Expand Down Expand Up @@ -263,7 +263,7 @@ Array [
exports[`gatsby-plugin-manifest Favicon Adds link favicon tag if "include_favicon" is set to true 1`] = `
Array [
<link
href="/icons/icon-48x48.png"
href="/favicon-32x32.png"
rel="icon"
/>,
<link
Expand Down Expand Up @@ -425,7 +425,7 @@ Array [
exports[`gatsby-plugin-manifest Manifest Link Generation Adds "icon" and "manifest" links and "theme_color" meta tag to head 1`] = `
Array [
<link
href="/icons/icon-48x48.png?v=contentDigest"
href="/favicon-32x32.png?v=00913339321ee5a854812aea11f8a5d4"
rel="icon"
/>,
<link
Expand All @@ -437,42 +437,42 @@ Array [
name="theme-color"
/>,
<link
href="/icons/icon-48x48.png?v=contentDigest"
href="/icons/icon-48x48.png?v=00913339321ee5a854812aea11f8a5d4"
rel="apple-touch-icon"
sizes="48x48"
/>,
<link
href="/icons/icon-72x72.png?v=contentDigest"
href="/icons/icon-72x72.png?v=00913339321ee5a854812aea11f8a5d4"
rel="apple-touch-icon"
sizes="72x72"
/>,
<link
href="/icons/icon-96x96.png?v=contentDigest"
href="/icons/icon-96x96.png?v=00913339321ee5a854812aea11f8a5d4"
rel="apple-touch-icon"
sizes="96x96"
/>,
<link
href="/icons/icon-144x144.png?v=contentDigest"
href="/icons/icon-144x144.png?v=00913339321ee5a854812aea11f8a5d4"
rel="apple-touch-icon"
sizes="144x144"
/>,
<link
href="/icons/icon-192x192.png?v=contentDigest"
href="/icons/icon-192x192.png?v=00913339321ee5a854812aea11f8a5d4"
rel="apple-touch-icon"
sizes="192x192"
/>,
<link
href="/icons/icon-256x256.png?v=contentDigest"
href="/icons/icon-256x256.png?v=00913339321ee5a854812aea11f8a5d4"
rel="apple-touch-icon"
sizes="256x256"
/>,
<link
href="/icons/icon-384x384.png?v=contentDigest"
href="/icons/icon-384x384.png?v=00913339321ee5a854812aea11f8a5d4"
rel="apple-touch-icon"
sizes="384x384"
/>,
<link
href="/icons/icon-512x512.png?v=contentDigest"
href="/icons/icon-512x512.png?v=00913339321ee5a854812aea11f8a5d4"
rel="apple-touch-icon"
sizes="512x512"
/>,
Expand Down
63 changes: 59 additions & 4 deletions packages/gatsby-plugin-manifest/src/__tests__/gatsby-node.js
View file
Expand Up @@ -43,7 +43,7 @@ jest.mock(`gatsby-core-utils`, () => {
return {
slash: originalCoreUtils.slash,
cpuCoreCount: jest.fn(() => `1`),
createContentDigest: jest.fn(() => `contentDigest`),
createContentDigest: originalCoreUtils.createContentDigest,
}
})

Expand Down Expand Up @@ -86,6 +86,13 @@ const manifestOptions = {
],
}

// Some of these tests check the number of sharp calls to assert that the
// correct number of images are created.
//
// As long as an `icon` is defined in the config, there's always an extra
// call to sharp to check the source icon is square. Therefore the assertions
// check for N + 1 sharp calls, where N is the expected number of icons
// generated.
describe(`Test plugin manifest options`, () => {
beforeEach(() => {
fs.writeFileSync.mockReset()
Expand Down Expand Up @@ -151,6 +158,9 @@ describe(`Test plugin manifest options`, () => {
path.dirname(`other-icons/icon-48x48.png`)
)

// No sharp calls because this is manual mode: user provides all icon sizes
// rather than the plugin generating them
expect(sharp).toHaveBeenCalledTimes(0)
expect(fs.mkdirSync).toHaveBeenNthCalledWith(1, firstIconPath)
expect(fs.mkdirSync).toHaveBeenNthCalledWith(2, secondIconPath)
})
Expand All @@ -177,8 +187,44 @@ describe(`Test plugin manifest options`, () => {
...pluginSpecificOptions,
})

// One call to sharp to check the source icon is square
// + another for the favicon (enabled by default)
// + another for the single icon in the `icons` config
// => 3 total calls
expect(sharp).toHaveBeenCalledTimes(3)
expect(sharp).toHaveBeenCalledWith(icon, { density: 32 }) // the default favicon
expect(sharp).toHaveBeenCalledWith(icon, { density: size })
})

it(`skips favicon generation if "include_favicon" option is set to false`, async () => {
fs.statSync.mockReturnValueOnce({ isFile: () => true })

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`,
},
],
include_favicon: false,
}

await onPostBootstrap(apiArgs, {
...manifestOptions,
...pluginSpecificOptions,
})

// Only two sharp calls here: one to check the source icon size,
// and another to generate the single icon in the config.
// By default, there would be a 3rd call for the favicon, but that's
// disabled by the `include_favicon` option.
expect(sharp).toHaveBeenCalledTimes(2)
expect(sharp).toHaveBeenCalledWith(icon, { density: size })
})

it(`fails on non existing icon`, async () => {
Expand Down Expand Up @@ -236,7 +282,11 @@ describe(`Test plugin manifest options`, () => {
...pluginSpecificOptions,
})

expect(sharp).toHaveBeenCalledTimes(2)
// Two icons in the config, plus a favicon, plus one call to check the
// source icon size => 4 total calls to sharp.
expect(sharp).toHaveBeenCalledTimes(4)

// Filenames in the manifest should be suffixed with the content digest
expect(fs.writeFileSync).toMatchSnapshot()
})

Expand All @@ -253,7 +303,9 @@ describe(`Test plugin manifest options`, () => {
...pluginSpecificOptions,
})

expect(sharp).toHaveBeenCalledTimes(2)
// Two icons in the config, plus a favicon, plus one call to check the
// source icon size => 4 total calls to sharp.
expect(sharp).toHaveBeenCalledTimes(4)
expect(fs.writeFileSync).toHaveBeenCalledWith(
expect.anything(),
JSON.stringify(manifestOptions)
Expand All @@ -274,7 +326,9 @@ describe(`Test plugin manifest options`, () => {
...pluginSpecificOptions,
})

expect(sharp).toHaveBeenCalledTimes(2)
// Two icons in the config, plus a favicon, plus one call to check the
// source icon size => 4 total calls to sharp.
expect(sharp).toHaveBeenCalledTimes(4)
const content = JSON.parse(fs.writeFileSync.mock.calls[0][1])
expect(content.icons[0].purpose).toEqual(`all`)
expect(content.icons[1].purpose).toEqual(`maskable`)
Expand Down Expand Up @@ -338,6 +392,7 @@ describe(`Test plugin manifest options`, () => {
JSON.stringify(expectedResults[2])
)
})

it(`generates all language versions with pathPrefix`, async () => {
fs.statSync.mockReturnValueOnce({ isFile: () => true })
const pluginSpecificOptions = {
Expand Down

0 comments on commit 8cfe41b

Please sign in to comment.