-
Notifications
You must be signed in to change notification settings - Fork 22.6k
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
FF110 Using ReadableStream with async iteration #25284
Conversation
Preview URLs (comment last updated: 2023-03-24 16:16:15) |
@@ -145,7 +145,262 @@ This is the standard pattern you'll see when using stream readers: | |||
3. If there is more stream to read, you process the current chunk then run the function again. | |||
4. You keep chaining the `pipe` function until there is no more stream to read, in which case step 2 is followed. | |||
|
|||
> **Note:** The function looks as if `pump()` calls itself and leads to a potentially deep recursion. However, because `pump` is asynchronous and each `pump()` call is at the end of the promise handler, it's actually analogous to a chain of promise handlers. | |||
Removing all the code to actually perform a "pump", the code might be generalized to something like this: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FYI The pump()
example above this point is interesting, but kind of hides the algorithm in the steps above (line 146) among the code to try do something with the received data.
So what I have done below is tried to pull this out into the "simple form" using promises.
THEN, I've shown how the same algorithm is even simpler if written using await.
I think this is useful, though I've cheated a bit in the await case by not putting in error handling.
This all leads to the next section.
} | ||
``` | ||
|
||
## Consuming a fetch() using asynchronous iteration |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So this section is designed to show how you use the async iterator. I also wanted to highlight that you can abort the source of the data OR the loop.
Aborting the source is easy - you pass the signal to fetch. The people who wrote the spec say this will be fast, but gave me the impression that if you have a chain of code code here, such as passing to a stream that modifies the chunks, it might not be "optimal". I really don't understand the issue so I have not tried to explain that.
Alternatively, you can use the `AbortSignal` within the `for await...of` loop to call `break`, which will exit the loop. | ||
Note however that code in the loop is only run when there is new data to process, so there may be some delay between the signal being triggered and the `break` being called. | ||
|
||
### Example async reader |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The example section here is mostly mocked, and that code is hidden. In summary, fetch() is monkey patched to return a stream in body
which just returns data. This gets around me actually having to host a stream of data anywhere. Why have I bothered? Mostly because it makes the code more real, and allows easy testing of the abort. On chrome it allows me to immediately show the error because this feature is not supported. I like that.
I like these kind of mocks, and I think they are really helpful when you want to sanity check the test code. But appreciate that others might think they make things more ugly. Opinions welcome.
@Josh-Cena @sideshowbarker Could I please get a review of this sometime next week? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks mostly good to me. The example is very interesting!
files/en-us/web/api/streams_api/using_readable_streams/index.md
Outdated
Show resolved
Hide resolved
files/en-us/web/api/streams_api/using_readable_streams/index.md
Outdated
Show resolved
Hide resolved
files/en-us/web/api/streams_api/using_readable_streams/index.md
Outdated
Show resolved
Hide resolved
files/en-us/web/api/streams_api/using_readable_streams/index.md
Outdated
Show resolved
Hide resolved
files/en-us/web/api/streams_api/using_readable_streams/index.md
Outdated
Show resolved
Hide resolved
```js hidden | ||
// A mock push source. | ||
// Used to simulate some random data arriving |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I recommend you run Prettier over this file. I'll do some little refactors here right before this is ready for merge
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Josh-Cena I already ran Prettier on this. Just verified that - nothing changed before/after.
Co-authored-by: Joshua Chen <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@hamishwillee I've made some little refactors. The example's idea looks quite clever to me.
f752d3e
to
df02682
Compare
df02682
to
4d521e1
Compare
@Josh-Cena Thanks for your changes - all look good to me, and also for your review. I also like this kind of mocked example - certainly fun to write. If I ever get a task to update properly I hope to come back here and mock some sources that allow proper demonstration of backpressure and some of the other concepts that are discussed but not explored in https://developer.mozilla.org/en-US/docs/Web/API/Streams_API/Concepts |
This does an update of Using readable streams to include a section on how to use the
for await...of
syntax withfetch()
. It is intended to be a bit more of a real world example to complement the section added in ReadableStream.I've put a couple of notes inline about what I'm trying to achieve, so that you can help me decide if it is reasonable approach.
This is concluding work for #23678