Re: [whatwg/streams] add @@asyncIterator to ReadableStreamDefaultReader (#950)

domenic requested changes on this pull request.

Great start, and sorry for the delay in reviewing!

> +  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>
+
+<emu-alg>
+  1. Let _iterator_ be ? GetMethod(*this*, "iterator").
+  1. Return ? Call(_iterator_, *this*).

Yeah for spec style we should follow https://tc39.github.io/ecma262/#sec-map.prototype-@@iterator I guess.

> +</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>
+
+<emu-alg>
+  1. Let _iterator_ be ? GetMethod(*this*, "iterator").
+  1. Return ? Call(_iterator_, *this*).
+</emu-alg>
+
+<h3 id="default-reader-asynciterator-prototype" interface lt="ReadableStreamDefaultReaderAsyncIteratorPrototype">
+  ReadableStreamDefaultReaderAsyncIteratorPrototype
+</h3>

Style nit: this introduces spaces between the `>` and `R` and the `e` and the `<`. No good. Same throughout various places in the patch.

> +  <tr>
+    <td>\[[Reader]]
+    <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.

IsReadableStreamDefaultReaderAsyncIteratorPrototype doesn't seem to be defined?

> +</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. Return ? Call(_read_, _reader_).
+</emu-alg>
+
+<h4 id="default-reader-asynciterator-prototype-return" method for="ReadableStreamDefaultReaderAsyncIteratorPrototype">
+  return()
+</h4>

So apparently async iterator return methods should take a value argument and return a promise for `{ done: true, value }`.

You'll want to return the result of transforming the call to `cancel()` by a function that creates and returns the `{ done: true, value }` tuple (probably using ReadableStreamCreateReadResult for consistency).

> @@ -700,8 +700,52 @@ class ReadableStreamDefaultReader {
 
     ReadableStreamReaderGenericRelease(this);
   }
+
+  iterator({ cancel = true } = {}) {
+    const O = this;

To follow the spec more exactly, don't do the `O` aliasing. Here and everywhere below.

> @@ -1402,6 +1405,90 @@ 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
+  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]],

This should be `!`, not `?`, I think.

> +    if (typeof O !== 'object') {
+      throw new TypeError();
+    }
+    const reader = O._reader;
+    const read = reader.read;
+    return read.call(reader);
+  },
+  return() {
+    const O = this;
+    if (typeof O !== 'object') {
+      throw new TypeError();
+    }
+    if (O._cancel === true) {
+      const reader = O._reader;
+      const cancel = reader.cancel;
+      cancel.call(reader);

We should put a full GetMethod implementation in helpers.js.

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