-
Notifications
You must be signed in to change notification settings - Fork 34
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
Validate theme.json files against the JSON Schema #221
Conversation
Thank you for this @ajlende ! It seems like the action is running on the pre commit hook but when I made a change to one of the theme.json files it doesn't fail as expected. On bluenote I changed this line:
to this
the precommit validation seemed to pass. Running the command manually gives me a log like so:
|
Looking at the log, which maybe could be simplified for readability, but we can look into that on a separate PR, it looks like it's treating the schema version being 2 instead of 3 as an error |
|
Ugh, you are right, I made a PR to update the ones that had the wrong version but apparently I missed stacks... |
6abb898
to
8fb8156
Compare
Imagine if you had a nice script like this when you were updating them to confirm which ones were using the wrong schema :) |
It might also be helpful to set up a GitHub action to run the verification on the files touched in the PR. I can try to do that in a follow-up. I didn't want to do that here for two reasons.
|
I updated it to use They format the errors like this: $ npx ajv-cli -s ../gutenberg/schemas/json/theme.json -d ./blue-note/theme.json --allowMatchingProperties --allErrors
./blue-note/theme.json invalid
[
{
instancePath: '/styles/elements',
schemaPath: '#/additionalProperties',
keyword: 'additionalProperties',
params: { additionalProperty: 'textarea' },
message: 'must NOT have additional properties'
},
{
instancePath: '/styles/elements',
schemaPath: '#/additionalProperties',
keyword: 'additionalProperties',
params: { additionalProperty: 'input' },
message: 'must NOT have additional properties'
}
] I didn't use that CLI here because it requires the schemas to be saved to disk. It was more complicated to download all the schemas to a temp directory and run the CLI rather than just use the AJV library directly. |
Fixed in 8fb8156. It wasn't failing because I missed renaming the command in .lintstagedrc.json and the script doesn't exit with an error when you type an incorrect command. |
Ugh, you are right, I made a PR to update the ones that had the wrong version but apparently I missed stacks... |
Now it works as intended when I run the command manually, but the pre commit hook says
When I try to commit a faulty theme.json. I tried to debug why but your code seems fine and it should be working :S |
Preview changesI've detected changes to the following themes in this PR: Purr. You can preview these changes by following the links below: I will update this comment with the latest preview links as you push more changes to this PR. |
@@ -0,0 +1,3 @@ | |||
{ | |||
"{**/theme.json,**/styles/*.json,**/assets/fonts/*.json}": "node ./theme-utils.mjs validate-schema" |
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.
🤦♀️
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.
Thank you for the follow up! It works nicely, let's bring it in
Adds a command to validate the theme.json schemas.
The command is set up to be run with
lint-staged
which runs on theprecommit
hook thanks tohusky
. It can be run manually too.$ npx lint-staged