feat(gatsby-plugin-manifest): add icon_options as an option to support the purpose property

[kripod]

Description

Adds an option to specify PWA icon options, e.g. purpose.

Related Issues

Resolves #12793

[wardpeet]

It might be handy to add some documentation on how maskable works or when to use it. w3 links aren't super friendly for beginners

[sidharthachatterjee]

Thank you for adding this, @kripod 👍

Left some nits and a link to MDN instead of W3

[kripod]

Thank you for the review and the suggestions! MDN is a good idea, but unfortunately, it isn't nearly as detailed as W3C in this scenario.

[sidharthachatterjee]

@kripod Yeah, that's true. Do you think we should add a small comment on how maskable works like @wardpeet recommended?

[kripod]

@sidharthachatterjee yes, probably, until the docs of MDN get improved

[moonmeister]

@kripod I'm all for over documenting. I say we leave MDN in the comment and link w3 in the docs as an additional resource. that should cover everyone's curiosities without re-documenting the manifest spec itself.

[moonmeister]

Sry guys, but this PR has some breaking bugs. I didn't take a close enough look before it was merged. Namely, if this object isn't specified it'll fail cause the "forEach" method doesn't exist on an undefined object. Second, the array is never deleted from what gets written to the Manifest file.

I already have a clean up PR going to the Manifest code at #12757, I can add the fixes there. we just can't release without that PR then cause these will break everybody's build.

UPDATE: My bad, the forEach is on manifest.icons...that should mean it's not breaking but just runs merging an undefined object.

UPDATE 2: Another potential thing. Right now the contents of icon_options will override anything already in this icons array. Meaning, if you needed one icon to be unique, but still want these setting be applied to all other icons, there would be no way to override. Should it be switched so that any settings in the icons array overwrite the icon_options?

[kripod]

@moonmeister Overriding icons with icon_options is intentional, as by default, only the most basic options are specified for each icon (e.g. individual size/format). Merging an undefined object is perfectly fine and has a well-specified behavior as far as I know.

[moonmeister]

@kripod let me clarify: Currently, if I set my config like this:

{
  icon_options: {
    purpose: `maskable`,
  },
  icons: [
    {
      src: `icons/icon-48x48.png`,
      sizes: `48x48`,
      type: `image/png`,
      purpose: `all`,
    },
    {
      src: `icons/icon-128x128.png`,
      sizes: `128x128`,
      type: `image/png`,
    },
  ],
}

The resulting config will be:

{
  icons: [
    {
      src: `icons/icon-48x48.png`,
      sizes: `48x48`,
      type: `image/png`,
      purpose: `maskable`, // notice despite setting this in the 'icon' array, it has been changed.
    },
    {
      src: `icons/icon-128x128.png`,
      sizes: `128x128`,
      type: `image/png`,
      purpose: `maskable`, //this is applied
    },
  ],
}

What I am suggesting is that we make the resulting config be this:

{
  icons: [
    {
      src: `icons/icon-48x48.png`,
      sizes: `48x48`,
      type: `image/png`,
      purpose: `any`, // notice despite setting this in the 'icon_options' object, my setting from the 'icon' array has persisted.
    },
    {
      src: `icons/icon-128x128.png`,
      sizes: `128x128`,
      type: `image/png`,
      purpose: `maskable`, // this is is still applied
    },
  ],
}

This allows the more specific icon array override the more general icon_options. which makes logical sense to me. Second, it keeps less informed devs from overriding defaults that must be unique, like src, sizes, or type.

[kripod]

@moonmeister Thank you for the clear example! I agree with your points and support swapping the order of precedence.

[wardpeet]

@moonmeister as discussed in the oss meeting it's not a breaking issue. Browsers are super smart and actually do progressive enhancement pretty great. It doesn't really matter if extra stuff is in the manifest so it's not a breaking change :p but something we want to like fixed of course