Lint for missing frontmatter that will break build-json #209
Conversation
| const requiredFrontmatter = []; | ||
|
|
||
| // Right now, there's no way to collect the required frontmatter | ||
| // keys from the recipe itself, so they're hardcoded for now. |
There was a problem hiding this comment.
Can't you look at meta. keys that don't have a ? suffix? This will fail because we don't yet understand info_box, but it might be better to do this and comment info_box out of the recipe. Or is there some other reason this won't work?
There was a problem hiding this comment.
This would partially work. examples would be fixed by checking the recipe. But attributes is a weird one: it requires some nested keys that aren't in the recipe. global is conventional, though best as I can tell it's not used to build any JSON yet; element_specific is used in build-json but the only place it's described is in scripts/build-json/build-recipe-page-json.js.
There's a little bit of tension here in that I want to check that some frontmatter exists, but I'm not ready to build out complete checks for well-formed frontmatter (mostly because I don't think we actually know what well formed is yet).
There was a problem hiding this comment.
I think that checking the top-level properties can be done in a common way using the recipe, but any digging deeper (e.g. attributes.global) should not, and should be done individually for that ingredient. Because you have to, anyway. For example, attributes.element_specific has its own structure that you'll need to check (must be an array of paths to files, the directories must exist, maybe they must be lowercase. Then the files pointed to must also satisfy some rules). So in any case, you're going to have to write some code per-ingredient to check it. So I had envisaged you just use the recipe to check the top level, and to figure out which bit of linting code to call the check the details.
This is what build-json does: it looks at the recipe and for each top-level ingredient listed, calls some custom code to build the attributes, or the examples, or the BCD.
There was a problem hiding this comment.
OK, that sounds reasonable. I've changed it to check the recipe for top-level required frontmatter and ignore the nested keys. We can tackle the finer details (e.g., checking for valid spec URLs) in other PRs. Want to take another look?
| if (typeof entry === "string") { | ||
| return entry; | ||
| } else { | ||
| return Object.keys(entry)[0]; |
There was a problem hiding this comment.
Sorry to bring this up only now, but I think recipe body items should only be strings, see #106. We should fix css-property's info_box accordingly.
There was a problem hiding this comment.
I'm not clear on what the actual fix is here. Do I remove the nested info_box pieces? Or are we changing the notation for info box (e.g., info_box_thing1 or info_box.thing1 as top-level items)?
There was a problem hiding this comment.
For HTML elements we've removed the nested pieces, so the recipe is like:
...
- prose.*
- meta.examples
- meta.info_box
- meta.specifications
- meta.browser_compatibility
...
I think we should do the same for CSS properties. (I haven't done any maintaining of the CSS properties while I've been wrestling only with HTML documentation, so it has fallen behind. Which was fine, except we are now linting them).
There was a problem hiding this comment.
OK, thank you. I understand now. I've pushed a change to the recipe and removed the type checking on the recipe body entries.
There was a problem hiding this comment.
Thank you @ddbeck , this looks good and works well.
It does now introduce new errors, for the CSS properties. I'm not sure whether we should fix them, or just delete the CSS property pages, since we're not focusing on them at the moment. Probably the latter.
While working on #207, I wrote this to check for
build-json-breaking frontmatter omissions. It's only got a few right now, and they're hard coded, but I imagine we can build on this. Thankfully, this doesn't introduce any new warnings. 😃