- From: Domenic Denicola <notifications@github.com>
- Date: Fri, 29 Sep 2017 21:02:42 +0000 (UTC)
- To: heycam/webidl <webidl@noreply.github.com>
- Cc: Subscribed <subscribed@noreply.github.com>
- Message-ID: <heycam/webidl/pull/433/review/66264024@github.com>
domenic commented on this pull request.
I know you said not quite ready for review, but I couldn't resist :). This is looking so good!! The model is so much simpler and easier to understand this way.
Other things I noticed:
- [Unforgeable] contains a "legacy mixin" example still
- Some places that say "actually a mixin" (Ctrl+F for that) should be updated
> used to define that objects implementing an interface will always
-also implement another interface.
+also implement a [=mixin=]
Maybe we should avoid the verb "implement" for mixins, in favor of e.g. "include the members defined by that mixin" or similar?
> @@ -1130,6 +1156,392 @@ The following extended attributes are applicable to interfaces:
</div>
+<h3 id="idl-mixins">Mixins</h3>
+
+A <dfn export>mixin</dfn> is a definition (matching <emu-nt><a href="#prod-Mixin">Mixin</a></emu-nt>)
+that declares state and behavior that can be [=included=] by one or more [=interfaces=],
+and that are exposed by objects that implement an [=interface=] that [=includes=] the [=mixin=].
+
+<pre class="webidl" class="syntax">
+ mixin identifier {
+ /* mixin_members... */
+ };
+</pre>
+
+Note: Mixins, much like [=partial interfaces=], are intended for use as a specification editorial aide,
+allowing a coherent set of functionalities to be grouped together,
+and included in multiple interfaces, possibly across documents.
We should forward-reference your note below about the difference between them.
> +and that are exposed by objects that implement an [=interface=] that [=includes=] the [=mixin=].
+
+<pre class="webidl" class="syntax">
+ mixin identifier {
+ /* mixin_members... */
+ };
+</pre>
+
+Note: Mixins, much like [=partial interfaces=], are intended for use as a specification editorial aide,
+allowing a coherent set of functionalities to be grouped together,
+and included in multiple interfaces, possibly across documents.
+
+A [=mixin=] is a specification of a set of <dfn export lt="mixin member">mixin members</dfn>
+(matching <emu-nt><a href="#prod-MixinMembers">MixinMembers</a></emu-nt>),
+which are the [=constants=], [=regular operations=], [=regular attributes=],
+and an eventual [=stringifier=],
What's "eventual" mean here?
> + };
+
+ partial mixin <mark>SomeMixin</mark> {
+ /* mixin_members... */
+ };
+</pre>
+
+The order that members appear in has significance for property enumeration
+in the <a href="#es-namespaces">ECMAScript binding</a>.
+
+Note that unlike [=interfaces=] or [=dictionaries=], [=mixins=] do not create types.
+
+Of the extended attributes defined in this specification,
+only the [{{Exposed}}] and [{{SecureContext}}] extended attributes are applicable to mixins.
+
+An <dfn>includes statement</dfn> is a definition
I don't think it's a definition. Maybe a statement is what is meant here? Although then you're just saying "statement" three times in the same sentence :)
> +
+Each [=interface=] |I| that [=includes=] a [=mixin=] |M| must receive
+a copy of each of the [=mixin members|members=] of |M|.
+Each [=mixin member=] cooy is treated as if it had been declared on |I|.
+Its <dfn>host interface</dfn> is |I|.
+
+Note: In ECMAScript, this implies that each [=regular operation=]
+declared as a [=mixin member|member=] of [=mixin=] |M|,
+and exposed as a data property with a [=built-in function object=] value,
+is a distinct [=built-in function object=]
+in each [=interface prototype object=]
+whose associated [=interface=] [=includes=] |M|.
+Similarly, for [=attributes=], each copy of the accessor property has
+distinct [=built-in function objects=] for its getters and setters.
+
+The order of appearance of an [=includes statement=] does not matter.
Not sure exactly what this means, but at least one interpretation is false, since A includes B; A includes C; is different from A includes C; A includes B; in terms of property enumeration order. (Kinda. It's all a bit fuzzy especially cross-specification.)
> +a copy of each of the [=mixin members|members=] of |M|.
+Each [=mixin member=] cooy is treated as if it had been declared on |I|.
+Its <dfn>host interface</dfn> is |I|.
+
+Note: In ECMAScript, this implies that each [=regular operation=]
+declared as a [=mixin member|member=] of [=mixin=] |M|,
+and exposed as a data property with a [=built-in function object=] value,
+is a distinct [=built-in function object=]
+in each [=interface prototype object=]
+whose associated [=interface=] [=includes=] |M|.
+Similarly, for [=attributes=], each copy of the accessor property has
+distinct [=built-in function objects=] for its getters and setters.
+
+The order of appearance of an [=includes statement=] does not matter.
+
+No [=extended attributes=] defined in this specification are applicable to [=includes statements=].
I would think this goes without saying.
> +Note: In ECMAScript, this implies that each [=regular operation=]
+declared as a [=mixin member|member=] of [=mixin=] |M|,
+and exposed as a data property with a [=built-in function object=] value,
+is a distinct [=built-in function object=]
+in each [=interface prototype object=]
+whose associated [=interface=] [=includes=] |M|.
+Similarly, for [=attributes=], each copy of the accessor property has
+distinct [=built-in function objects=] for its getters and setters.
+
+The order of appearance of an [=includes statement=] does not matter.
+
+No [=extended attributes=] defined in this specification are applicable to [=includes statements=].
+
+<div class="example" id="example-mixin">
+
+ The following [=IDL fragment=] defines an [=interfaces=] and a [=mixin=] stating
s/interfaces/interface
> + you can extend the {{WindowOrWorkerGlobalScope}} [=mixin=] using a [=partial mixin|partial=]:
+
+ <pre class="webidl">
+ partial mixin WindowOrWorkerGlobalScope {
+ [Throws] readonly attribute Crypto crypto;
+ };
+ </pre>
+</div>
+
+<div data-fill-with="grammar-Partial"></div>
+
+<div data-fill-with="grammar-PartialDefinition"></div>
+
+<pre class="grammar" id="prod-Mixin">
+ Mixin :
+ "mixin" identifier Inheritance "{" MixinMembers "}" ";"
I don't think we want or need Inheritance, so like Namespace, we can have just Mixin, not PartialMixin.
> +</pre>
+
+<pre class="grammar" id="prod-MixinMember">
+ MixinMember :
+ Const
+ RegularOperation
+ Stringifier
+ ReadOnly AttributeRest
+</pre>
+
+<pre class="grammar" id="prod-IncludesStatement">
+ IncludesStatement :
+ identifier "includes" identifier ";"
+</pre>
+
+<h3 id="idl-namespaces">Namespaces</h3>
Maybe we can land a re-arranging commit separately to make the diff clearer.
> + };
+</pre>
+
+Note: Mixins, much like [=partial interfaces=], are intended for use as a specification editorial aide,
+allowing a coherent set of functionalities to be grouped together,
+and included in multiple interfaces, possibly across documents.
+
+A [=mixin=] is a specification of a set of <dfn export lt="mixin member">mixin members</dfn>
+(matching <emu-nt><a href="#prod-MixinMembers">MixinMembers</a></emu-nt>),
+which are the [=constants=], [=regular operations=], [=regular attributes=],
+and an eventual [=stringifier=],
+that appear between the braces in the mixin declaration.
+
+These [=constants=], [=operations=], and [=attributes=]
+describe the behaviors that can be implemented by an object,
+as if they were specified on the [=interface=] that [=includes=] them.
We may want to note the different members you cannot mixin, such as maplike/setlike/iterable declarations. Maybe we should also disallow mixing in "special operations" (getters/setters/deleters). In general I think mixins should not be able to change the shape of their target object in such fundamental ways.
> + [=mixin=], [=partial mixin=],
+ [=namespace=], or [=partial namespace=]
+ |C| is declared on.
+ 1. If |C| is a [=partial interface=], [=partial mixin=], or [=partial namespace=], then:
+ 1. If the [{{Exposed}}] [=extended attribute=] is specified on |C|, then:
+ 1. If |H| is set, return the [=set/intersection=] of |C|'s [=own exposure set=]
+ and |H|'s [=exposure set=].
+ 1. Otherwise, return |C|'s [=own exposure set=].
+ 1. Otherwise, set |C| to be the original [=interface=], [=mixin=], or [=namespace=]
+ definition of |C|.
+ 1. If |C| is a [=mixin=], then:
+ 1. If the [{{Exposed}}] [=extended attribute=] is specified on |C|, then:
+ 1. Return the [=set/intersection=] of |C|'s [=own exposure set=]
+ and |H|'s [=exposure set=].
+ 1. Otherwise, return |H|'s [=exposure set=].
+ 1. If |C| is a [=interface=] or [=namespace=], then:
Nit: Un-indent the following lines and convert this into an "Assert: |C| is either an interface or a namespace"
> + and |H|'s [=exposure set=].
+ 1. Otherwise, return |C|'s [=own exposure set=].
+ 1. Otherwise, set |C| to be the original [=interface=], [=mixin=], or [=namespace=]
+ definition of |C|.
+ 1. If |C| is a [=mixin=], then:
+ 1. If the [{{Exposed}}] [=extended attribute=] is specified on |C|, then:
+ 1. Return the [=set/intersection=] of |C|'s [=own exposure set=]
+ and |H|'s [=exposure set=].
+ 1. Otherwise, return |H|'s [=exposure set=].
+ 1. If |C| is a [=interface=] or [=namespace=], then:
+ 1. If the [{{Exposed}}] [=extended attribute=] is specified on |C|, then:
+ 1. Return |C|'s [=own exposure set=].
+ 1. Otherwise:
+ 1. Let |defaults| be a new [=list=].
+ 1. [=list/Append=] the [=primary global interface=] to |defaults|.
+ 1. Return |defaults|.
These three steps could just be "Return « the primary global interface »". That seems a bit nicer?
> +the [=member=] is declared on.
+
+Note: This is because [{{Exposed}}] extended attribute on partials are basicaly shorthand
+for annotating every [=member=] of the partial.
+
+If [{{Exposed}}] appears on a [=partial interface=] or [=partial namespace=],
+then the partial's [=own exposure set=] must be a subset of
+the [=exposure set=] of the partial's original [=interface=] or [=namespace=].
+
+If [{{Exposed}}] appears on an [=interface member|interface=] or [=namespace member=],
+then the [=member=]'s [=exposure set=] must be a subset
+of the [=exposure set=] of the [=interface=] or [=namespace=] it is a member of.
+
+If [{{Exposed}}] appears both on a [=partial mixin=] and its original [=mixin=],
+then the [=partial mixin=]'s [=own exposure set=]
+must be a subset of the [=mixin=]'s [=own exposure set=].
Hmm, why "own" for mixin partials but not-own for interface and namespace partials? I imagine you've thought about this longer so I'm sure you've got it right, I'm just hoping you can explain it to me :)
> +If [{{Exposed}}] appears both on a [=partial mixin=] and its original [=mixin=],
+then the [=partial mixin=]'s [=own exposure set=]
+must be a subset of the [=mixin=]'s [=own exposure set=].
+
+If [{{Exposed}}] appears both on a [=mixin member=] and the [=mixin=] it is a member of,
+then the [=mixin members=]'s [=own exposure set=]
+must be a subset of the [=mixin=]'s [=own exposure set=].
+
+As [=mixins=] can be [=included=] by different [=interfaces=],
+the [=exposure set=] of each copy of their [=mixin members|members=] is function of
+the [=interface=] that [=includes=] them.
+If either the [=mixin member=] itself, the [=partial mixin=] it is a member of (if applicable),
+or the [=mixin=] it is a member of have an [=own exposure set=],
+then the [=mixin member=]'s [=exposure set=] is the [=set/intersection=]
+of the first of these [=own exposure set=] with the the [=host interface=]'s [=exposure set=].
+Otherwise, it is the [=host interface=]'s [=exposure set=].
Some grammar issues with this paragraph, and maybe some long sentences could be cut up. Also I think it's a statement of fact about the above algorithm, and not normative; if so, then maybe making that clearer would be good.
> + {{ConstrainablePattern}},
+ {{WEBGL_compressed_texture_astc}},
+ {{WEBGL_compressed_texture_s3tc_srgb}},
+ {{WEBGL_draw_buffers}}.
+ {{WEBGL_lose_context}},
+ {{ANGLE_instanced_arrays}},
+ {{EXT_blend_minmax}},
+ {{EXT_color_buffer_float}},
+ {{EXT_disjoint_timer_query}},
+ {{OES_standard_derivatives}}, and
+ {{OES_vertex_array_object}}.
+ [[GEOLOCATION-API]]
+ [[ORIENTATION-EVENT]]
+ [[MEDIACAPTURE-STREAMS]]
+ (various [[WEBGL]] extension specifications)
+ </small>
It may be worth including a note about its usage to define what-used-to-be-mixins during a transitional period, so people don't say "those aren't the only ones, DOM uses it too!".
In fact maybe it's worth including a small paragraph in the mixins section about the pattern this is replacing, for the transitional period? Not sure, hmm. Thoughts from others appreciated.
> @@ -11091,71 +11267,58 @@ The [=return type=] of the [=default toJSON operation=] must be {{object}}.
<div class=example id=example-tojson-default-inheritance-and-mixins>
- The following [=IDL fragment=] defines a number of [=interfaces=]
- which are [=inherited interface|inherited=] or [=consequential interfaces=] of <code class="idl">A</code>,
+ The following [=IDL fragment=] defines a number of [=interfaces=] and [=mixins=]
+ which are respectively [=inherited interfaces=] of <code class="idl">A</code>
+ and [=mixins=] [=included=] by <code class="idl">A</code>,
The mixins aren't all included by A.
> @@ -12000,9 +12130,7 @@ The value of the [=function object=]’s “name” property is the String value
For both of “add” and “delete”, if:
-* |A| and |A|’s
- [=consequential interfaces=]
- do not declare an [=interface member=]
+* |A| does not declare an [=interface member=]
Should this be "member" too?
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/heycam/webidl/pull/433#pullrequestreview-66264024
Received on Friday, 29 September 2017 21:03:30 UTC