Re: [heycam/webidl] Replace serializers by toJSON and [Default] extended attribute (#323)

domenic commented on this pull request.

Overall looking pretty great. I guess the two big outstanding questions to me are:

1. Whether we want the generic-ness of [Default] and default operations
2. Whether we want the default algorithm to perform all these observable-from-ES steps, or go straight to the backing stuff

> @@ -1985,6 +1989,124 @@ The following extended attributes are applicable to operation arguments:
         "void"
 </pre>
 
+<h5 id="idl-tojson-operation">toJSON</h5>
+
+By declaring a “toJSON” [=regular operation=],
+an [=interface=] specifies how to convert the objects that implement it to [=JSON values=].
+
+The “toJSON” [=regular operation=] is reserved for this usage.
+It must take zero arguments and return a [=JSON value=].

I think the distinction here between values and types is a bit confusing. I think just renaming this to "JSON type" is the way to go. If you ever have to say "a value of a JSON type" then you can do so, but that should be rare...

> +The “toJSON” [=regular operation=] is reserved for this usage.
+It must take zero arguments and return a [=JSON value=].
+
+The list of types that <dfn id="dfn-json-values" export lt="JSON value">JSON values</dfn> can have
+is as follows:
+
+*   a [=numeric type=],
+*   a {{boolean}} type,
+*   a [=string type=],
+*   a [=nullable type=] whose [=inner type=] is a [=JSON value=],
+*   a [=union type=] where all of its [=member types=] are [=JSON values=],
+*   a [=sequence type=] whose parameterized type is a [=JSON value=],
+*   a [=dictionary=] where all of its [=dictionary members|members=] are [=JSON values=],
+*   a [=record=] where all of its [=map/values=] are [=JSON values=], and
+*   an [=object type=],
+*   an [=interface type=] that has a “toJSON” method declared on itself or

s/method/operation, right?

> +is as follows:
+
+*   a [=numeric type=],
+*   a {{boolean}} type,
+*   a [=string type=],
+*   a [=nullable type=] whose [=inner type=] is a [=JSON value=],
+*   a [=union type=] where all of its [=member types=] are [=JSON values=],
+*   a [=sequence type=] whose parameterized type is a [=JSON value=],
+*   a [=dictionary=] where all of its [=dictionary members|members=] are [=JSON values=],
+*   a [=record=] where all of its [=map/values=] are [=JSON values=], and
+*   an [=object type=],
+*   an [=interface type=] that has a “toJSON” method declared on itself or
+    one of its [=inherited interface|inherited=] or
+    [=consequential interface|consequential=] interface.
+
+How the “toJSON” [=regular operation=]

I might just omit this paragraph (and the next one), as kind of the point here is that it behaves just like any other operation in terms of language bindings. The restrictions are there so that when you perform the usual exposure steps, the result is still usable by ECMAScript's JSON.stringify, but toJSON itself isn't really special, besides just having to follow some restrictions.

> +          [Default] any toJSON();
+        };
+    </pre>
+
+    Assuming each of the attributes of <code class="idl">Transaction</code> is backed by an associated value,
+    so that each [=attribute getter=] returns the corresponding associated value,
+    then the “toJSON” [=regular operation=] could be defined as follows:
+
+    <blockquote>
+        <div algorithm="example tojson">
+
+            The “toJSON” operation of the <code class="idl">Transaction</code> interface
+            must follow these steps:
+
+            1.  Let |json| be a new <code class="idl">TransactionJSONValue</code> dictionary.
+            1.  Set |json|[“from”] to this <code class="idl">Transaction</code>'s associated "from" value.

Can we make these quotes not be curly?

> @@ -8058,6 +7765,79 @@ for an interface is to be implemented.
 </div>
 
 
+<h4 id="Default" extended-attribute lt="Default">[Default]</h4>

Do we anticipate using this super-general "default operations" framework for anything else in the future? I'd be tempted to just go with [DefaultToJSON] and make it all specific to ToJSON here...

> +
+The [{{Default}}] extended attribute must not
+be used on anything other than a [=regular operation=]
+for which a [=corresponding default operation=] has been defined.
+
+<div class="example">
+
+    As an example, the [{{Default}}] extended attribute is suitable
+    for use on the “toJSON” [=regular operation=] of the
+    <a href="https://w3c.github.io/browser-payment-api/#paymentresponse-interface"><code class="idl">PaymentResponse</code></a>
+    interface [[payment-request]].
+
+    <pre highlight="webidl">
+        [SecureContext]
+        interface PaymentResponse {
+          readonly attribute DOMString paymentRequestId;

Note that this is probably changing to an `id` member in payment request. Maybe using a made-up example is better.

> @@ -10684,11 +10463,19 @@ property-installation style as namespaces.)
                 [=identifier=] |id| on |target| and with argument count |n|.
             1.  Let &lt;|operation|, |values|&gt; be the result of passing
                 |S| and |arg|<sub>0..|n|−1</sub> to the [=overload resolution algorithm=].
-            1.  Let |R| be the result of performing the actions listed in the
-                description of |operation|, on |O| if |O| is not <emu-val>null</emu-val>, with |values|
-                as the argument values.
-            1.  Return the result of [=converted to an ECMAScript value|converting=] |R| to
-                an ECMAScript value of the type |op| is declared to return.
+            1.  Let |R| be <emu-val>null</emu-val>.
+            1.  If |operation| is declared with a [{{Default}}] [=extended attribute=],
+                then:
+                1.  Set |R| be the result of performing the actions listed in
+                    |operation|'s [=corresponding default operation=]
+                    on |O| if |O| is not <emu-val>null</emu-val>,

I think we should assert O is not null here instead of including this clause.

Similarly if we go for un-genericifying this we can assert that _values_ is zero-length.

> @@ -10710,6 +10497,45 @@ property-installation style as namespaces.)
     1.  Return |F|.
 </div>
 
+
+<h5 id="es-default-operations">Default operations</h5>
+
+Only [=regular operations=] which have a <dfn>corresponding default operation</dfn> defined below
+may be declared with a [{{Default}}] [=extended attribute=].
+
+
+<h6 id="es-default-tojson">default toJSON operation</h5>

Capitalize "D"

> +        that declares a “toJSON” operation
+        or which has a [=consequential interface=] that does.
+        If such an interface exists, then:
+        1.  Let |P| be that interface.
+        1.  If a “toJSON” operation is declared on |P|,
+            then let |super| be that “toJSON” operation.
+        1.  Otherwise, let |super| be the “toJSON” operation
+            declared on one of |P|'s [=consequential interfaces=].
+        1.  Let |result| be [=?=] [=Call=](|super|, |O|).
+        1.  If Type(|result|) is not Object, return |result|.
+    1.  Otherwise, let |result| be [=!=] [=ObjectCreate=]([=%ObjectPrototype%=]).
+    1.  For each [=exposed=] [=regular attribute=] |attr| that is an [=interface member=] of |I|:
+        1.  Let |id| be |attr|’s [=identifier=] [=converted to an ECMAScript value=].
+        1.  Let |value| be [=?=] [=Get=](|O|, |id|).
+        1.  Perform [=?=] [=CreateDataProperty=](|result|, |id|, |value|).
+    1.  Return |result|.

So I guess we have open questions as to whether this algorithm should observably call the super, or observably perform Get()s. I think it's easy enough to phrase it as doing neither. E.g. to be non-observable instead of

> Let _result_ be ? Call(_super_, _O_)

say something like

> Let _result_ be the result of performing the steps listed in the description of _super_ on _O_ (or the default steps defined for _super_ if _super_ is annotated with [Default])

(and switch around subsequent the type testing to work on IDL values instead of ES values).

And instead of

> Let _value_ be ? Get(_O_, _id_)

say something like

> Let _value_ be the result of performing the steps listed in the description the getter for _id_ on _O_.

-- 
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/323#pullrequestreview-25100169