Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(plugin-manifest): Allow for all valid WebAppManifest properties #27951

Merged
merged 9 commits into from
Nov 14, 2020

Conversation

moonmeister
Copy link
Contributor

@moonmeister moonmeister commented Nov 10, 2020

Description

Current Plugin Options Validator for gatsby-plugin-manifest is very incomplete. This PR implements a spec compliant validator.

Documentation

I've added a not to docs about validation and the version of the spec it implements.

##TODO:

This is copied form the previous PR...testing and updates are needed.

Related Issues

fixes #27927

@gatsbot gatsbot bot added the status: triage needed Issue or pull request that need to be triaged and assigned to a reviewer label Nov 10, 2020
@moonmeister moonmeister marked this pull request as ready for review November 10, 2020 18:50
@moonmeister moonmeister requested review from a team as code owners November 10, 2020 18:50
@moonmeister
Copy link
Contributor Author

This should be good. I'll handle changing how defaults are handled in another PR so we can get this fix out ASAP.

pvdz
pvdz previously approved these changes Nov 10, 2020
Copy link
Contributor

@pvdz pvdz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nais 💜

@@ -47,4 +47,4 @@
"engines": {
"node": ">=10.13.0"
}
}
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

prettier should fix this (you could tune your editor ;) )

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(won't show up in discussions but the line ending is missing here)

mxstbr
mxstbr previously approved these changes Nov 11, 2020
Copy link
Contributor

@mxstbr mxstbr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SO GOOD!!! Thank you so much for digging into this 💯

@pvdz
Copy link
Contributor

pvdz commented Nov 11, 2020

(ran formatter)

@moonmeister moonmeister dismissed stale reviews from mxstbr and pvdz via 0d38cbf November 11, 2020 14:59
Copy link
Contributor

@mxstbr mxstbr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks fantastic to me!

@mxstbr mxstbr added the bot: merge on green Gatsbot will merge these PRs automatically when all tests passes label Nov 12, 2020
@moonmeister
Copy link
Contributor Author

This is pending a review from docs before the bot will merge.

@mxstbr mxstbr merged commit 88b990a into gatsbyjs:master Nov 14, 2020
@vladar vladar removed the status: triage needed Issue or pull request that need to be triaged and assigned to a reviewer label Nov 16, 2020
vladar pushed a commit that referenced this pull request Nov 16, 2020
…27951)

* bring in complete(albeit outdated) WebAppManifest validation frrom previous unmerged PR

* Cleanup Gatsby Plugin Options

* update validation to match recent spec changes

* update compatible gatsby version

* reomve unused service worker object and fix numberic string

* lint

* add details from suplementry spec

* final return

* Prettify

Co-authored-by: Max Stoiber <contact@mxstbr.com>
(cherry picked from commit 88b990a)
vladar added a commit that referenced this pull request Nov 17, 2020
…27951) (#28099)

Co-authored-by: Max Stoiber <contact@mxstbr.com>
Co-authored-by: Alex Moon <moonmeister@users.noreply.github.com>

(cherry picked from commit 88b990a)
@vladar
Copy link
Contributor

vladar commented Nov 17, 2020

Published in gatsby-plugin-manifest@2.6.1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bot: merge on green Gatsbot will merge these PRs automatically when all tests passes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

gatsby-plugin-manifest doesn't allow app shortcuts
5 participants