Re: [whatwg/streams] ReadableStream.from(asyncIterable) (#1083)

@domenic commented on this pull request.

Very nice!! I originally was thinking we should add an async iterable type (e.g. `async sequence<>`) to Web IDL, but probably it's best to stick with `any` plus manual plumbing for now until there's a second consumer on the web platform that would use it.

> +  1. Return the result of [=reacting=] to |nextPromise| with the following fulfillment steps,
+     given the argument |iterResult|:
+   1. If [$Type$](|iterResult|) is not Object, throw a {{TypeError}}.
+   1. Let |done| be ? [$IteratorComplete$](|iterResult|).
+   1. If |done| is true:
+    1. Perform ! [$ReadableStreamDefaultControllerClose$](stream.[=ReadableStream/[[controller]]=]).
+   1. Otherwise:
+    1. Let |value| be ? [$IteratorValue$](|iterResult|).
+    1. Perform ! [$ReadableStreamDefaultControllerEnqueue$](stream.[=ReadableStream/[[controller]]=],
+       |value|).
+ 1. Let |cancelAlgorithm| be the following steps:
+  1. Let |returnMethod| be ? [$GetMethod$](|iteratorRecord|.\[[Iterator]], "return").
+  1. If |returnMethod| is undefined, return [=a promise resolved with=] undefined.
+  1. Let |returnResult| be ? [$Call$](|returnMethod|, |iteratorRecord|.\[[Iterator]]).
+  1. Return [=a promise resolved with=] |returnResult|.
+ 1. Set |stream| to ! [$CreateReadableStream$](|startAlgorithm|, |pullAlgorithm|, |cancelAlgorithm|).

This needs to be ?, since startAlgorithm could throw.

> @@ -787,6 +795,38 @@ option. If {{UnderlyingSource/type}} is set to undefined (including via omission
      |underlyingSource|, |underlyingSourceDict|, |highWaterMark|, |sizeAlgorithm|).
 </div>
 
+<div algorithm>
+ The static <dfn id="rs-from" method for="ReadableStream">from(|asyncIterable|)</dfn> method steps
+ are:
+
+ 1. Let |stream| be undefined.
+ 1. Let |iteratorRecord| be undefined.
+ 1. Let |startAlgorithm| be the following steps:
+  1. Set |iteratorRecord| to ? [$GetIterator$](|asyncIterable|, async).

Putting this inside startAlgorithm, instead of just doing it straightaway, means that from() also works if [Symbol.asyncIterator]() returns a promise for an async iterator. That's probably not intended.

> +  1. Let |nextResult| be [$IteratorNext$](|iteratorRecord|).
+  1. If |nextResult| is an abrupt completion, return [=a promise rejected with=]
+     |nextResult|.\[[Value]].
+  1. Let |nextPromise| be [=a promise resolved with=] |nextResult|.\[[Value]].
+  1. Return the result of [=reacting=] to |nextPromise| with the following fulfillment steps,
+     given the argument |iterResult|:
+   1. If [$Type$](|iterResult|) is not Object, throw a {{TypeError}}.
+   1. Let |done| be ? [$IteratorComplete$](|iterResult|).
+   1. If |done| is true:
+    1. Perform ! [$ReadableStreamDefaultControllerClose$](stream.[=ReadableStream/[[controller]]=]).
+   1. Otherwise:
+    1. Let |value| be ? [$IteratorValue$](|iterResult|).
+    1. Perform ! [$ReadableStreamDefaultControllerEnqueue$](stream.[=ReadableStream/[[controller]]=],
+       |value|).
+ 1. Let |cancelAlgorithm| be the following steps:
+  1. Let |returnMethod| be ? [$GetMethod$](|iteratorRecord|.\[[Iterator]], "return").

```suggestion
  1. Let |returnMethod| be ? [$GetMethod$](|iteratorRecord|.\[[Iterator]], "`return`").
```

> +     |nextResult|.\[[Value]].
+  1. Let |nextPromise| be [=a promise resolved with=] |nextResult|.\[[Value]].
+  1. Return the result of [=reacting=] to |nextPromise| with the following fulfillment steps,
+     given the argument |iterResult|:
+   1. If [$Type$](|iterResult|) is not Object, throw a {{TypeError}}.
+   1. Let |done| be ? [$IteratorComplete$](|iterResult|).
+   1. If |done| is true:
+    1. Perform ! [$ReadableStreamDefaultControllerClose$](stream.[=ReadableStream/[[controller]]=]).
+   1. Otherwise:
+    1. Let |value| be ? [$IteratorValue$](|iterResult|).
+    1. Perform ! [$ReadableStreamDefaultControllerEnqueue$](stream.[=ReadableStream/[[controller]]=],
+       |value|).
+ 1. Let |cancelAlgorithm| be the following steps:
+  1. Let |returnMethod| be ? [$GetMethod$](|iteratorRecord|.\[[Iterator]], "return").
+  1. If |returnMethod| is undefined, return [=a promise resolved with=] undefined.
+  1. Let |returnResult| be ? [$Call$](|returnMethod|, |iteratorRecord|.\[[Iterator]]).

Probably we should call it with the cancel reason?

> +     given the argument |iterResult|:
+   1. If [$Type$](|iterResult|) is not Object, throw a {{TypeError}}.
+   1. Let |done| be ? [$IteratorComplete$](|iterResult|).
+   1. If |done| is true:
+    1. Perform ! [$ReadableStreamDefaultControllerClose$](stream.[=ReadableStream/[[controller]]=]).
+   1. Otherwise:
+    1. Let |value| be ? [$IteratorValue$](|iterResult|).
+    1. Perform ! [$ReadableStreamDefaultControllerEnqueue$](stream.[=ReadableStream/[[controller]]=],
+       |value|).
+ 1. Let |cancelAlgorithm| be the following steps:
+  1. Let |returnMethod| be ? [$GetMethod$](|iteratorRecord|.\[[Iterator]], "return").
+  1. If |returnMethod| is undefined, return [=a promise resolved with=] undefined.
+  1. Let |returnResult| be ? [$Call$](|returnMethod|, |iteratorRecord|.\[[Iterator]]).
+  1. Return [=a promise resolved with=] |returnResult|.
+ 1. Set |stream| to ! [$CreateReadableStream$](|startAlgorithm|, |pullAlgorithm|, |cancelAlgorithm|).
+ 1. Return |stream|.

Can we combine steps 1, step 6, and step 7 into just "Return ? CreateReadableStream(...)"?

-- 
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/1083#pullrequestreview-527623377