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

domenic commented on this pull request.



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

This should throw a TypeErro for consistency with closed dictionaries.

> @@ -7529,6 +7575,95 @@ iterable |iterable| and an iterator getter
 </div>
 
 
+<h4 id="es-open-dictionary">Open dictionaries — OpenDictionary&lt;[|K|,] |V|&gt;</h4>
+
+IDL {{OpenDictionary}}&lt;|K|, |V|&gt; values are represented by
+ECMAScript <emu-val>Object</emu-val> values.
+
+<p id="es-to-open-dictionary">
+    An ECMAScript value |O| is [=converted to an IDL value|converted=] to an IDL <code>{{OpenDictionary}}<|K|, |V|></code> value as follows:
+</p>
+
+<ol class="algorithm">
+    1.  Let |result| be a new empty instance of <code>{{OpenDictionary}}<|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>.

This check should not be there, for consistency with closed dictionaries.

> +IDL {{OpenDictionary}}&lt;|K|, |V|&gt; values are represented by
+ECMAScript <emu-val>Object</emu-val> values.
+
+<p id="es-to-open-dictionary">
+    An ECMAScript value |O| is [=converted to an IDL value|converted=] to an IDL <code>{{OpenDictionary}}<|K|, |V|></code> value as follows:
+</p>
+
+<ol class="algorithm">
+    1.  Let |result| be a new empty instance of <code>{{OpenDictionary}}<|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 [=?=] [=EnumerableOwnProperties=](|O|, "key").
+    1.  Repeat, for each element |key| of |keys| in [=List=] order:
+        1.  If [=!=] [=HasOwnProperty=](|O|, |key|) is <code>true</code>, then:

Interesting. So this comes in to play if enumerating/getting causes side effects that delete the property. I don't think that's great semantics though. I think instead you want to inline the EnumerableOwnProperties algorithm here yourself, instead of separating the enumeration pass from the conversion/Get pass.

> +    </pre>
+
+    Open dictionary keys and values can be constrained, although keys can only be
+    constrained among the three string types.
+    The following conversions have the described results:
+    <table class="data">
+        <thead><th>Value</th><th>Passed to type</th><th>Result</th></thead>
+        <tr>
+            <td><code>{"😞": 1}</code></td>
+            <td><code>{{OpenDictionary}}&lt;ByteString, double></code></td>
+            <td><emu-val>TypeError</emu-val></td>
+        </tr>
+        <tr>
+            <td><code>{"\uD83D": 1}</code></td>
+            <td><code>{{OpenDictionary}}&lt;USVString, double></code></td>
+            <td>[ "\uFFFD" ⇒ 1 ]</td>

It seems you are inconsistent about notation for the mappings; here you use an arrow, whereas above you use "Repeat, for each mapping (key, value) in D". Both seem reasonable, but it would be good to pick one and maybe mention it in the section that introduces the concept of mappings.

-- 
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-2426495