Re: [heycam/webidl] Introduce the observable array type (proxy-based) (#840)

TimothyGu commented on this pull request.

Couldn't spot any errors in the proxy traps themselves, but some editorial comments are provided.

> @@ -13629,6 +13800,250 @@ internal method as follows.
     1.  Return <a abstract-op>OrdinaryGetOwnProperty</a>(|O|, |P|).
 </div>
 
+<h3 id="es-observable-arrays">Observable array exotic objects</h3>
+
+An <dfn>observable array exotic object</dfn> is a specific type of ECMAScript
+[=Proxy exotic object=] which is created using the proxy hooks defined in this section. They are
+defined in this manner because the ECMAScript specification includes special treatment for
+[=Proxy exotic objects=] that have <code>Array</code> instances as their proxy target, and we want
+to ensure that [=observable array types=] are exposed to ECMAScript code with this special treatment
+intact.
+
+The proxy traps used by observable array exotic objects work to ensure a number of invariants beyond
+those of normal <code>Array</code> instances:
+
+*   The arrays have no holes, i.e. every property in the inclusive range 0 through
+    <code>observableArray.length</code> will be filled with a value compatible with the specified

<code>observableArray.length</code> – 1? Or use something like "every property in the interval [0, <code>observableArray.length</code>".

> +    properties, while the <code>length</code> property remains as a non-configurable,
+    non-enumerable, and writable data property.
+*   Adding additional properties to the array cannot be prevented using, for example,
+    <code>Object.preventExtensions()</code>.
+
+<div algorithm>
+    To <dfn lt="creating an observable array exotic object">create an observable array exotic object</dfn>
+    in a [=Realm=] |realm|, given Web IDL type |T| and algorithms |setAlgorithm| and |deleteAlgorithm|:
+
+    1.  Let |innerArray| be [=!=] [$ArrayCreate$](0).
+    1.  Let |handler| be [$ObjectCreate$](<emu-val>null</emu-val>, « \[[Type]], \[[SetAlgorithm]], \[[DeleteAlgorithm]], \[[BackingList]] »).
+    1.  Set |handler|.\[[Type]] to |T|.
+    1.  Set |handler|.\[[SetAlgorithm]] to |setAlgorithm|.
+    1.  Set |handler|.\[[DeleteAlgorithm]] to |deleteAlgorithm|.
+    1.  Let |defineProperty| be [=!=] [$CreateBuiltinFunction$](the steps from [[#es-observable-array-defineProperty]], « », |realm|).
+    1.  Perform [=!=] [$CreateDataProperty$](|handler|, "defineProperty", |defineProperty|).

Use `<code>` with string literals, per Infra. But I wonder if we could do a spec-for loop, which will be easier to read, IMO.

> +    1.  Let |defineProperty| be [=!=] [$CreateBuiltinFunction$](the steps from [[#es-observable-array-defineProperty]], « », |realm|).
+    1.  Perform [=!=] [$CreateDataProperty$](|handler|, "defineProperty", |defineProperty|).
+    1.  Let |deleteProperty| be [=!=] [$CreateBuiltinFunction$](the steps from [[#es-observable-array-deleteProperty]], « », |realm|).
+    1.  Perform [=!=] [$CreateDataProperty$](|handler|, "deleteProperty", |deleteProperty|).
+    1.  Let |get| be [=!=] [$CreateBuiltinFunction$](the steps from [[#es-observable-array-get]], « », |realm|).
+    1.  Perform [=!=] [$CreateDataProperty$](|handler|, "get", |get|).
+    1.  Let |getOwnPropertyDescriptor| be [=!=] [$CreateBuiltinFunction$](the steps from [[#es-observable-array-getOwnPropertyDescriptor]], « », |realm|).
+    1.  Perform [=!=] [$CreateDataProperty$](|handler|, "getOwnPropertyDescriptor", |getOwnPropertyDescriptor|).
+    1.  Let |has| be [=!=] [$CreateBuiltinFunction$](the steps from [[#es-observable-array-has]], « », |realm|).
+    1.  Perform [=!=] [$CreateDataProperty$](|handler|, "has", |has|).
+    1.  Let |preventExtensions| be [=!=] [$CreateBuiltinFunction$](the steps from [[#es-observable-array-preventExtensions]], « », |realm|).
+    1.  Perform [=!=] [$CreateDataProperty$](|handler|, "preventExtensions", |preventExtensions|).
+    1.  Let |set| be [=!=] [$CreateBuiltinFunction$](the steps from [[#es-observable-array-set]], « », |realm|).
+    1.  Perform [=!=] [$CreateDataProperty$](|handler|, "set", |set|).
+    1.  Let |ownKeys| be [=!=] [$CreateBuiltinFunction$](the steps from [[#es-observable-array-ownKeys]], « », |realm|).
+    1.  Perform [=!=] [$CreateDataProperty$](|handler|, "ownKeys", |ownKeys|).

Could we use an alphabetical order here?

> +    1.  Perform [=!=] [$CreateDataProperty$](|handler|, "preventExtensions", |preventExtensions|).
+    1.  Let |set| be [=!=] [$CreateBuiltinFunction$](the steps from [[#es-observable-array-set]], « », |realm|).
+    1.  Perform [=!=] [$CreateDataProperty$](|handler|, "set", |set|).
+    1.  Let |ownKeys| be [=!=] [$CreateBuiltinFunction$](the steps from [[#es-observable-array-ownKeys]], « », |realm|).
+    1.  Perform [=!=] [$CreateDataProperty$](|handler|, "ownKeys", |ownKeys|).
+    1.  Return [=!=] [$ProxyCreate$](|innerArray|, |handler|).
+</div>
+
+<h4 id="es-observable-array-defineProperty"><code>defineProperty</code></h4>
+
+<div algorithm="observable array exotic object defineProperty trap">
+    The steps for the <code>defineProperty</code> proxy trap for
+    [=observable array exotic objects=], given |O|, |P|, and |descriptorObj| are as follows:
+
+    1.  Let |handler| be the <emu-val>this</emu-val> value.
+    1.  Let |descriptor| be [=?=] [$ToPropertyDescriptor$](|descriptorObj|).

Can this ever throw?

> +    1.  Return [=!=] [$ProxyCreate$](|innerArray|, |handler|).
+</div>
+
+<h4 id="es-observable-array-defineProperty"><code>defineProperty</code></h4>
+
+<div algorithm="observable array exotic object defineProperty trap">
+    The steps for the <code>defineProperty</code> proxy trap for
+    [=observable array exotic objects=], given |O|, |P|, and |descriptorObj| are as follows:
+
+    1.  Let |handler| be the <emu-val>this</emu-val> value.
+    1.  Let |descriptor| be [=?=] [$ToPropertyDescriptor$](|descriptorObj|).
+    1.  If |P| is "length", then:
+        1.  If [=!=] [$IsAccessorDescriptor$](|descriptor|) is <emu-val>true</emu-val>, then return
+            <emu-val>false</emu-val>.
+        1.  If |descriptor| has a \[[Configurable]] field and |descriptor|.\[[Configurable]] is
+            <emu-val>true</emu-val>, return <emu-val>false</emu-val>.

More concise wording:

> If _descriptor_.[[Configurable]] is present and has the value **true**, return **false**.

or

> If _descriptor_.[[Configurable]] is present and it is **true**, return **false**.

> +
+<h4 id="es-observable-array-deleteProperty"><code>deleteProperty</code></h4>
+
+<div algorithm="observable array exotic object deleteProperty trap">
+    The steps for the <code>deleteProperty</code> proxy trap for
+    [=observable array exotic objects=], given |O| and |P|, are as follows:
+
+    1.  Let |handler| be the <emu-val>this</emu-val> value.
+    1.  If |P| is "length", then return <emu-val>false</emu-val>.
+    1.  If |P| [=is an array index=], then:
+        1.  Let |oldLen| be |handler|.\[[BackingList]]'s [=list/size=].
+        1.  Let |index| be [=!=] [$ToUint32$](|P|).
+        1.  If |index| ≠ |oldLen| &minus; 1, then return
+            <emu-val>false</emu-val>.
+        1.  Perform the algorithm steps given by |handler|.\[[DeleteAlgorithm]], given
+            |handler|.\[[BackingList]][|index|] and |index|.

Can we just say

> Perform _handler_.[[DeleteAlgorithm]](_handler_.[[BackingList]][_index_], _index_).

> @@ -13629,6 +13800,250 @@ internal method as follows.
     1.  Return <a abstract-op>OrdinaryGetOwnProperty</a>(|O|, |P|).
 </div>
 
+<h3 id="es-observable-arrays">Observable array exotic objects</h3>
+
+An <dfn>observable array exotic object</dfn> is a specific type of ECMAScript
+[=Proxy exotic object=] which is created using the proxy hooks defined in this section. They are

The ES spec calls them traps rather than hooks.

-- 
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/840#pullrequestreview-362326611