Skip to content

streams: implement Readable.from async iterator utility - #27660

Closed
guybedford wants to merge 1 commit into
nodejs:masterfrom
guybedford:readable-from
Closed

guybedford wants to merge 1 commit into
nodejs:masterfrom
guybedford:readable-from

Conversation

@guybedford

Copy link
Copy Markdown
Contributor

This implements a Readable.from utility method for easily converting async iterators into readable streams.

The implementation and approach are taken directly from the toReadable implementation in https://github.com/mcollina/stream-iterators-utils.

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message follows commit guidelines

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@nodejs-github-bot nodejs-github-bot added the stream Issues and PRs related to Node.js streams. label May 12, 2019
@guybedford
guybedford requested a review from mcollina May 12, 2019 17:05
@devsnek

devsnek commented May 12, 2019

Copy link
Copy Markdown
Member

fun fact: web streams are also async iterators :P

Comment thread lib/_stream_readable.js Resolved
Comment thread lib/_stream_readable.js Resolved
Comment thread test/parallel/test-readable-from.js Resolved
Comment thread test/parallel/test-readable-from.js Resolved

@benjamingr benjamingr left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Added some comments, generally LGTM and 👍

@benjamingr

Copy link
Copy Markdown
Member

fun fact: web streams are also async iterators :P

@devsnek oh cool that means you ended up following through and making that change?

Comment thread doc/api/stream.md Resolved
Comment thread lib/_stream_readable.js Resolved
Comment thread lib/_stream_readable.js Resolved
Comment thread doc/api/stream.md Resolved
Comment thread doc/api/stream.md Resolved
@jasnell jasnell added the semver-minor PRs that contain new features and should be released in the next minor version. label May 12, 2019

@mcollina mcollina left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM, awesome work!

@benjamingr benjamingr left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is great, thanks for this and for the changes 🙇

I think this, its counterpart of readable streams being async iterable, once promisify, its counterpart callbackify and the promise warnings mean we finally have a good async story for Node.js

I'm excited :D

Comment thread doc/api/stream.md Resolved
Comment thread doc/api/stream.md Resolved
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@guybedford guybedford closed this May 18, 2019
@BridgeAR BridgeAR mentioned this pull request May 21, 2019
4 tasks done
Comment thread doc/api/stream.md
(async function() {
for await (const chunk of iterator) {
// Handle backpressure on write
if (!writeable.write(value))

ghost May 21, 2019

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

where does value come from?

ghost May 21, 2019

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Should be chunk, feel free to open a PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

author ready PRs with CI started, the required approvals, and no outstanding review comments. semver-minor PRs that contain new features and should be released in the next minor version. stream Issues and PRs related to Node.js streams.

Projects

None yet

Development

Successfully merging this pull request may close these issues.