gatsby-plugin-manifest doesn't allow app shortcuts

Created on 9 Nov 2020  路  11Comments  路  Source: gatsbyjs/gatsby

Description

Adding app shortcuts to the plugin options in gatsby-plugin-manifest 2.5.0+ results in an _Invalid plugin options_ error

I _think_ this was caused by enabling 'plugin option validation', released in #27437 and published in 2.5.0 of gatsby-plugin-manifest. The new options validation doesn't include the shortcuts option. It worked before 2.5.0 - I've just upgraded from 2.4.21 where it was working.

Steps to reproduce

  1. Install gatsby-plugin-manifest 2.5.0
  2. Add shortcuts to the plugin's options:
// gatsby-config.s
module.exports = {
  plugins: [
    {
      resolve: `gatsby-plugin-manifest`,
      options: {
+        shortcuts: [
+          { name: "Topics A to Z", url: "/topics/?utm_source=a2hs&utm_medium=shortcuts", }
+        ]
      }
    }
  ]
}

Expected result

Shortcuts are added to the generated _manifest.webmanifest_

Actual result

You get the following CLI error:

error Invalid plugin options for "gatsby-plugin-manifest": "shortcuts" is not allowed

Environment

System:
OS: Windows 10 10.0.19041
CPU: (4) x64 Intel(R) Core(TM) i7-5500U CPU @ 2.40GHz
Binaries:
Node: 12.16.3 - C:\Program Files\nodejs\node.EXE
Yarn: 1.16.0 - C:\Program Files (x86)\Yarn\bin\yarn.CMD
npm: 6.14.4 - C:\Program Files\nodejs\npm.CMD
Languages:
Python: 2.7.11
Browsers:
Chrome: 86.0.4240.183
Edge: Spartan (44.19041.423.0)
npmPackages:
gatsby: ^2.25.3 => 2.25.3
gatsby-plugin-catch-links: ^2.3.15 => 2.3.15
gatsby-plugin-eslint: ^2.0.8 => 2.0.8
gatsby-plugin-google-tagmanager: ^2.4.0 => 2.4.0
gatsby-plugin-manifest: ^2.5.2 => 2.5.2
gatsby-plugin-offline: ^3.3.2 => 3.3.2
gatsby-plugin-preact: ^4.0.16 => 4.0.16
gatsby-plugin-prefetch-google-fonts: ^1.4.3 => 1.4.3
gatsby-plugin-react-helmet: ^3.3.14 => 3.3.14
gatsby-plugin-sass: ^2.4.2 => 2.4.2
gatsby-plugin-sitemap: ^2.5.1 => 2.5.1
gatsby-plugin-split-css: ^1.0.2 => 1.0.2
gatsby-plugin-typescript: ^2.5.0 => 2.5.0
gatsby-react-router-scroll: 3.0.0 => 3.0.0

PWA bug

Most helpful comment

@mxstbr I wish I could build such a big code base with so few (noticeable) bugs. Congrats and thanks for an overall astoundingly great job!

All 11 comments

Looks like there are other properties missing too, see https://github.com/gatsbyjs/gatsby/issues/27839

Seems to me that was a breaking change - should have been expressed in the version number

@barbalex I think you're right! I see you've worked round it in #27839 by downgrading - I think I'll do the same for the timebeing.

I'm happy to submit a PR to add shortcuts and scope as properties - hopefully that'll fix both issues. I'll also do a quick scan of https://web.dev/add-manifest/#manifest-properties whilst I'm there to see if there's anything else obviously missing. Watch this space!

Not a breaking change, simply an oversight bug! Sorry about the troubles, looking forward to your PR @ediblecode :pray: The code for this is in gatsby-plugin-manifest's gatsby-node.js.

@mxstbr I wish I could build such a big code base with so few (noticeable) bugs. Congrats and thanks for an overall astoundingly great job!

The validation is incomplete for manifest. I have an old PR with the correct validation. I'll try and get that pulled forward and merged. I'll post a PR link here when I have it.

Perfect, thanks @moonmeister, I'll hold off until you've found that that then, thanks.

And related, there's a new flag coming to disable option validation, see #27885 which will be a nicer workaround than downgrading

27951 has the changes from the old PR. The spec version that validator was built against is over a year old. If anyone want's to help get it updated that'd be great. Current spec is here: https://www.w3.org/TR/2020/WD-appmanifest-20201019/

@moonmeister this looks great. I've been through with a fairly fine-toothed comb, and it looks like you've captured everything in the latest spec, so great work! I had a flick through various previous iterations of the spec, and although it's changed a lot, most are wording changes - it doesn't look like any properties have changed recently as far as I could tell.

There are only a few, minor things I've spotted but they really are nitpicks:

  • The categories member recommends "Manifest authors are encouraged to use lower-case." in the spec so mabye we could add .lowercase()? It doesn't really matter, because they'll get lowercased anyway.
  • The type member on ManifestImageResource is missing optional. (I know optional is optional but it's explicit everywhere else)
  • There's a supplementary spec (I know, not confusing at all) that contains extra members. The two relevant ones are platform and label. I suggest maybe we at least add label to screenshots because "For accessibility, authors are encouraged to provide a label for each screenshot."

And related, there's a new flag coming to disable option validation, see #27885 which will be a nicer workaround than downgrading

I actually paused that PR to disable validation for a couple reasons (https://github.com/gatsbyjs/gatsby/pull/27885#issuecomment-726214558) mainly because I think we just merged in a more ideal solution, which is warning about invalid options rather than failing to build. If we find that we really need the bypass flag we could add it, but I think cases like this would be covered by what was done in https://github.com/gatsbyjs/gatsby/pull/27938.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

kalinchernev picture kalinchernev  路  3Comments

ferMartz picture ferMartz  路  3Comments

brandonmp picture brandonmp  路  3Comments

ghost picture ghost  路  3Comments

signalwerk picture signalwerk  路  3Comments