Closed Bug 1414372 Opened 7 years ago Closed 5 years ago

Add support for WebIDL mixins

Categories

(Core :: DOM: Bindings (WebIDL), enhancement, P3)

enhancement

Tracking

()

RESOLVED FIXED
mozilla70
Tracking Status
firefox70 --- fixed

People

(Reporter: tobie.langel, Assigned: saschanaz)

References

(Blocks 1 open bug)

Details

(Keywords: dev-doc-complete)

Attachments

(1 file)

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_10_5) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/61.0.3163.100 Safari/537.36

Steps to reproduce:

Hi all!

WebIDL recently introduced dedicated syntax for mixins[0].

This syntax replaces the [NoInterfaceObject] extended attribute and "implements statement" which have been deprecated (except for a few legacy uses cases explicitly mentioned in the spec[1]).

You can read more about it in the spec[2].

In most cases, the changes should be relatively straightforward. The now deprecated:

    interface Foo { };

    [NoInterfaceObject]  // (Mostly) DEPRECATED
    interface Bar { };
    Foo implementes Bar; // DEPRECATED


should just be rewritten as:

    interface Foo { };
    
    interface mixin Bar { };
    Foo includes Bar;

Please feel free to reach out if you have any questions.

Thanks!

[0]: https://github.com/heycam/webidl/commit/45e8173d40ddff8dcf81697326e094bcf8b92920
[1]: https://heycam.github.io/webidl/#NoInterfaceObject
[2]: https://heycam.github.io/webidl/#idl-interface-mixins

This is tracked in: https://github.com/heycam/webidl/issues/472
The original pull-request: https://github.com/heycam/webidl/pull/433
Tobie, were there semantic changes too?

For example, it sure looks to me like MixinMember doesn't allow everything allowed in InterfaceMember, right?

What (if anything) replaced the concept of "consequential interfaces"?  It looks like it's not possible for a mixin to include another mixin, right?  Instead, all the interfaces including the first mixin would have to manually include the second one?
Flags: needinfo?(tobie.langel)
> Tobie, were there semantic changes too?

Yes. Mixins are much simpler (and more restricted) than what we previously had.

> For example, it sure looks to me like MixinMember doesn't allow everything allowed in InterfaceMember, right?

Yes. I took the Gecko code code and ran the following commands on it:

https://gist.github.com/tobie/bb83f160e93a084afdfb74ae29fddf2a

This allowed identifying what was really needed in practice:

https://github.com/heycam/webidl/issues/363#issuecomment-325821479

> What (if anything) replaced the concept of "consequential interfaces"?

Nothing. I couldn't find examples where that was actually needed.

> It looks like it's not possible for a mixin to include another mixin, right?  Instead, all the interfaces including the first mixin would have to manually include the second one?

That's correct. Alternatively a partial mixin could be used instead.

In practice I didn't find mixins that were implementing other mixins.
Flags: needinfo?(tobie.langel)
The key difference between a partial mixin and a mixin pulling in a mixin is this.  Say we have three specs: A, B, C.  Spec A defines mixin 1.  Spec B wants to define mixin 2 that includes all of mixin 1 but adds some things; it does not want to extend mixin 1 because it doesn't want to affect all the consumers of mixin 1.  Spec C wants to use the mixin spec B defines.  With what's in the IDL spec right now, C has to manually pull in both B's mixin and A's mixin instead of just pulling in B's mixin.  In particular, that makes it harder to factor part of B's mixin out of B into a new spec (A).

> In practice I didn't find mixins that were implementing other mixins.

We have two standard-ish ones in the Gecko tree, based on debug print statements I just added to our binding generator: XPathEvaluator and WebGL2RenderingContextBase.

XPathEvaluator doesn't really have a spec yet (and is an implemented interface that is NOT [NoInterfaceObject] to boot; it can probably be factored out into a separate mixin pulled in by both that interface and Document).  WebGL2RenderingContextBase comes from https://www.khronos.org/registry/webgl/specs/latest/2.0/ and is very much a mixin implementing a mixin:

  WebGL2RenderingContext implements WebGL2RenderingContextBase;
  WebGL2RenderingContextBase implements WebGLRenderingContextBase;

We also have a non-standard "BrowserElement" mixin which implements two other mixins (BrowserElementCommon/BrowserElementPrivileged), but we could probably get rid of that indirection, since nothing else implements them.
Can't the WebGL example be expressed just as well as:

    interface mixin WebGL2RenderingContextBase {
      /* ... */
    };
    
    interface WebGL2RenderingContext { };
    WebGL2RenderingContext includes WebGLRenderingContextBase;
    WebGL2RenderingContext includes WebGL2RenderingContextBase;
I did ask them about this at the time we implemented webgl2, fwiw.  They are doing this very purposefully, so when they introduce a WebGL3RenderingContext they don't have to explicitly write down that it implements all the preceding "base" interfaces; they can just say that it implements WebGL2RenderingContextBase and that picks up WebGLRenderingContextBase automatically.

In any case, the upshot is that if webidl doesn't support this then we need to get khronos to change how they're doing things.

In practice, I suspect allowing mixins to include other mixins is simpler than trying to get people to not do it.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Priority: -- → P3
Component: DOM → DOM: Bindings (WebIDL)
We need to make sure this change is reflected in our docs. Mostly this will just involve updating https://developer.mozilla.org/en-US/docs/MDN/Contribute/Howto/Write_an_API_reference/Information_contained_in_a_WebIDL_file
Keywords: dev-doc-needed
Flags: needinfo?(bzbarsky)

OK, so in terms of what needs to be done here (more or less broken down by changeset if I were doing this):

  1. Decide whether mixins should basically use the same data structure as IDLInterfaceOrNamespace or something simpler. I suspect something simpler would be better, because we don't actually want most of the relevant work of IDLInterfaceOrNamespace.finish to happen on mixins: we just want to pull their members into the things including them and do all the validation and whatnot after that. I also suspect interface and namespace mixins can use basically the same data structure with just a namespace-or-interface boolean flag to make sure people don't include an interface mixin in a namespace or vice versa. Plus maybe whatever make-it-static-my-default behavior namespace mixins need. But it's worth looking through the spec to see what operations need to be supported on mixins and then deciding how to refactor the class hierarchy based on that. Implement the actual data structure for mixins and whatever work partial mixins will need.
  2. Implement the behavior of the includes directive, in terms of copying members from a mixin to the including interface or namespace and whatever other work or validation has to happen in the process.
  3. Modify the grammar to allow mixins (and start creating the objects from step 1 as needed) but not remove implements yet.
  4. Switch over all in-tree uses of implements to use includes with mixins. This might be multiple changesets, especially if some of the changes are nontrivial (e.g. the webgl ones, because the webgl spec has invalid IDL now that it's using mixins...)
  5. Remove implements from the grammar.
  6. Remove the handling of implements in terms of copying things over to the implementing interface.
  7. Various simplifications, possibly in multiple changsets, to remove concepts like consequential interfaces, etc, etc.
Flags: needinfo?(bzbarsky)

I also suspect interface and namespace mixins can use basically the same data structure with just a namespace-or-interface boolean flag to make sure people don't include an interface mixin in a namespace or vice versa.

Do we have namespace mixins? I skimmed the code and it seems IDLImplementsStatement throws if it's used for any non-interfaces, and AFAIK the spec also does not define mixins for namespace.

Flags: needinfo?(bzbarsky)

Do we have namespace mixins?

Looks like no. At some point there was discussion about having them in the spec, but I guess that got deferred or nixed.

Flags: needinfo?(bzbarsky)
Assignee: nobody → saschanaz

Add IDLInterfaceMixin with a new superclass shared with existing IDLInterfaceOrNamespace.

I filed bug 1574195 and bug 1574201 as followups to this.

Pushed by bzbarsky@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/1b097ae05490
Introduce interface mixins r=bzbarsky
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla70
Blocks: 1579457
Blocks: 1577660
Type: defect → enhancement

Marking this as dev-doc-complete; I already make the appropriate changes to https://developer.mozilla.org/en-US/docs/MDN/Contribute/Howto/Write_an_API_reference/Information_contained_in_a_WebIDL_file#Mixins after :bzbarsky announced it on dev-platform.

I'm not going to add this to the Fx70 rel notes, as it is not really interesting to web devs.

You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: