-
Notifications
You must be signed in to change notification settings - Fork 2.4k
enhance(build): validate built data.json
against schemas
#27685
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
base: main
Are you sure you want to change the base?
Conversation
data.json
against schemas
Tip: Review these changes grouped by change (recommended for most PRs), or grouped by feature (for large PRs). |
const writeData = async () => { | ||
const dest = new URL('data.json', targetdir); | ||
const data = await createDataBundle(); | ||
validate(data); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note
If validation fails, the validation errors are logged via console.error
, but we don't bail out, to avoid disruption for contributors in the unlikely event that this ever happens on main
. In either case, these errors won't get unnoticed, because they will appear on every npm install
(via the prepare
script).
FWIW, I raised this as a concern in #17574 and got shot down on doing anything about it. |
FWIW, here's the meta schema (isolated, like the other two): {
"$schema": "http://json-schema.org/schema",
"definitions": {
"meta_block": {
"type": "object",
"properties": {
"timestamp": {
"type": "string",
"format": "date-time"
},
"version": {
"type": "string"
}
},
"required": ["timestamp", "version"],
"additionalProperties": false
}
},
"properties": {
"__meta": {
"$ref": "#/definitions/meta_block"
}
},
"title": "MetaData",
"type": "object",
"required": ["__meta"],
"additionalProperties": false,
"maxProperties": 1,
"minProperties": 1
} Unfortunately, ajv gives me an error when validating the data:
That error doesn't make sense, but given this is not a priority, I would declare the scope of this PR to validate the built data against the existing schemas. |
The built data.json contains multiple browsers.
…clarative Push) (#28000) * Add Declarative Push * Apply suggestions from code review Co-authored-by: Claas Augner <[email protected]> * Set webview to false --------- Co-authored-by: Claas Augner <[email protected]>
…p}` (#28074) Co-authored-by: Claas Augner <[email protected]>
b884975
to
c046688
Compare
Summary
data.json
against the schemas.maxProperties
requirement for thebrowsers
object from the browsers schema.Test results and supporting details
This revealed two minor issues:
browsers
. This requirement only applies to the individualbrowser/*.json
files, not the builtdata.json
, so this requirement was removed.__meta
object in the builtdata.json
is not currently covered by the schema, so we cannot validate it.Related issues
Fixes #27564.