Re: [heycam/webidl] Specify open dictionaries. (#180)

domenic commented on this pull request.

Mostly LGTM, a few nits and suggestions

> @@ -7529,6 +7576,101 @@ iterable |iterable| and an iterator getter
 </div>
 
 
+<h4 id="es-record">Records — record&lt;|K|, |V|&gt;</h4>
+
+IDL {{record}}&lt;|K|, |V|&gt; values are represented by
+ECMAScript <emu-val>Object</emu-val> values.
+
+<p id="es-to-record">
+    An ECMAScript value |O| is [=converted to an IDL value|converted=] to an IDL <code>{{record}}&lt;|K|, |V|></code> value as follows:
+</p>
+
+<ol class="algorithm">
+    1.  Let |result| be a new empty instance of <code>{{record}}&lt;|K|, |V|></code>.
+    1.  If [=Type=](|O|) is <emu-val>Undefined</emu-val> or <emu-val>Null</emu-val>,

Nit: types (Undefined, Null, and Object) are not emu-vals. That is reserved for language values, not spec values.

> +    1.  If [=Type=](|O|) is <emu-val>Undefined</emu-val> or <emu-val>Null</emu-val>,
+        return |result|.
+    1.  If [=Type=](|O|) is not <emu-val>Object</emu-val>,
+        <a lt="es throw">throw a <emu-val>TypeError</emu-val></a>.
+    1.  Let |keys| be [=?=] |O|.[=[[OwnPropertyKeys]]=]().
+    1.  Repeat, for each element |key| of |keys| in [=List=] order:
+        1.  Let |desc| be [=?=] |O|.[=[[GetOwnProperty]]=](|key|).
+        1.  If |desc| is not <emu-val>undefined</emu-val>
+            and |desc|.[=[[Enumerable]]=] is <emu-val>true</emu-val>:
+            1.  Let |typedKey| be |key| [=converted to an IDL value=] of type |K|.
+            1.  Let |value| be [=?=] [=Get=](|O|, |key|).
+            1.  Let |typedValue| be |value| [=converted to an IDL value=] of type |V|.
+            1.  If |typedKey| is already a key in |result|, set its value to |typedValue|.
+
+                Note: This can happen when |O| is a proxy object.
+            1.  Otherwise, append to |result| a mapping from |typedKey| to |typedValue|.

Link mapping? And use (|typedKey|, |typedValue|) notation?

> +    An IDL <code>{{record}}&lt;…></code> value |D| is [=converted to an
+    ECMAScript value|converted=] to an ECMAScript value as follows:
+</p>
+
+<ol class="algorithm">
+    1.  Let |result| be [=!=] [=ObjectCreate=]([=%ObjectPrototype%=]).
+    1.  Repeat, for each [=record/mapping=] (|key|, |value|) in |D|:
+        1.  Let |esKey| be |key| [=converted to an ECMAScript value=].
+        1.  Let |esValue| be |value| [=converted to an ECMAScript value=].
+        1.  Let |created| be [=!=] [=CreateDataProperty=](|result|, |esKey|, |esValue|).
+        1.  Assert: |created| is <emu-val>true</emu-val>.
+    1.  Return |result|.
+</ol>
+
+<div class="example" id="example-es-record">
+    Passing the ECMAScript value <code>{b: 3, a: 4}</code> to a

s/to/as/

> +</p>
+
+<ol class="algorithm">
+    1.  Let |result| be [=!=] [=ObjectCreate=]([=%ObjectPrototype%=]).
+    1.  Repeat, for each [=record/mapping=] (|key|, |value|) in |D|:
+        1.  Let |esKey| be |key| [=converted to an ECMAScript value=].
+        1.  Let |esValue| be |value| [=converted to an ECMAScript value=].
+        1.  Let |created| be [=!=] [=CreateDataProperty=](|result|, |esKey|, |esValue|).
+        1.  Assert: |created| is <emu-val>true</emu-val>.
+    1.  Return |result|.
+</ol>
+
+<div class="example" id="example-es-record">
+    Passing the ECMAScript value <code>{b: 3, a: 4}</code> to a
+    <code>{{record}}&lt;DOMString, double></code> argument
+    would result in the IDL value <code>[ ("b", 3), ("a", 4) ]</code>.

I don't know about this new array-ish notation. I'd instead say something like "a record containing the two mappings ("b", 3) and ("a", 4), in that order." Hmm, I guess it makes an appearance later in the table too, where it's more necessary for space reasons... not sure what to do. At least, remove the `<code>` from this one, IMO.

I think this also contradicts the statement "There is no way to represent a constant record value in IDL."

> @@ -7695,6 +7837,9 @@ represented by ECMAScript values that correspond to the union’s
         1.  If |types| includes a [=dictionary type=], then return the
             result of [=converted to an IDL value|converting=]
             |V| to that dictionary type.
+        1.  If |types| includes a [=record type=], then return the
+            result of [=converted to an IDL value|converting=]
+            |V| to that dictionary type.

s/dictionary/record

> +<h4 id="es-record">Records — record&lt;|K|, |V|&gt;</h4>
+
+IDL {{record}}&lt;|K|, |V|&gt; values are represented by
+ECMAScript <emu-val>Object</emu-val> values.
+
+<p id="es-to-record">
+    An ECMAScript value |O| is [=converted to an IDL value|converted=] to an IDL <code>{{record}}&lt;|K|, |V|></code> value as follows:
+</p>
+
+<ol class="algorithm">
+    1.  Let |result| be a new empty instance of <code>{{record}}&lt;|K|, |V|></code>.
+    1.  If [=Type=](|O|) is <emu-val>Undefined</emu-val> or <emu-val>Null</emu-val>,
+        return |result|.
+    1.  If [=Type=](|O|) is not <emu-val>Object</emu-val>,
+        <a lt="es throw">throw a <emu-val>TypeError</emu-val></a>.
+    1.  Let |keys| be [=?=] |O|.[=[[OwnPropertyKeys]]=]().

IMO the links for the internal methods and [[enumerable]] are not good to add here. If we want to do that more generally, we can, but we shouldn't do it just in this section.

-- 
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/180#pullrequestreview-4332904