Skip to content
This repository has been archived by the owner on Sep 7, 2021. It is now read-only.

U - data.browser_compatibility ingredient doesn't allow multiple BCD tables #405

Closed
Elchi3 opened this issue Apr 28, 2020 · 10 comments
Closed
Labels
js-linting All issues that concern linting of JS docs

Comments

@Elchi3
Copy link
Member

Elchi3 commented Apr 28, 2020

This compat ingredient (https://github.com/mdn/stumptown-content/blob/master/scripts/scraper-ng/rules/html-require-recipe-ingredients/ingredient-handlers/data-browser-compatibility.js) doesn't allow multiple tables. These occur two times in JS docs and affects the current goal of fixing all linter errors. This also happens in CSS docs sometimes.

I assume we should update the ingredient handler for this.

Acceptance criteria

The linter spec is updated to describe how to represent multiple compat tables, and the linter is updated accordingly.

@Elchi3 Elchi3 added the js-linting All issues that concern linting of JS docs label Apr 30, 2020
@Elchi3
Copy link
Member Author

Elchi3 commented Apr 30, 2020

Fixing this will affect our linting error count to go down by 2 (so, not a priority but needed for the finish line).


1 error javascript-language-feature/data.browser_compatibility/expected-macro
1 error javascript-method/data.browser_compatibility/expected-macro

@chinikes chinikes added this to the Golf Hotel (S2 Q2 2020) milestone May 6, 2020
@wbamberg wbamberg changed the title data.browser_compatibility ingredient doesn't allow multiple BCD tables U - data.browser_compatibility ingredient doesn't allow multiple BCD tables May 7, 2020
@wbamberg
Copy link

wbamberg commented May 7, 2020

Of course it's easy to enable to linter to accept multiple calls to {{Compat}}.

I wonder what having multiple {{Compat}} calls means for the semantics of structured content. At the moment we say: an item has a data.browser_compatibility item, which resolves to a single blob of JSON that tells the compat story for that item. So if, for instance, your application is a devtool that wants to fetch "the compat data" for a CSS property, and display some warning if the item isn't supported in certain browsers. Then if there's just one BCD query, there's (sort of) one answer. But if there are two, how should you interpret that?

Probably it's premature to worry about that. Maybe this is enough of an edge case anyway that a tool could treat this as a special case.

At least though I think the linter should require that if there's more than one {{Compat}}, then each one has an H3 heading that provides some context for that table (as is our usual practice here).

@Elchi3
Copy link
Member Author

Elchi3 commented May 8, 2020

At least though I think the linter should require that if there's more than one {{Compat}}, then each one has an H3 heading that provides some context for that table (as is our usual practice here).

Yes, I think we should require h3 demarcation.

But if there are two, how should you interpret that?

Does that hint that we should have separate MDN pages? https://developer.mozilla.org/en-US/docs/Web/CSS/gap#Browser_compatibility is a prime example for multiple compat tables, I think.

I guess you could say that gap should be presented on 3 different pages depending on the context given it has different compat data, different code examples, use cases, etc, but I don't know how practical that would be.

@wbamberg
Copy link

wbamberg commented May 8, 2020

I guess you could say that gap should be presented on 3 different pages depending on the context given it has different compat data, different code examples, use cases, etc, but I don't know how practical that would be.

I don't think we should split gap across three pages. I think people understand it as one property (one name, one specification, one set of values) that can be used in three different contexts, and it would be confusing to treat it as three properties.

To make it silly, we wouldn't consider having different pages for https://developer.mozilla.org/en-US/docs/Web/CSS/color_value because we use it in many different contexts, and might have different examples demonstrating them.

Also this wouldn't help with the hypothetical devtools example above, would it? I think the problem there is basically one of mapping a CSS property name onto a __compat object. Usually this is straightforward, but occasionally, as here, it isn't - gap doesn't have a __compat object directly underneath it. But what it does have underneath is objects with a helpful description, e.g. "Supported in Flex Layout". So if it were me trying to write a tool here, I might special-case this by saying something like: if there's no __compat object directly under the property name, find all the __compat objects further down, and present them all, using the description to label them to give the user context.

In fact I wonder: if we want to require H3 titles when a page contains more than one compat table, should that title text be taken from the BCD "description"? And if so, should we require that if a stumptown item contains more than one BCD query, that every one must have a description?

But I do think that there's a good case for saying that Class fields should be split into two pages: "Public class fields" and "Private class fields". You just have to look at the table of contents to see that it's describing two quite different things:

## Public fields
## Public methods
## Private fields
## Private Methods

String.match() feels like yet another case :). It seems like there's a "primary" table that you'll always care about , whatever you're doing with match(), and a kind of subsidiary table that's giving you more information about a particular aspect of it. In this case javascript.builtins.String.match does not have a "description" that we could use as an H3 title (or a contextual note in some other application).

I would be tempted, in this case, to omit the "named capture groups" table, and instead just rely on a link to the relevant page from the documentation for groups in this page. I can see how it is helpful to display the table directly here. But I'm struggling to think of other cases where we take this approach. For instance, in CSS we have width which can take a value of calc(). calc() is newer than width, so it's possible you could be able to use width but not width: calc() - but we don't embed the calc() table in the width page. This seems quite analogous to String.match() - why do we do it there and not here?

Or perhaps we should stop worrying about all that for now, and just update the linter to allow multiple tables, with mandatory H3 headings :).

@ddbeck
Copy link
Contributor

ddbeck commented May 14, 2020

I've got some thoughts on multiple compat tables. I don't like them and I wish I had resisted allowing the CSS "contexts" pattern in the first place. The other cases seem easier to fix, because it's not so much a problem of the underlying data.

Take gap for example. The page's multiple compat tables paper over problems in the page and in the compat data.

On the one hand, there's a real content problem in that the gap page doesn't talk about the different modes that gap can operate in until you reach the compat tables. Honestly, I still don't fully understand these contexts. The extra compat tables are pointless without some information about those contexts.

On the other hand, the structure in BCD is weird. It's nearly one-off. Apart these CSS properties, I can't think of anywhere else we do anything like this. It's as if we've hidden a subfolder in /css/properties/. It was a source of considerable confusion when this structure first landed; take a look at mdn/browser-compat-data#2516, which I kinda regret merging. I think I had the right idea at the time, that there should've been one and only one basic support feature above any "context" features.

Instead, I would like to see the data get fixed and the tables reduced to one: there should be one basic support feature ("Does this browser recognize this property name?") and subfeatures for different contexts ("Does this browser do something like the spec'ed behavior for display: whatever?").

Regarding this case:

I would be tempted, in this case, to omit the "named capture groups" table, and instead just rely on a link to the relevant page from the documentation for groups in this page. I can see how it is helpful to display the table directly here. But I'm struggling to think of other cases where we take this approach.

I agree with this. I like the idea of linking to another page and being done with it (we have a structure for this—See also—after all). But if we want to be a bit more permissive, I could see allowing additional compat tables in other sections besides "Browser compatibility". But in that section, there should be one and only feature represented.

@wbamberg
Copy link

Instead, I would like to see the data get fixed and the tables reduced to one: there should be one basic support feature ("Does this browser recognize this property name?") and subfeatures for different contexts ("Does this browser do something like the spec'ed behavior for display: whatever?").

I feel bad since this was apparently my suggestion back then. I'd be happy to have a "basic support" for gap. But...

I think the thing that led us into this was that there's no practical sense talking about "support for gap" without knowing which layout you're talking about. So having basic support could allow data like:

gap:
  basic support: yes
  support in grid: no
  support in flex: no
  support in multicol: no

...and that doesn't seem sensible. And as a practical guide to readers "basic support" in the case of gap seems completely unhelpful, because they're always interested in support in a particular layout context.

mdn/browser-compat-data#2516 (comment) says '"Basic support" in other CSS data has meant something along the lines of "the browser recognizes this property or value and includes it in the cascade."' which I suppose leaves room for data like my example above. But that definition of basic support isn't documented anywhere that I can see. The schema doc defines basic support as "indicating that a minimal implementation of a functionality is included", which, well, I'm not sure how to interpret.

It seems like this is leading us towards a rule where every CSS property has a __compat object directly under it. Is that right? This seems like a change in BCD policy, which in general has been loose about which things have __compat objects and which are just containers of subfeatures. Do we want to have similar rules for other domains? Do we want some way to make this explicit in the data, so we know when a thing needs __compat and when it doesn't?

@Elchi3
Copy link
Member Author

Elchi3 commented May 15, 2020

The schema doc defines basic support as "indicating that a minimal implementation of a functionality is included", which, well, I'm not sure how to interpret.

The schema docs also say:

What it represents exactly depends of the evolution of the feature over time, both in terms of specifications and of browser support.

So, I think, you could say that the minimal implementation is the initial implementation in the flex context (I guess) and that there are then sub features that represent evolutions of this feature, namely its availability in other contexts. I don't know if this makes it more practical, but I think this is the way we've seen sub features in other domains.

It seems like this is leading us towards a rule where every CSS property has a __compat object directly under it. Is that right? This seems like a change in BCD policy, which in general has been loose about which things have __compat objects and which are just containers of subfeatures. Do we want to have similar rules for other domains? Do we want some way to make this explicit in the data, so we know when a thing needs __compat and when it doesn't?

I think it would be great actually to be more consistent about this. BCD consumers have been bitten by this before and iirc had to special case this in their code.

On the cases class fields and String.match:
I agree the class fields page should be split, I will do that. I also agree we don't need to have "named capture groups" table for String.match. So, I think, for JS docs, we can resolve this issue in content, but for CSS the discussion above is worth to resolve.

@ddbeck
Copy link
Contributor

ddbeck commented May 18, 2020

I opened mdn/browser-compat-data#6175 to discuss the feature structure issue in BCD. Maybe we can get that in the next sprint?

@Elchi3
Copy link
Member Author

Elchi3 commented May 19, 2020

I've removed the second table from String.match and I will split the class fields page soon.

Thanks for filing that issue, Daniel! I think we can come back to this topic when we fix CSS linting issues, which will probably in the next sprint or the sprint after.

@Elchi3
Copy link
Member Author

Elchi3 commented May 27, 2020

I've split the class fields page into two pages and so now there is only one compat table per BCD section. I will close this issue, and when we're dealing with CSS docs, we should file a new issue for dealing with these cases in CSS.

@Elchi3 Elchi3 closed this as completed May 27, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
js-linting All issues that concern linting of JS docs
Projects
None yet
Development

No branches or pull requests

4 participants