Re: [whatwg/streams] add @@asyncIterator to ReadableStreamDefaultReader (#950) from Timothy Gu on 2018-08-22 (public-webapps-github@w3.org from August 2018)

From: Timothy Gu <notifications@github.com>
Date: Wed, 22 Aug 2018 16:14:36 -0700
To: whatwg/streams <streams@noreply.github.com>
Cc: Subscribed <subscribed@noreply.github.com>
Message-ID: <whatwg/streams/pull/950/review/148705241@github.com>
TimothyGu commented on this pull request.



> @@ -1402,6 +1405,92 @@ lt="ReadableStreamDefaultReader(stream)">new ReadableStreamDefaultReader(<var>st
   1. Perform ! ReadableStreamReaderGenericRelease(*this*).
 </emu-alg>
 
+<h5 id="default-reader-iterator" method for="ReadableStreamDefaultReader">
+iterator({ cancel = true } = {})</h5>

Just don't worry about the line length and put it on the same line as `<h5>`.

> +  reader.
+</div>
+
+<emu-alg>
+  1. If ! IsReadableStreamDefaultReader(*this*) is *false*, throw a *TypeError* exception.
+  1. If *this*.[[ownerReadableStream]] is *undefined*, throw a *TypeError* exception.
+  1. Let _iterator_ be ? ObjectCreate(`<a idl>ReadableStreamDefaultReaderAsyncIteratorPrototype</a>`, « [[Reader]],
+  [[Cancel]] »).
+  1. Set _iterator_.[[Reader]] to *this*.
+  1. Set _iterator_.[[Cancel]] to ! ToBoolean(_cancel_).
+  1. Return _iterator_.
+</emu-alg>
+
+<h5 id="default-reader-@@asynciterator" for="ReadableStreamDefaultReader">[@@asyncIterator]()</h5>
+
+<div class="note">

`<p class="note">`

> +  1. If *this*.[[ownerReadableStream]] is *undefined*, throw a *TypeError* exception.
+  1. Let _iterator_ be ? ObjectCreate(`<a idl>ReadableStreamDefaultReaderAsyncIteratorPrototype</a>`, « [[Reader]],
+  [[Cancel]] »).
+  1. Set _iterator_.[[Reader]] to *this*.
+  1. Set _iterator_.[[Cancel]] to ! ToBoolean(_cancel_).
+  1. Return _iterator_.
+</emu-alg>
+
+<h5 id="default-reader-@@asynciterator" for="ReadableStreamDefaultReader">[@@asyncIterator]()</h5>
+
+<div class="note">
+  The <code>@@asyncIterator</code> method is an alias of {{ReadableStreamDefaultReader/iterator}}.
+</div>
+
+The initial value of the <code>@@asyncIterator</code> method is the same function object as the initial value of the
+<code>iterator</code> method.

This should be linked. Also it should be made explicit which `iterator` method we are talking about (which object it belongs to), if we have more than one in the future.

> @@ -1402,6 +1405,92 @@ lt="ReadableStreamDefaultReader(stream)">new ReadableStreamDefaultReader(<var>st
   1. Perform ! ReadableStreamReaderGenericRelease(*this*).
 </emu-alg>
 
+<h5 id="default-reader-iterator" method for="ReadableStreamDefaultReader">
+iterator({ cancel = true } = {})</h5>

My point about the naming remains. I still would like to see it as `getIterator()` or `iterate()`.

> @@ -1402,6 +1405,92 @@ lt="ReadableStreamDefaultReader(stream)">new ReadableStreamDefaultReader(<var>st
   1. Perform ! ReadableStreamReaderGenericRelease(*this*).
 </emu-alg>
 
+<h5 id="default-reader-iterator" method for="ReadableStreamDefaultReader">
+iterator({ cancel = true } = {})</h5>
+
+<div class="note">

Use a paragraph element.

> @@ -1402,6 +1405,92 @@ lt="ReadableStreamDefaultReader(stream)">new ReadableStreamDefaultReader(<var>st
   1. Perform ! ReadableStreamReaderGenericRelease(*this*).
 </emu-alg>
 
+<h5 id="default-reader-iterator" method for="ReadableStreamDefaultReader">
+iterator({ cancel = true } = {})</h5>
+
+<div class="note">
+  The <code>iterator</code> method returns an async iterator which can be used to consume the stream. The
+  <code>return</code> method of this iterator object will optionally {{ReadableStreamDefaultReader/cancel()}} this

`return` should be linked.

> +
+<h5 id="default-reader-@@asynciterator" for="ReadableStreamDefaultReader">[@@asyncIterator]()</h5>
+
+<div class="note">
+  The <code>@@asyncIterator</code> method is an alias of {{ReadableStreamDefaultReader/iterator}}.
+</div>
+
+The initial value of the <code>@@asyncIterator</code> method is the same function object as the initial value of the
+<code>iterator</code> method.
+
+<h3 id="default-reader-asynciterator-prototype" interface lt="ReadableStreamDefaultReaderAsyncIteratorPrototype">
+ReadableStreamDefaultReaderAsyncIteratorPrototype</h3>
+
+<h4 id="default-reader-asynciterator-prototype-internal-slots">Internal slots</h4>
+
+Instances of {{ReadableStreamDefaultReaderAsyncIteratorPrototype}} are created with the internal slots described in the

You can't have instances of a prototype. Just say "`ReadableStreamDefaultReader` Async Iterator objects" or something similar, and use that name consistently. See https://tc39.github.io/ecma262/#sec-async-from-sync-iterator-objects for how ES does it.

> +
+<h4 id="default-reader-asynciterator-prototype-next" method for="ReadableStreamDefaultReaderAsyncIteratorPrototype">
+next()</h4>
+
+<emu-alg>
+  1. If ! IsReadableStreamDefaultReaderAsyncIteratorPrototype(*this*) is *false*, throw a *TypeError* exception.
+  1. Let _reader_ be *this*.[[Reader]].
+  1. Let _read_ be GetMethod(_reader_, `"read"`).
+  1. If _read_ is an abrupt completion, return a promise rejected with _read_.[[Value]].
+  1. let _result_ be Call(_read_, _reader_).
+  1. If _result_ is an abrupt completion, return a promise rejected with _result_.[[Value]].
+  1. Return NormalCompletion(_result_).
+</emu-alg>
+
+<h4 id="default-reader-asynciterator-prototype-return" method for="ReadableStreamDefaultReaderAsyncIteratorPrototype">
+return(value)</h4>

"value" needs to be in &lt;var>

> +  1. Let _read_ be GetMethod(_reader_, `"read"`).
+  1. If _read_ is an abrupt completion, return a promise rejected with _read_.[[Value]].
+  1. let _result_ be Call(_read_, _reader_).
+  1. If _result_ is an abrupt completion, return a promise rejected with _result_.[[Value]].
+  1. Return NormalCompletion(_result_).
+</emu-alg>
+
+<h4 id="default-reader-asynciterator-prototype-return" method for="ReadableStreamDefaultReaderAsyncIteratorPrototype">
+return(value)</h4>
+
+<emu-alg>
+  1. If ! IsReadableStreamDefaultReaderAsyncIteratorPrototype(*this*) is *false*, throw a *TypeError* exception.
+  1. Let _reader_ be *this*.[[Reader]].
+  1. If *this*.[[Cancel]] is *true*, then:
+    1. Let _cancel_ be ? GetMethod(_reader_, `"cancel"`).
+    1. let _result_ be Call(_cancel_, _reader_, &laquo; _value_ &raquo;).

Let

> +    <td class="non-normative">A {{ReadableStreamDefaultReader}} instance.
+  </tr>
+  <tr>
+    <td>\[[Cancel]]
+    <td class="non-normative">Boolean value indicating if the reader will be cancelled when the stream returns.
+  </tr>
+</table>
+
+<h4 id="default-reader-asynciterator-prototype-next" method for="ReadableStreamDefaultReaderAsyncIteratorPrototype">
+next()</h4>
+
+<emu-alg>
+  1. If ! IsReadableStreamDefaultReaderAsyncIteratorPrototype(*this*) is *false*, throw a *TypeError* exception.
+  1. Let _reader_ be *this*.[[Reader]].
+  1. Let _read_ be GetMethod(_reader_, `"read"`).
+  1. If _read_ is an abrupt completion, return a promise rejected with _read_.[[Value]].

Link "promise rejected with"

> +    <td>\[[Cancel]]
+    <td class="non-normative">Boolean value indicating if the reader will be cancelled when the stream returns.
+  </tr>
+</table>
+
+<h4 id="default-reader-asynciterator-prototype-next" method for="ReadableStreamDefaultReaderAsyncIteratorPrototype">
+next()</h4>
+
+<emu-alg>
+  1. If ! IsReadableStreamDefaultReaderAsyncIteratorPrototype(*this*) is *false*, throw a *TypeError* exception.
+  1. Let _reader_ be *this*.[[Reader]].
+  1. Let _read_ be GetMethod(_reader_, `"read"`).
+  1. If _read_ is an abrupt completion, return a promise rejected with _read_.[[Value]].
+  1. let _result_ be Call(_read_, _reader_).
+  1. If _result_ is an abrupt completion, return a promise rejected with _result_.[[Value]].
+  1. Return NormalCompletion(_result_).

I would wrap the _result_ in a [PromiseResolve](https://tc39.github.io/ecma262/#sec-promise-resolve) (or equivalently, [a promise resolved with](https://www.w3.org/2001/tag/doc/promises-guide/#a-promise-resolved-with) _result_) to guarantee that `next` always returns a promise.

> +  1. Return NormalCompletion(_result_).
+</emu-alg>
+
+<h4 id="default-reader-asynciterator-prototype-return" method for="ReadableStreamDefaultReaderAsyncIteratorPrototype">
+return(value)</h4>
+
+<emu-alg>
+  1. If ! IsReadableStreamDefaultReaderAsyncIteratorPrototype(*this*) is *false*, throw a *TypeError* exception.
+  1. Let _reader_ be *this*.[[Reader]].
+  1. If *this*.[[Cancel]] is *true*, then:
+    1. Let _cancel_ be ? GetMethod(_reader_, `"cancel"`).
+    1. let _result_ be Call(_cancel_, _reader_, &laquo; _value_ &raquo;).
+    1. If _result_ is an abrupt completion, return a promise rejected with _result_.[[Value]].
+  1. Let _iterResult_ be ObjectCreate(%ObjectPrototype%).
+  1. Perform CreateDataProperty(_iterResult_, `"done"`, *true*).
+  1. Perform CreateDataProperty(_iterResult_, `"value"`, _value_).

Use [CreateIterResultObject](https://tc39.github.io/ecma262/#sec-createiterresultobject).

> +</emu-alg>
+
+<h4 id="default-reader-asynciterator-prototype-return" method for="ReadableStreamDefaultReaderAsyncIteratorPrototype">
+return(value)</h4>
+
+<emu-alg>
+  1. If ! IsReadableStreamDefaultReaderAsyncIteratorPrototype(*this*) is *false*, throw a *TypeError* exception.
+  1. Let _reader_ be *this*.[[Reader]].
+  1. If *this*.[[Cancel]] is *true*, then:
+    1. Let _cancel_ be ? GetMethod(_reader_, `"cancel"`).
+    1. let _result_ be Call(_cancel_, _reader_, &laquo; _value_ &raquo;).
+    1. If _result_ is an abrupt completion, return a promise rejected with _result_.[[Value]].
+  1. Let _iterResult_ be ObjectCreate(%ObjectPrototype%).
+  1. Perform CreateDataProperty(_iterResult_, `"done"`, *true*).
+  1. Perform CreateDataProperty(_iterResult_, `"value"`, _value_).
+  1. Return a promise resolved with _iterResult_.

Link "a promise resolved with"

> @@ -1562,6 +1651,16 @@ ReadableStreamBYOBReader(<var>stream</var>)</h4>
   1. Return *true*.
 </emu-alg>
 
+<h4 id="is-readable-stream-default-reader-asynciterator-prototype">
+aoid="IsReadableStreamDefaultReaderAsyncIteratorPrototype" nothrow>IsReadableStreamDefaultReaderAsyncIteratorPrototype (
+ <var>x</var> )</h4>
+
+<emu-alg>
+  1. If Type(_x_) is not Object, return *false*.

Per ECMAScript convention, objects usually use a single uppercase letter or a camel-cased phrase as variable name. In this case I believe _argument_ would suffice.

> @@ -1562,6 +1651,16 @@ ReadableStreamBYOBReader(<var>stream</var>)</h4>
   1. Return *true*.
 </emu-alg>
 
+<h4 id="is-readable-stream-default-reader-asynciterator-prototype">
+aoid="IsReadableStreamDefaultReaderAsyncIteratorPrototype" nothrow>IsReadableStreamDefaultReaderAsyncIteratorPrototype (
+ <var>x</var> )</h4>
+
+<emu-alg>
+  1. If Type(_x_) is not Object, return *false*.
+  1. If _x_ does not have a [[Reader]] and [[Cancel]] internal slots, return *false*.

The "a" seems misplaced.

> @@ -1562,6 +1651,16 @@ ReadableStreamBYOBReader(<var>stream</var>)</h4>
   1. Return *true*.
 </emu-alg>
 
+<h4 id="is-readable-stream-default-reader-asynciterator-prototype">
+aoid="IsReadableStreamDefaultReaderAsyncIteratorPrototype" nothrow>IsReadableStreamDefaultReaderAsyncIteratorPrototype (

You are testing for if an object is an AsyncIterator, not if it's an object.

> +  1. Let _reader_ be *this*.[[Reader]].
+  1. Let _read_ be GetMethod(_reader_, `"read"`).
+  1. If _read_ is an abrupt completion, return a promise rejected with _read_.[[Value]].
+  1. let _result_ be Call(_read_, _reader_).
+  1. If _result_ is an abrupt completion, return a promise rejected with _result_.[[Value]].
+  1. Return NormalCompletion(_result_).
+</emu-alg>
+
+<h4 id="default-reader-asynciterator-prototype-return" method for="ReadableStreamDefaultReaderAsyncIteratorPrototype">
+return(value)</h4>
+
+<emu-alg>
+  1. If ! IsReadableStreamDefaultReaderAsyncIteratorPrototype(*this*) is *false*, throw a *TypeError* exception.
+  1. Let _reader_ be *this*.[[Reader]].
+  1. If *this*.[[Cancel]] is *true*, then:
+    1. Let _cancel_ be ? GetMethod(_reader_, `"cancel"`).

You should wrap this ? in a promise too.

> +
+<h4 id="default-reader-asynciterator-prototype-internal-slots">Internal slots</h4>
+
+Instances of {{ReadableStreamDefaultReaderAsyncIteratorPrototype}} are created with the internal slots described in the
+following table:
+
+<table>
+  <thead>
+    <tr>
+      <th>Internal Slot</th>
+      <th>Description (<em>non-normative</em>)</th>
+    </tr>
+  </thead>
+  <tr>
+    <td>\[[Prototype]]
+    <td>%AsyncIteratorPrototype%

Any reason why this isn't in a .non-normative?

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/whatwg/streams/pull/950#pullrequestreview-148705241
Received on Wednesday, 22 August 2018 23:14:59 UTC