Add a Specifications macro and extract a special specification section to the Document

[Elchi3]

We have specification URLs on MDN in HTML tables, see https://github.com/mdn/content/blob/main/files/en-us/web/javascript/reference/global_objects/array/tolocalestring/index.html#L159. This is not very cool.

We have spec_urls in BCD! https://github.com/mdn/browser-compat-data/blob/main/javascript/builtins/Array.json#L1871 This is very cool! They are maintained, up-to-date, and other projects use them!

So, I was thinking: How can MDN use the spec urls from BCD instead of the weird HTML tables with the laborious "specName" macros? And so I thought maybe we could model it the way Yari does the compat tables.

So analog to the compat special section, have a special "Specification" section! Meaning that authors would write this:

<h2 id="Specifications">Specifications</h2>
<p>{{Specifications}}</p>

Which Kumascript turns into

<h2 id="Specifications">Specifications</h2>
<div class="bc-specs" data-bcd-query="foo.bar.thing">...</div>

Which is then picked up by build/document-extractor.js so that there is a special document section called "specifications".

Which then a SpecificationSection renderer can render using BCD spec_url and probably other data.

Currently, the specName macro uses https://github.com/mdn/yari/blob/main/kumascript/macros/SpecData.json but that file outdates quickly as well. So, for fancy specification table rendering, we probably want to look into loading some w3c maintained data like https://github.com/w3c/browser-specs which will allow us have more spec information to render neatly into an MDN specification section. But I didn't do this yet. I first wanted some feedback on whether it is at all a good idea to go down this path.

That said, I'm eager to hear if this is complete non-sense or if there are parts that make sense and that could get us to better "Specification" sections on MDN. Thanks for looking into this and for making yari hackable for anyone really! :)

[Ryuno-Ki]

I only read the code here, but haven't checked it out.

My understanding is, that the table assumes, that the spec data follows a similar shape of the data than we have for the tables we use already.

Can you attach a screenshot of a rendered table and the markup it uses under the hood?

[peterbe]

@Elchi3 I haven't even started to review but I looooove the idea. In particular, this'll solve the problem with Spec tables and Markdown.

[Elchi3]

I only read the code here, but haven't checked it out.

My understanding is, that the table assumes, that the spec data follows a similar shape of the data than we have for the tables we use already.

Can you attach a screenshot of a rendered table and the markup it uses under the hood?

Thanks for your review. The table looks almost the same as the current table. I haven't spend much time on making it render nicely yet, though.

The code can probably be improved as well. Happy to get into that if the overall approach seems promising. (or happy to hand this over to someone actually doing Yari engineering :).

@Elchi3 I haven't even started to review but I looooove the idea. In particular, this'll solve the problem with Spec tables and Markdown.

Great! I talked to Will about this as well and he reckons that it would reduce HTML tables by maybe 90% which is good because tables in GFM aren't very nice.

[peterbe]

The CI errors are unrelated and not your fault. They'll probably just pass next time you run.
For this to become ready for review, I think you should try to add a test that uses this. What I do to write tests is I start by setting:

CONTENT_ROOT=/Users/peterbe/dev/MOZILLA/MDN/yari/testing/content/files

in my .env, temporarily, then I yarn dev and start messing with the sample content in testing/content/files/en-us/... until it works great on localhost:3000. (PS. create a dedicated page just for this instead of cramming it into the en-us/web/foo/index.html file :)
Then when that's working you can write some tests in headless.test.js that navigate to the page.
I'm more than happy to help with anything regarding the end-to-end tests. If you feel like it's boring or slow, I can make a PR against your PR to tighten up the tests.

One last thing; I'm not in love with the naming. specs is a bit vague. The React component is called SpecificationTable but the macro is called specs. What do you think about calling the macro SpecificationTable? Whatever you can think of just to make it a bit more verbose and unambiguous.

[wbamberg]

Great! I talked to Will about this as well and he reckons that it would reduce HTML tables by maybe 90% which is good because tables in GFM aren't very nice.

I just did some counting and as near as I can tell:

• out of 922 pages in JavaScript
• we have 872 tables
• of which 773 are spec tables

So, yes, 90% wasn't a bad guess.

[Elchi3]

I've renamed the macro to SpecificationTable. Much nicer, thanks for the suggestion!

I've also added a KumaScript macro test and a functional test for the section extraction. Let me know if these are okay, or if more is needed.

If we are okay with the Kumascript and extraction side of things, I can continue to think about what we actually want to render. (It can't be just be the specURL, obviously, we probably want the name of the spec, too).

[peterbe]

If we are okay with the Kumascript and extraction side of things, I can continue to

As part of this PR? or something you'd like to do in a follow-up PR once this is merged?

I see your tests are passing but the PR is still a draft.

[Elchi3]

I guess this part could land without trouble? I mean the macro isn't ready yet to be called given the SpecificationTable renderer isn't ready yet but I can continue with that in a separate PR if that's preferable for review.

[peterbe]

but I can continue with that in a separate PR if that's preferable for review.

No please append it to this one. Because then we should be able to able to look at the final result in a page that has migrated and page that hasn't yet migrated. Right? Otherwise, we'd only be reviewing the code and not the final outcome.
Or am I misinterpreting you?

[peterbe]

Looks great so far!
I look forward to reviewing it more in-depth next week.

[Elchi3]

Updated this some more. It now uses the https://github.com/w3c/browser-specs package for the spec meta data. So far I'm only using the spec title but more from that package could be used to make the table more rich.

Rendering:

If BCD or the browser-specs package provide no data:

Edit: using more browser-specs data could look like this, for example:

I'm open for proposals, see https://github.com/w3c/browser-specs#spec-object for what sort of data is there.

[peterbe]

Drive-by comment (I'm in the doctors office waiting room)...

Can you please switch to const instead of let. let is only for variables that might change type.

Also, you're only testing if 'specifications' is non-falsy. What if it actually is an array but it contained 0 elements?

[foolip]

@Elchi3 you asked (in a meeting) about including links to wpt.fyi. That's a fascinating possibility, is there more discussion about that anywhere?

[Elchi3]

Thanks @foolip 👍

Yes, I learned this from Mike and Dom in a discussion in the #writing-docs channel on the Open Web Docs slack today.

"As far as the test link, browser-specs provides a testPaths array, so the wpt.fyi link(s) can be created by concatenating https://wpt.fyi/results/ + the testPaths value(s)."

"That won’t work for JavaScript tests and WebGL tests, but it will for that vast majority of cases. More generally, the rule is that it will work when tests.repository is set to the WPT URL."

[peterbe]

I started typing some comments to back up my point I wanted to "come with code" for my suggestions. So I typed it into your branch to test. One thing led to another now all my comments are available here: Elchi3#294 as a PR.
If you don't think they're crazy suggestions, you can merge that in and we can continue to test and play here in this PR.

[peterbe]

Disclaimer: I still don't know how Specifications work! I noticed, when playing with javascript.builtins.Array.toLocaleString that the Specifications table renders very differently from https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/toLocaleString#specifications where there's a second row in smaller font. 🤷🏻‍♂️

[Ryuno-Ki]

From a semantic point of view a table renders tabular data.
If you feel like it is used for layout here, perhaps another component pattern makes more sense.
From the screenshots, perhaps a card?

(Can you mark the PR as draft to avoid someone hitting merge by accident?)

[Elchi3]

We're still discussing what would be best to render in mdn/content#4482 but it became clear that it doesn't necessarily need to be a table, so I updated this PR to avoid the term SpecificationTable. It is now just called Specifications or SpecificationSection.

I also updated the browser-specs package to the latest version.

The rendering as it stands right now is a table and does almost the same as you can see on MDN now with the manual HTML tables. I would say we can review/merge this PR as is and see if the overall approach works on a few JS docs and then come back and improve the rendering in a second phase. Let me know if this works.

[peterbe]

I noticed you upgraded the npm browser-specs package within the PR. No need unless there are some hot new features you need for testing. Once this PR lands in main Dependabot will automatically upgrade browser-specs every week.

[peterbe]

The rendering as it stands right now is a table and does almost the same as you can see on MDN now with the manual HTML tables. I would say we can review/merge this PR as is and see if the overall approach works on a few JS docs and then come back and improve the rendering in a second phase. Let me know if this works.

Are you saying it's my turn to play now?

[Elchi3]

Are you saying it's my turn to play now?

Yes, I think we want to land this and iterate on the rendering more as needed. The current rendering tries to get close to what we currently render (in the javascript/ docs at least and that's where we want to test this).

[peterbe]

Perhaps it's my ignorance about the actual data and stuff, but the random page I picked was http://localhost:5000/en-US/docs/Web/SVG/Attribute/stdDeviation#specifications and it doesn't work if I change it. This is the change I made for testing:

diff --git a/files/en-us/web/svg/attribute/stddeviation/index.html b/files/en-us/web/svg/attribute/stddeviation/index.html
index 06e2a1230..270fe0274 100644
--- a/files/en-us/web/svg/attribute/stddeviation/index.html
+++ b/files/en-us/web/svg/attribute/stddeviation/index.html
@@ -1,6 +1,7 @@
 ---
 title: stdDeviation
 slug: Web/SVG/Attribute/stdDeviation
+browser-compat: svg.elements.feGaussianBlur.stdDeviation
 tags:
   - Filters
   - SVG
@@ -66,28 +67,8 @@ tags:
 
 <h2 id="Specifications">Specifications</h2>
 
-<table class="standard-table">
- <thead>
-  <tr>
-   <th scope="col">Specification</th>
-   <th scope="col">Status</th>
-   <th scope="col">Comment</th>
-  </tr>
- </thead>
- <tbody>
-  <tr>
-   <td>{{SpecName("Filters 1.0", "#element-attrdef-fegaussianblur-stddeviation", "stdDeviation")}}</td>
-   <td>{{Spec2("Filters 1.0")}}</td>
-   <td>No change</td>
-  </tr>
-  <tr>
-   <td>{{SpecName("SVG1.1", "filters.html#feGaussianBlurStdDeviationAttribute", "stdDeviation")}}</td>
-   <td>{{Spec2("SVG1.1")}}</td>
-   <td>Initial definition</td>
-  </tr>
- </tbody>
-</table>
+{{Specifications}}
 
 <h2 id="Browser_compatibility">Browser compatibility</h2>
 
-<p>{{Compat("svg.elements.feGaussianBlur.stdDeviation")}}</p>
+<p>{{Compat}}</p>

So how come it had specifications before? Clearly, svg.elements.feGaussianBlur.stdDeviation is a valid thing in BCD because that part works

Another highlight of my ignorance is; if you look at the hand-made table at https://developer.mozilla.org/en-US/docs/Web/SVG/Attribute/stdDeviation#specifications today, it has a "Status" column.
That seems like a loss to not have. But perhaps it's just something that I don't understand about the data and if you tell me to, I'm happy to ignore this topic. After all you did say the exact rendering of the <table> will be a follow-up.

[peterbe]

You need to change the makeTOC function. You'll find it in /build/index.js (around line 214). I think it's clear what you need to do.

If you add specifications to that if OR logic, you'll get "Specifications" showing up in the ToC again

[Elchi3]

Perhaps it's my ignorance about the actual data and stuff, but the random page I picked was http://localhost:5000/en-US/docs/Web/SVG/Attribute/stdDeviation#specifications and it doesn't work if I change it.

No, you're not ignorant at all :) Thanks for picking random pages! Here, BCD doesn't have a spec_url. So if we would change that page to use this new mechanism, it would "fail" with the note saying that it should be added to BCD. We have added spec_urls to about two thirds of the BCD data and on the content side we would only update pages where we are sure that the data is there. So the page in question wouldn't yet use the new macro but would stick with the old table (for now). We are working on getting spec_urls for everything, so it is a matter of time when everything will be renderable with this approach.

Another highlight of my ignorance is; if you look at the hand-made table at https://developer.mozilla.org/en-US/docs/Web/SVG/Attribute/stdDeviation#specifications today, it has a "Status" column.
That seems like a loss to not have. But perhaps it's just something that I don't understand about the data and if you tell me to, I'm happy to ignore this topic. After all you did say the exact rendering of the <table> will be a follow-up.

Right, this page is quite old and we said for quite some time now that "Status" and "comment" aren't very useful (and outdated in many cases), so we wanted to remove them but didn't do so throughout all docs (the JS docs don't have this anymore). If we want to bring back "Status" everywhere, the good thing in the future is that we can implement it here in Yari and have it everywhere consistently.

Also of interest content-wise: oftentimes it will not be the same spec links if you compare what is on MDN now vs what this brings in. This is because we actually thought through what are relevant and up to date specs for web developers. Currently, MDN displays all sorts of specs and many outdated ones.

So, overall: This isn't the exact them thing as before but the idea is that it will be a lot better. As far as rolling this out on actual content, I would be quite conservative at the start and for now only use this with docs under https://developer.mozilla.org/en-US/docs/Web/JavaScript/ where we are sure nothing will fall apart too badly. If that then worked out okay, we can add it to more pages.

[peterbe]

This algorithm is quite different from how we treat things in _buildSpecialBCDSection on line 354-359.
In fact, I ran a measurement of ALL BCD queries possible and calculated how many times you get some specURLs if you just look in in compat.spec_url vs. possibly digging deeper into compat.__compat.spec_url:

{ hasSpecURLs: 3067, hasSpecURLs2: 3103, hasBCD: 8085, hasNotBCD: 262 }

(where hasSpecURLs2 is an alternative implementation that checks if compat.__compat is the object to look inside)

[peterbe]

What's confusing is that sometimes there's something in compat.spec_url and sometimes it's in compat.__compat.spec_url and but here's a breakdown of all the BCD queries where the number of found specURLss is different: https://gist.github.com/peterbe/f6e132913cf4cc185cd9aa1b6a5460bc depending on how you look.

PS. I can't even remember why the BCD table does it like this. Sorry. Just seems a bit weird that the algorithm is different for .support vs. .spec_url.

[Elchi3]

Great catch! I believe this only matters when BCD grouped things under a tree and that tree itself has no __compat (which is very rare).

The usual case (at least looking at your gist, in almost all of the cases, it is okay to not bother about what is below a tree, because the tree we've pointed at, e.g. api.AbortController has api.AbortController.__compat and that's exactly what we need for our specification link (BCD tables want more, they want all the sub features, too).

The edge case this deals with, though, is this: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Lexical_grammar with the BCD query: javascript.grammar which itself has no javascript.grammar.__compat. The BCD table will find something anyways though, as it looks into javascript.grammar.whatever.__compat and generate a table with all the features grouped as "grammar".
I need to think about what we want here in terms of specs. In theory all these features have spec urls but that would obviously make a table as long as the compat table. Maybe this is actually a sign that the BCD and content organization is nuts and we should rather have separate pages for these things. Oh well :)

[Elchi3]

So, I played with this and it is by design that I didn't go into the compat.__compat.spec_urls. As said in the earlier comment, unlike the BCD tables, we don't want to list all specs for all the sub features. So not this:

But rather just the main features' spec:

[Elchi3]

And on the grammar page, we will need to see. Probably also not that useful to list specs for all the features that that compat table has.

[peterbe]

For what it's worth I did check every single BCD query where there is a .spec_url and for each one, I checked that the URL itself always has a #. I also checked that there is always a non-zero-length title and a non-zero-length shortTitle. They are all OK and worked.

[Elchi3]

I checked that the URL itself always has a #.

Yup, the BCD linter fails if there is no anchored link. spec_urls should point to the exact definition of a feature so we made sure this is provided over in BCD.

[peterbe]

I think we should address this comment. XXX comments are not supposed to go into main. They're useful for drafts and stuff.
What would be useful is to state, in a code comment, that we're aware that it could be an array of length 0 and that that should be treated as if the specifications can't be found at all.

[Elchi3]

I'm happy to remove this comment (I think you added it). I'm not quite sure if there is a third condition to be described here. I mean, either we collected specifications using BCD and browser-specs, or that failed and so we show the warning. But maybe I'm not realizing what you meant by this comment exactly. Can you elaborate or should I just remove this comment?

[Elchi3]

I removed the comment

[peterbe]

With nits but totally ready to try.
I love that we can land with this without worrying because it only matters when we start testing it in some content. And, god forbid, we discover a bug in the rendering or something we can just wait with those content updates.

Let's do this!