Stop exposing mixins; instead add them to their target interfaces

[Elchi3]

It seems like there is largely consensus to hide mixins to the world of web developers for reasons discussed in #472 for BCD and in mdn/content#1940 for MDN docs.

@rachelandrew notes an authoring drawback when hiding mixins and documenting things directly on the interfaces, though:

So ... if I am adding ARIAMixin, which was AriaAttributes to Element, for BCD does that mean adding all of these to the 9000+ line long Element BCD file?

I think this is a good point. So, how can we remove mixins and while keeping some authoring conveniences (i.e. avoiding large BCD files)? Luckily (and ironically) BCD supports mixing in data to a particular tree! We just haven't made a lot of use of that.

Right now we have api/Element.json and it is exposing its content as "api.Element.*".
We also added files like api/AriaMixin.json and exposed content as "api.AriaMixin.*". This is not hiding the mixin but exposes it directly :(

However, to hide mixins, we could have a file AriaMixin-for-Element.json and expose its content as "api.Element.*" (we mix in data to Element, ha!)
For that to happen, the AriaMixin-for-Element.json file needs to start like this:

{
  "api": {
    "Element": {
      "ariaRole": {
        "__compat": {
          "mdn_url": "https://developer.mozilla.org/docs/Web/API/Element/animate",
          "support": {
            "chrome": {

Note the "Element" after "api". This will extend what is already defined in api/Element.json.

To keep things clean, we can also move AriaMixin-for-Element.json to api/mixins/AriaMixin-for-Element.json, so that we know files in this folder extend other interfaces in the main api/ folder.

If in the year 2525, AriaMixin is exposed to CyberElement, we have a new compat story, and so we should have api/mixins/AriaMixin-for-CyberElement.json and it will add data to the exposed API CyberElement.

Thoughts?

[ddbeck]

Thank you for writing this up! I'm excited to begin making mixins more manageable. Overall, I think this is a good direction to go.

Some initial thoughts and questions, which are mostly minor:

1. Is there a reason not to name the mixin files themselves after their names in the spec. For example, based on this proposal, I'd imagine that something like WindowOrWorkerGlobalScope would be represented in /api/_mixins/WindowOrWorkerGlobalScopeMixin-for-Window.json. Perhaps we could use the folder structure alone to indicate that it's a mixin? For example: /api/_mixins/WindowOrWorkerGlobalScope__Window.json. (Secret 2a question I'm afraid to ask: does ending the exposure of mixins mean we need to reformulate the api namespace for the main thread and worker global scopes)?

2. Suppose /api/_mixins/AriaMixin.json contains api.Element.ariaRole and /api/Element.json also contains api.Element.ariaRole. Which one wins? Is it desirable to have the ability to enter into both places? Do you anticipate that we would need to build some linting to protect against unintentionally clobbering data?

3. Can we park the mixin files in a folder such as /api/_mixins/ (or some other character/convention) to show that the folder structure is internal to BCD only?

4. I'd expect, before the first introduction of a new-style mixin, a PR adding a data guideline showing how to do this. Critically, I think we should have some sort of explicit criteria for when existing data may be de-mixinified. My preference is probably for a scripted migration, but if that's not practical, I don't want a free for all when it comes to restructuring the data.

[Elchi3]

Thanks for the feedback, Daniel!

1.: Yes, /api/_mixins/WindowOrWorkerGlobalScope__Window.json seems good to me. Note that AriaMixin is the actual name per spec (sorry for choosing a confusing example).

2a: main thread and worker global scopes: no matter how we treat mixins, real interfaces will be exposed to different contexts and so that remains (in theory, will investigate more).

2.: Yes, my current thinking is that BCD should disallow defining entries twice.

3.: Yes

4.: Yes, totally needs a data guideline. I'm opening a proof of concept PR that re-organizes a mixin. From there we can hash this out further and validate/invalidate the overall approach I'm proposing in this issue.

[jpmedley]

I support the general approach, but defer to you all regarding the implementation.

[jpmedley]

It's so simple, I'm wondering why no one has thought of it before.

[Elchi3]

@foolip Can you comment if this approach would work with the tooling you maintain?

[foolip]

I strongly support this flattening. Whether the entries are in api/Element.json or in a file like api/mixins/AriaMixin-for-Element.json doesn't matter for mdn-bcd-collector as long as the object paths are api.Element.*. (Supporting filenames that can't be simply inferred from the API names is already necessary for javascript/builtins/intl/* and similar.)

However, @ddbeck's secret question is spot on, there is a complication we have to consider:

does ending the exposure of mixins mean we need to reformulate the api namespace for the main thread and worker global scopes

We could naively flatten these mixins and end up with api.Window.fetch + api.WorkerGlobalScope.fetch instead of api.WindowOrWorkerGlobalScope.fetch. However, I think we should treat these mixins as different from all the rest.

It's an accident of how Web IDL works that the exposure set of interfaces like Request is defined using [Exposed=(Window,Worker)] while the exposure set of methods like fetch is defined using partial interface mixin WindowOrWorkerGlobalScope. In MDN documentation and in BCD, I think we should find a common way to represent the worker exposure. Many different approaches could work, but my first suggestion would be api.WindowOrWorkerGlobalScope.fetch → api.fetch and working through the many consequences of that.

However, I think it would be smart to set WindowOrWorkerGlobalScope aside in the mixin flattening, and sort it out in a later step, because the issues are very different.

[Elchi3]

Daniel:

2. Suppose /api/_mixins/AriaMixin.json contains api.Element.ariaRole and /api/Element.json also contains api.Element.ariaRole. Which one wins? Is it desirable to have the ability to enter into both places? Do you anticipate that we would need to build some linting to protect against unintentionally clobbering data?

Florian:

2.: Yes, my current thinking is that BCD should disallow defining entries twice.

So, this is filed in #4200 (we don't just need to protect ourselves against duplicates with mixins but we should protect against duplicates generally).

[Elchi3]

The consistency checker probably needs to get better, too: #9044

[ddbeck]

@Elchi3 for the PRs that actually demix interfaces, do you have a sense of how you want to be reviewed, with regard to discrepancies or validity? For example, for #9048 should I care that the versions in api.Text.assignedSot aren't equivalent to api.Slottable.assignedSlot? Should we be trying to verify the demixed data at all?

[Elchi3]

Yeah, good point. I've asked Philip to assist with bcd-collector automation to get to better data, but I've been focusing on re-organizing the data instead of doing the research. Verifying all remixed data is quite a bit of work, I'm afraid.

should I care that the versions in api.Text.assignedSot aren't equivalent to api.Slottable.assignedSlot?

fwiw, often it is bug in the current data that these are the same. Many times things are exposed earlier to old school interfaces like Document and a lot later on interfaces like ShadowRoot, for example. Mixin data then does something and it is often wrong, so I think this isn't something to care about in all situations.

[ddbeck]

focusing on re-organizing the data instead of doing the research

Thanks! This sounds good and that's how I'm going to review these now.

[foolip]

Here's a burndown list of interface mixins in BCD:

• AbstractWorker (Demix AbstractWorker↠ Worker/SharedWorker/ServiceWorker.onerror #10744)
• Body (Demix Body to Request and Response #10958)
• ChildNode (Demix ChildNode #10506)
• DocumentOrShadowRoot (Demix DocumentOrShadowRoot #9045)
• GeometryUtils (Demix GeometryUtils to Document/Element/Text.getBoxQuads #10721)
• GlobalEventHandlers
• HTMLHyperlinkElementUtils (Flatten HTMLHyperlinkElementUtils mixin to HTMLAnchorElement/HTMLAreaElement #8933)
• LinkStyle (Demix LinkStyle mixin #10724)
• NavigatorConcurrentHardware (Demix Navigator* mixins into Navigator and WorkerNavigator #11413)
• NavigatorID (Demix Navigator* mixins into Navigator and WorkerNavigator #11413)
• NavigatorLanguage (Demix Navigator* mixins into Navigator and WorkerNavigator #11413)
• NavigatorOnLine (Demix Navigator* mixins into Navigator and WorkerNavigator #11413)
• NavigatorPlugins (Demix Navigator* mixins into Navigator and WorkerNavigator #11413)
• NavigatorStorage (Demix Navigator* mixins into Navigator and WorkerNavigator #11413)
• NonDocumentTypeChildNode (Demix NonDocumentTypeChildNode #9065)
• ParentNode (Remix ParentNode #9064)
• SVGAnimatedPoints (Demix SVGAnimatedPoints to SVGPolygonElement and SVGPolylineElement #10698)
• SVGFilterPrimitiveStandardAttributes (Demix SVGFilterPrimitiveStandardAttributes #10646)
• SVGTests (Demix SVGTests to SVGAnimationElement and SVGGraphicsElement #10691)
• SVGURIReference (Remove api.SVGURIReference entry #10522)
• WindowEventHandlers
• WindowOrWorkerGlobalScope

[ddbeck]

(Edited @foolip's burndown list to add links to open PRs. I hope that's OK!)

[Elchi3]

Thanks for helping with this effort, @foolip. I appreciate it!
I'm trying to burn through it all here and on mdn/content.

[foolip]

Thanks @Elchi3! I don't plan to send any more PRs since you're already working on it, but I'd be happy to review them.

[foolip]

With #10958 out, everything that now remains requires deciding how to deal with worker exposure.

[Elchi3]

That's great news! Do we have a separate issue for that already?

[foolip]

No, it's just all the unchecked boxes (except Body) in #8929 (comment)

[foolip]

Well actually, WindowEventHandlers can be demixed in Window + HTMLBodyElement + HTMLFrameSetElement without worrying about workers.

[foolip]

But that instead runs into the events mess. I really can't claim it's going to be an improvement to have 3 entries for onfoo attributes, plus various foo_event attributes spread around in an ad-hoc manner, and some onfoo content attributes. How can anyone make sense of that to answer "can I use this event and where/how do I listen for it?"

[Elchi3]

I think it would help to start new issues and spell out the concrete remaining data points and options we have for them.
(At least that would help me to think about solutions.)

[foolip]

Existing issues cover everything that remains:

• Representing support of events #7545 for GlobalEventHandlers and WindowEventHandlers
• Representing WorkerLocation and WorkerNavigator similarly to other worker-exposed interfaces #9125 for Navigator* mixins
• Get rid of WindowOrWorkerGlobalScope without splitting the entries #9127 for WindowOrWorkerGlobalScope

[Elchi3]

This is all done except for GlobalEventHandlers and
WindowEventHandlers which I'm now tracking in #12290

[foolip]

Amazing, I never thought we'd (almost) get rid of mixins :)