Skip to content
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

add @@asyncIterator to ReadableStream #954

Closed
wants to merge 7 commits into from
Closed
Show file tree
Hide file tree
Changes from 5 commits
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
100 changes: 100 additions & 0 deletions index.bs
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ spec:promises-guide; type:dfn;
<pre class="anchors">
urlPrefix: https://tc39.github.io/ecma262/; spec: ECMASCRIPT
text: %Uint8Array%; url: #sec-typedarray-objects; type: constructor
text: %AsyncIteratorPrototype%; url: #sec-asynciteratorprototype; type: interface
text: AsyncIterator; url: #sec-asynciterator-interface; type: interface
text: ArrayBuffer; url: #sec-arraybuffer-objects; type: interface
text: DataView; url: #sec-dataview-objects; type: interface
text: Number; url: #sec-ecmascript-language-types-number-type; type: interface
Expand Down Expand Up @@ -403,6 +405,8 @@ like
<a href="#rs-pipe-through">pipeThrough</a>({ writable, readable }, options)
<a href="#rs-pipe-to">pipeTo</a>(dest, { preventClose, preventAbort, preventCancel } = {})
<a href="#rs-tee">tee</a>()
domenic marked this conversation as resolved.
Show resolved Hide resolved

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

Expand Down Expand Up @@ -866,6 +870,93 @@ option. If <code><a for="underlying source">type</a></code> is set to <code>unde
</code></pre>
</div>

<h5 id="rs-asynciterator" method for="ReadableStream">getIterator({ <var>preventCancel</var> = false } = {})</h5>

<div 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, by default,
<a lt="cancel a readable stream">cancel</a> the stream; it will also release the reader.
</div>

<emu-alg>
1. If ! IsReadableStream(*this*) is *false*, throw a *TypeError* exception.
1. Let _reader_ be ? AcquireReadableStreamDefaultReader(*this*).
1. Let _iterator_ be ! ObjectCreate(`<a idl>ReadableStreamAsyncIteratorPrototype</a>`).
1. Set _iterator_.[[asyncIteratorReader]] to _reader_.
1. Set _iterator_.[[preventCancel]] to ! ToBoolean(_preventCancel_).
1. Return _iterator_.
</emu-alg>

<!-- Bikeshed doesn't let us mark this up correctly: https://github.com/tabatkins/bikeshed/issues/1344 -->
<h5 id="rs-asynciterator" for="ReadableStream">[@@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
{{ReadableStream/getIterator()}} method.

<h3 id="rs-asynciterator-prototype" interface
lt="ReadableStreamAsyncIteratorPrototype">ReadableStreamAsyncIteratorPrototype</h3>

{{ReadableStreamAsyncIteratorPrototype}} is an ordinary object that is used by {{ReadableStream/[@@asyncIterator]()}} to
construct the objects it returns. Instances of {{ReadableStreamAsyncIteratorPrototype}} implement the {{AsyncIterator}}
abstract interface from the JavaScript specification. [[!ECMASCRIPT]]

The {{ReadableStreamAsyncIteratorPrototype}} object must have its \[[Prototype]] internal slot set to
{{%AsyncIteratorPrototype%}}.

<h4 id="default-reader-asynciterator-prototype-internal-slots">Internal slots</h4>
Objects created by {{ReadableStream/[@@asyncIterator]()}}, using {{ReadableStreamAsyncIteratorPrototype}} as their
prototype, 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>\[[asyncIteratorReader]]
<td class="non-normative">A {{ReadableStreamDefaultReader}} instance
</tr>
<tr>
<td>\[[preventCancel]]
<td class="non-normative">A boolean value indicating if the stream will be <a lt="cancel a readable
stream">canceled</a> when the async iterator's {{ReadableStreamAsyncIteratorPrototype/return()}} method is called
</tr>
</table>

<h4 id="rs-asynciterator-prototype-next" method for="ReadableStreamAsyncIteratorPrototype">next()</h4>

<emu-alg>
1. If ! IsReadableStreamAsyncIterator(*this*) is *false*, <a>a promise rejected with</a> *TypeError* exception.
Copy link
Collaborator

Choose a reason for hiding this comment

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

The word "return" is missing in this sentence.

1. Let _reader_ be *this*.[[asyncIteratorReader]].
1. If _reader_.[[ownerReadableStream]] is *undefined*, return <a>a promise rejected with</a> a *TypeError* exception.
1. Return ! ReadableStreamDefaultReaderRead(_reader_, *true*).
</emu-alg>

<h4 id="rs-asynciterator-prototype-return" method
for="ReadableStreamAsyncIteratorPrototype">return( <var>value</var> )</h4>

<emu-alg>
1. If ! IsReadableStreamAsyncIterator(*this*) is *false*, return <a>a promise rejected with</a> *TypeError* exception.
1. Let _reader_ be *this*.[[asyncIteratorReader]].
1. If *this*.[[preventCancel]] is *false*, then:
1. If _reader_.[[ownerReadableStream]] is *undefined*, return <a>a promise rejected with</a> a *TypeError*
Copy link
Collaborator

Choose a reason for hiding this comment

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

These two steps are in both branches of the if statement and so should be moved up above it.

Copy link
Collaborator

Choose a reason for hiding this comment

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

This should also be changed in the reference implementation.

exception.
1. If _reader_.[[readRequests]] is not empty, return <a>a promise rejected with</a> a *TypeError* exception.
1. Let _result_ be ! ReadableStreamReaderGenericCancel(_reader_, _value_).
devsnek marked this conversation as resolved.
Show resolved Hide resolved
1. Perform ! ReadableStreamReaderGenericRelease(_reader_).
1. Return the result of <a>transforming</a> _result_ by a fulfillment handler that returns !
ReadableStreamCreateReadResult(_value_, *true*, *true*).
1. If _reader_.[[ownerReadableStream]] is *undefined*, return <a>a promise rejected with</a> a *TypeError* exception.
1. If _reader_.[[readRequests]] is not empty, return <a>a promise rejected with</a> a *TypeError* exception.
1. Perform ! ReadableStreamReaderGenericRelease(_reader_).
Copy link
Collaborator

Choose a reason for hiding this comment

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

This should reject if reader.[[ownerReadableStream]] is undefined. Otherwise, we may fail an assert in ReadableStreamReaderGenericRelease.

We should probably just move step 3a to before step 3.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think it's impossible for it to be undefined. User code would have to get access to [[reader]] to call releaseLock(), and it can't. So this should be an assert.

Copy link
Collaborator

Choose a reason for hiding this comment

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

It's not impossible for it to be undefined, but you really have to go out of your way to break it:

const iterator = readable.getIterator();
iterator.return(); // releases lock
iterator.return(); // boom!

Copy link
Collaborator

Choose a reason for hiding this comment

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

What happens when there are pending read requests? See previous discussion.

Normally, ReadableStreamDefaultReader.releaseLock() checks whether this.[[readRequests]] is empty. However, this implementation uses ReadableStreamReaderGenericRelease directly without going through the same checks as releaseLock.

1. Return <a>a promise resolved with</a> ! ReadableStreamCreateReadResult(_value_, *true*, *true*).
</emu-alg>

<h3 id="rs-abstract-ops">General readable stream abstract operations</h3>

The following abstract operations, unlike most in this specification, are meant to be generally useful by other
Expand Down Expand Up @@ -984,6 +1075,15 @@ readable stream is <a>locked to a reader</a>.
1. Return *true*.
</emu-alg>

<h4 id="is-readable-stream-asynciterator" aoid="IsReadableStreamAsyncIterator" nothrow
export>IsReadableStreamAsyncIterator ( <var>x</var> )</h4>

<emu-alg>
1. If Type(_x_) is not Object, return *false*.
1. If _x_ does not have a [[asyncIteratorReader]] internal slot, return *false*.
1. Return *true*.
</emu-alg>

<h4 id="readable-stream-tee" aoid="ReadableStreamTee" throws export>ReadableStreamTee ( <var>stream</var>,
<var>cloneForBranch2</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
65 changes: 65 additions & 0 deletions reference-implementation/lib/readable-stream.js
Original file line number Diff line number Diff line change
Expand Up @@ -267,8 +267,61 @@ class ReadableStream {
const branches = ReadableStreamTee(this, false);
return createArrayFromList(branches);
}

getIterator({ preventCancel = false } = {}) {
if (IsReadableStream(this) === false) {
throw streamBrandCheckException('getIterator');
}
const reader = AcquireReadableStreamDefaultReader(this);
const iterator = Object.create(ReadableStreamAsyncIteratorPrototype);
iterator._asyncIteratorReader = reader;
iterator._preventCancel = Boolean(preventCancel);
return iterator;
}
}

const AsyncIteratorPrototype = Object.getPrototypeOf(Object.getPrototypeOf(async function* () {}).prototype);
const ReadableStreamAsyncIteratorPrototype = Object.setPrototypeOf({
next() {
if (IsReadableStreamAsyncIterator(this) === false) {
return Promise.reject(new TypeError());
}
const reader = this._asyncIteratorReader;
if (reader._ownerReadableStream === undefined) {
return Promise.reject(new TypeError());
}
return ReadableStreamDefaultReaderRead(reader, true);
},

return(value) {
if (IsReadableStreamAsyncIterator(this) === false) {
return Promise.reject(new TypeError());
}
const reader = this._asyncIteratorReader;
if (this.preventCancel === false) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should be this._preventCancel.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Good catch! This would have been an embarrassing bug. 😛

if (reader._ownerReadableStream === undefined) {
return Promise.reject(new TypeError());
}
if (reader._readRequests.length > 0) {
return Promise.reject(new TypeError());
}
const result = ReadableStreamReaderGenericCancel(reader, value);
ReadableStreamReaderGenericRelease(reader);
return result.then(() => ReadableStreamCreateReadResult(value, true, true));
}
if (reader._ownerReadableStream === undefined) {
return Promise.reject(new TypeError());
}
if (reader._readRequests.length > 0) {
return Promise.reject(new TypeError());
}
ReadableStreamReaderGenericRelease(reader);
return Promise.resolve(ReadableStreamCreateReadResult(value, true, true));
}
}, AsyncIteratorPrototype);

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

module.exports = {
CreateReadableByteStream,
CreateReadableStream,
Expand Down Expand Up @@ -364,6 +417,18 @@ function IsReadableStreamLocked(stream) {
return true;
}

function IsReadableStreamAsyncIterator(x) {
if (!typeIsObject(x)) {
return false;
}

if (!Object.prototype.hasOwnProperty.call(x, '_asyncIteratorReader')) {
return false;
}

return true;
}

function ReadableStreamTee(stream, cloneForBranch2) {
assert(IsReadableStream(stream) === true);
assert(typeof cloneForBranch2 === 'boolean');
Expand Down