Skip to content

add @@asyncIterator to ReadableStreamDefaultReader #950

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
97 changes: 97 additions & 0 deletions index.bs
Original file line number Diff line number Diff line change
Expand Up @@ -1283,6 +1283,9 @@ would look like
<a href="#default-reader-cancel">cancel</a>(reason)
<a href="#default-reader-read">read</a>()
<a href="#default-reader-release-lock">releaseLock</a>()

<a href="#default-reader-getiterator">getIterator</a>({ preventCancel = false })
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In both the algorithm and the header, we should follow the pattern of pipeTo (and other web APIs), of not defaulting to false, but instead doing ToBoolean on the argument.

<a href="#default-reader-@@asynciterator">[@@asyncIterator]</a>({ preventCancel = false })
}
</code></pre>

Expand Down Expand Up @@ -1402,6 +1405,90 @@ lt="ReadableStreamDefaultReader(stream)">new ReadableStreamDefaultReader(<var>st
1. Perform ! ReadableStreamReaderGenericRelease(*this*).
</emu-alg>

<h5 id="default-reader-getiterator" method for="ReadableStreamDefaultReader">getIterator({ preventCancel = false } = {})
</h5>

<p class="note">
The <code>getIterator</code> method returns an async iterator which can be used to consume the stream. The
{{ReadableStreamDefaultReaderAsyncIteratorPrototype/return()}} method of this iterator object will optionally
{{ReadableStreamDefaultReader/cancel()}} this reader.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should talk about canceling the stream, not link to the public API on reader that does so.

We should also mention the auto-release behavior.

</p>

<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]],
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ObjectCreate should not fail ,so add !

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we don't use ! anywhere else.

[[PreventCancel]] »).
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting. Elsewhere in the spec we fail to add the internal slots. For consistency let's remove that here, and file a separate issue to fix it. (Although I'd like a fix that is less verbose and annoying than the ES spec's repetition of the internal slots list.)

1. Set _iterator_.[[Reader]] to *this*.
1. Set _iterator_.[[PreventCancel]] to ! ToBoolean(_cancel_).
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cancel -> preventCancel

1. Return _iterator_.
</emu-alg>

<h5 id="default-reader-@@asynciterator" for="ReadableStreamDefaultReader">[@@asyncIterator]()</h5>

<p class="note">
The <code>@@asyncIterator</code> method is an alias of {{ReadableStreamDefaultReader/getIterator()}}.
</p>

The initial value of the <code>@@asyncIterator</code> method is the same function object as the initial value of the
{{ReadableStreamDefaultReader/getIterator()}} method.

<h3 id="default-reader-asynciterator-prototype" interface lt="ReadableStreamDefaultReaderAsyncIteratorPrototype">
ReadableStreamDefaultReaderAsyncIteratorPrototype</h3>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This introduces whitespace between the > and the R, so this is not the right place to wrap.


<h4 id="default-reader-asynciterator-prototype-internal-slots">Internal slots</h4>

{{ReadableStreamDefaultReaderAsyncIteratorPrototype}} is created with the internal slots described in the following
table:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not super-comfortable with describing the internal slots of prototypes this way. Let me try something different.

In particular, [[Prototype]] is an internal slot of ReadableStreamDefaultReaderAsyncIteratorPrototype, whereas the other two are slots on the instances.


<table>
<thead>
<tr>
<th>Internal Slot</th>
<th>Description (<em>non-normative</em>)</th>
</tr>
</thead>
<tr>
<td>\[[Prototype]]
<td class="non-normative">%AsyncIteratorPrototype%
</tr>
<tr>
<td>\[[Reader]]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For no good reason, we use lowercase slot names in this spec. We have an open bug to fix, but we should align until then.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should have a slot name that's more distinguishable for the brand check.

<td class="non-normative">A {{ReadableStreamDefaultReader}} instance.
</tr>
<tr>
<td>\[[PreventCancel]]
<td class="non-normative">Boolean value indicating if the reader will be cancelled when the stream returns.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's the stream that is canceled, not the reader. And it's not the stream that returns, it's the async iterator.

</tr>
</table>

<h4 id="default-reader-asynciterator-prototype-next" method for="ReadableStreamDefaultReaderAsyncIteratorPrototype">
next()</h4>

<emu-alg>
1. If ! IsReadableStreamDefaultReaderAsyncIterator(*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>a promise rejected with</a> _read_.[[Value]].
1. Let _result_ be Call(_read_, _reader_).
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing .[[Value]]

1. If _result_ is an abrupt completion, return <a>a promise rejected with</a> _result_.[[Value]].
1. Return <a>a promise resolved with</a> _result_.[[Value]].
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is missing auto-release if result.[[Value]]'s done property is set to true, discussed in #778 (comment)

</emu-alg>

<h4 id="default-reader-asynciterator-prototype-return" method for="ReadableStreamDefaultReaderAsyncIteratorPrototype">
return( <var>value</var> )</h4>

<emu-alg>
1. If ! IsReadableStreamDefaultReaderAsyncIterator(*this*) is *false*, throw a *TypeError* exception.
1. Let _reader_ be *this*.[[Reader]].
1. If *this*.[[PreventCancel]] is *false*, then:
1. Let _cancel_ be GetMethod(_reader_, `"cancel"`).
1. If _cancel_ is an abrupt completion, return <a>a promise rejected with</a> _cancel_.[[Value]].
1. Let _result_ be Call(_cancel_.[[Value]], _reader_, &laquo; _value_ &raquo;).
1. If _result_ is an abrupt completion, return <a>a promise rejected with</a> _result_.[[Value]].
1. Return <a>a promise resolved with</a> ? ReadableStreamCreateReadResult(_value_, *true*, *true*).
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ReadableStreamCreateReadResult cannot fail, so use !

</emu-alg>

<h3 id="byob-reader-class" interface lt="ReadableStreamBYOBReader">Class <code>ReadableStreamBYOBReader</code></h3>

The {{ReadableStreamBYOBReader}} class represents a <a>BYOB reader</a> designed to be vended by a {{ReadableStream}}
Expand Down Expand Up @@ -1562,6 +1649,16 @@ ReadableStreamBYOBReader(<var>stream</var>)</h4>
1. Return *true*.
</emu-alg>

<h4 id="is-readable-stream-default-reader-asynciterator-prototype">
aoid="IsReadableStreamDefaultReaderAsyncIterator" nothrow>IsReadableStreamDefaultReaderAsyncIterator (
<var>argument</var> )</h4>

<emu-alg>
1. If Type(_argument_) is not Object, return *false*.
1. If _argument_ does not have [[Reader]] and [[PreventCancel]] internal slots, return *false*.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We only generally test one slot.

1. Return *true*.
</emu-alg>

<h4 id="is-readable-stream-byob-reader" aoid="IsReadableStreamBYOBReader" nothrow>IsReadableStreamBYOBReader (
<var>x</var> )</h4>

Expand Down
2 changes: 1 addition & 1 deletion reference-implementation/.eslintrc.json
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
"es6": true
},
"parserOptions": {
"ecmaVersion": 6
"ecmaVersion": 2018
},
"globals": {
"ReadableStream": false,
Expand Down
12 changes: 12 additions & 0 deletions reference-implementation/lib/helpers.js
Original file line number Diff line number Diff line change
Expand Up @@ -156,3 +156,15 @@ exports.MakeSizeAlgorithmFromSizeFunction = size => {
}
return chunk => size(chunk);
};

// https://tc39.github.io/ecma262/#sec-getmethod
exports.GetMethod = (V, P) => {
const func = V[P];
if (func === undefined || func === null) {
return undefined;
}
if (typeof func !== 'function') {
throw new TypeError('func is not callable');
}
return func;
};
72 changes: 71 additions & 1 deletion reference-implementation/lib/readable-stream.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,8 @@
const assert = require('better-assert');
const { ArrayBufferCopy, CreateAlgorithmFromUnderlyingMethod, IsFiniteNonNegativeNumber, InvokeOrNoop,
IsDetachedBuffer, TransferArrayBuffer, ValidateAndNormalizeHighWaterMark, IsNonNegativeNumber,
MakeSizeAlgorithmFromSizeFunction, createArrayFromList, typeIsObject } = require('./helpers.js');
MakeSizeAlgorithmFromSizeFunction, createArrayFromList, typeIsObject, GetMethod, Call } =
require('./helpers.js');
const { rethrowAssertionErrorRejection } = require('./utils.js');
const { DequeueValue, EnqueueValueWithSize, ResetQueue } = require('./queue-with-sizes.js');
const { AcquireWritableStreamDefaultWriter, IsWritableStream, IsWritableStreamLocked,
Expand Down Expand Up @@ -700,8 +701,57 @@ class ReadableStreamDefaultReader {

ReadableStreamReaderGenericRelease(this);
}

getIterator({ preventCancel = false } = {}) {
if (IsReadableStreamDefaultReader(this) === false) {
throw defaultReaderBrandCheckException('getIterator');
}
if (this._ownerReadableStream === undefined) {
throw readerLockException('getIterator');
}
const iterator = Object.create(ReadableStreamDefaultReaderAsyncIteratorPrototype, {
_reader: { value: undefined },
_preventCancel: { value: undefined }
});
iterator._reader = this;
iterator._preventCancel = Boolean(preventCancel);
return iterator;
}
}

ReadableStreamDefaultReader.prototype[Symbol.asyncIterator] = ReadableStreamDefaultReader.prototype.getIterator;

const ReadableStreamDefaultReaderAsyncIteratorPrototype = Object.setPrototypeOf({
next() {
if (!IsReadableStreamDefaultReaderAsyncIterator(this)) {
throw defaultReaderAsyncIteratorBrandCheckException('next');
}
try {
const reader = this._reader;
const read = GetMethod(reader, 'read');
return Call(read, reader, []);
} catch (e) {
return Promise.reject(e);
}
},
return(value) {
if (!IsReadableStreamDefaultReaderAsyncIterator(this)) {
throw defaultReaderAsyncIteratorBrandCheckException('return');
}
try {
if (this._preventCancel === false) {
const reader = this._reader;
const cancel = GetMethod(reader, 'cancel');
Call(cancel, reader, []);
return Promise.resolve(ReadableStreamCreateReadResult(value, true, true));
}
return { done: true, value: undefined };
} catch (e) {
return Promise.reject(e);
}
}
}, Object.getPrototypeOf(Object.getPrototypeOf(async function* () {}).prototype));

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What benefit does it add setting the prototype to this?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.


class ReadableStreamBYOBReader {
constructor(stream) {
if (!IsReadableStream(stream)) {
Expand Down Expand Up @@ -808,6 +858,17 @@ function IsReadableStreamDefaultReader(x) {
return true;
}

function IsReadableStreamDefaultReaderAsyncIterator(argument) {
if (!typeIsObject(argument)) {
return false;
}
if (!Object.prototype.hasOwnProperty.call(argument, '_reader') ||
!Object.prototype.hasOwnProperty.call(argument, '_cancel')) {
return false;
}
return true;
}

function ReadableStreamReaderGenericInitialize(reader, stream) {
reader._ownerReadableStream = stream;
stream._reader = reader;
Expand Down Expand Up @@ -2013,6 +2074,15 @@ function defaultReaderClosedPromiseResolve(reader) {
reader._closedPromise_reject = undefined;
}

// Helper functions for the ReadableStreamDefaultReaderAsyncIterator

function defaultReaderAsyncIteratorBrandCheckException(name) {
return new TypeError(
`ReadableStreamDefaultReaderAsyncItreator.prototype.${name} can only be used on a
ReadableStreamDefaultReaderAsyncIterator`);
}


// Helper functions for the ReadableStreamDefaultReader.

function byobReaderBrandCheckException(name) {
Expand Down