Re: [heycam/webidl] [WIP] Define Web IDL Modules (#675)

domenic commented on this pull request.

Very nice. Overall LGTM, although maybe we should hold off on merging until we get synthetic module records accepted.

> +<dfn id="dfn-module-member" export lt="module member">module members</dfn>
+(matching <emu-nt><a href="#prod-ModuleMembers">ModuleMembers</a></emu-nt>),
+which are the [=interfaces=], [=regular operations=] and [=read only=] [=regular
+attributes=] that appear between the braces in the module declaration. These
+operations and attributes describe the behaviors packaged into the module.
+
+The IDL for modules can be split into multiple parts by using <dfn
+id="dfn-partial-module" export>partial module</dfn> definitions (matching
+<emu-t>partial</emu-t> <emu-nt><a href="#prod-Module">Module</a></emu-nt>).
+The module specifier of a partial module definition must be the same as the
+identifier of a module definition. All of the members that appear on each of
+the partial module definitions are considered to be members of the module
+itself.
+
+<pre class="syntax">
+    module <mark>"std:example"</mark> {

As discussed previously, I am hopeful we can centralize the prefix in one place. (Whether it be HTML, or more likely it seems, Web IDL.) Having specs need to remember seems bad.

> +The module specifier of a partial module definition must be the same as the
+identifier of a module definition. All of the members that appear on each of
+the partial module definitions are considered to be members of the module
+itself.
+
+<pre class="syntax">
+    module <mark>"std:example"</mark> {
+      /* module_members... */
+    };
+
+    partial module <mark>"std:example"</mark> {
+      /* module_members... */
+    };
+</pre>
+
+Note: As with partial interface definitions, partial module definitions are

Nit: xref partial interface definitions

> +    };
+
+    partial module <mark>"std:example"</mark> {
+      /* module_members... */
+    };
+</pre>
+
+Note: As with partial interface definitions, partial module definitions are
+intended for use as a specification editorial aide, allowing the definition
+of a module to be separated over more than one section of the document, and
+sometimes multiple documents.
+
+The order that members appear in has significance for property enumeration in
+the <a href="#es-modules">ECMAScript binding</a>.
+
+Note that like namespaces, modules do not create types.

xref namespaces

> +[=interfaces=] and [=partial interfaces=] which are [=module members=], must not
+be annotated with the [{{Exposed}}] or [{{SecureContext}}] extended attributes.
+
+Note: This restriction could be relaxed if there is a sufficient use case;
+it is omitted here for simplicity of specification and implementation, and a
+lack of clear use cases. Please <a
+href="https://github.com/heycam/webidl/issues/new?title=Fine%20grained%20exposure%20in%20modules">
+file an issue</a> if you see a need for this feature.
+
+Each module may have <dfn>module evaluation steps</dfn> which are executed
+when the module is first imported. These steps may throw an [=exception=], which
+prevents the module from being accessed.
+
+Rather than defining behavior when accessed, the attributes of a [=module=] act
+as a simple storage location. The steps to "set a module attribute" to an IDL
+value of the type of the attribute. Each [=attribute=] which is a [=module

"The steps..." sentence seems to be incomplete or missing words.

> +be annotated with the [{{Exposed}}] or [{{SecureContext}}] extended attributes.
+
+Note: This restriction could be relaxed if there is a sufficient use case;
+it is omitted here for simplicity of specification and implementation, and a
+lack of clear use cases. Please <a
+href="https://github.com/heycam/webidl/issues/new?title=Fine%20grained%20exposure%20in%20modules">
+file an issue</a> if you see a need for this feature.
+
+Each module may have <dfn>module evaluation steps</dfn> which are executed
+when the module is first imported. These steps may throw an [=exception=], which
+prevents the module from being accessed.
+
+Rather than defining behavior when accessed, the attributes of a [=module=] act
+as a simple storage location. The steps to "set a module attribute" to an IDL
+value of the type of the attribute. Each [=attribute=] which is a [=module
+member=] must be initialized in [=module evaluation steps=] to set the module

"the" module evaluation steps

> +    <pre class="syntax">
+        [Exposed=Window, SecureContext]
+        module "std:timezone" {
+          interface Timezone {
+            readonly attribute USVString name;
+            long long offsetMs(long long unixTime);
+          };
+          Timezone getCurrentTimezone();
+          readonly attribute Timezone initialTimezone;
+        };
+    </pre>
+
+    <blockquote>
+        The [=module evaluation steps=] of "std:timezone" are as follows:
+            1.  [=set a module attribute|Set the attribute=]
+                "initialTimezone" of "std:timezone" to a [=new=]

Typographically I think `initialTimezone` should be code and should cross-link to the definition. (Not sure cross-linking is possible/easy with example code though). And "of "std:timezone"" should not be necessary here, ideally, although I'm not sure how to rigorize that.

> +            long long offsetMs(long long unixTime);
+          };
+          Timezone getCurrentTimezone();
+          readonly attribute Timezone initialTimezone;
+        };
+    </pre>
+
+    <blockquote>
+        The [=module evaluation steps=] of "std:timezone" are as follows:
+            1.  [=set a module attribute|Set the attribute=]
+                "initialTimezone" of "std:timezone" to a [=new=]
+                <code>Timezone</code> representing the User Agent's preferred
+                time zone.
+    </blockquote>
+
+    An ECMAScript implementation would then expose a global <code class="idl">"std:timezone"</code>

Unsure what "global" means here

> +        The [=module evaluation steps=] of "std:timezone" are as follows:
+            1.  [=set a module attribute|Set the attribute=]
+                "initialTimezone" of "std:timezone" to a [=new=]
+                <code>Timezone</code> representing the User Agent's preferred
+                time zone.
+    </blockquote>
+
+    An ECMAScript implementation would then expose a global <code class="idl">"std:timezone"</code>
+    module which includes a constructor for the interface and a function for the operation:
+
+    <pre highlight="js">
+        import * from "std:timezone" as timezone;
+        Object.keys(timezone);                      // Evaluates to ["Timezone", "getCurrentTimezone", "currentTimezone"]
+        typeof timezone.Timezone;                   // Evaluates to "function"
+        typeof timezone.getCurrentTimezone;         // Evaluates to "function"
+        typeof timezone.currentTimezone;            // Evaluates to "object"

currentTimezone -> initialTimezone

> +            1.  If |member| is an [=interface=]:
+                1.  Let |interfaceObject| be the result of [=create an interface
+                    object|creating an interface object=] for |member| with |id|
+                    in |realm|.
+                1.  Perform ! [=SetSyntheticModuleExport=](|moduleRecord|, |id|,
+                    |interfaceObject|).
+            1.  Otherwise, if |member| is an [=operation=]:
+                1.  Let |method| be the result of [=creating an operation function=]
+                    given |member|, |module|, and |realm|.
+                1.  Perform ! [=SetSyntheticModuleExport=](|moduleRecord|, |id|,
+                    |method|).
+            1.  Otherwise:
+                1.  Assert: |member| is an [=attribute=].
+                1.  Assert: The module evaluation steps will set each attribute.
+        1.  Run the [=module evaluation steps=] for the [=module/specifier=] of
+            |module|, if any are present.

I don't think they're "for the specifier of _module_", I think they're just "for _module_", right?

-- 
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/675#pullrequestreview-210970886

Received on Tuesday, 5 March 2019 23:21:53 UTC