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

Convert async iterators to streams #2401

Merged
merged 2 commits into from
Jan 5, 2021
Merged

Conversation

olanod
Copy link
Contributor

@olanod olanod commented Dec 20, 2020

An initial implementation of what I proposed in #2399
It would be nice to have the ability to turn async iterators into Rust streams in the same way we can turn promises into futures. Instead of creating a separate crate for this utility I think it fits well in the scope of wasm-bindgen-futures.

@alexcrichton
Copy link
Contributor

Thanks for this! I'm a bit hesitant to do this at this time though since it introduces futures-core as a public dependency. AFAIK Stream is intended at some point to make its way into the standard library, right? If so perhaps this could be gated behind an off-by-default Cargo feature?

@olanod
Copy link
Contributor Author

olanod commented Jan 4, 2021

Yeah it looks like rust-lang/rust#79023 could land anytime which would also remove the need for the dev-dependency. I'll put this under a stream feature if that's ok?

The stream module exposes the JsStream type used to convert
JS objects implementing the AsyncIterator interface to be used
as Rust streams.
@olanod
Copy link
Contributor Author

olanod commented Jan 5, 2021

@alexcrichton Added the feature under the stream flag even for the test because it seemed I would have to change the version of wasm-bindgen-test all over the place. Also mentioned the existence of feature in the Readme as experimental which can be made "stable" later when there is core::stream::Stream and perhaps also adding the feature of Stream to AsyncIterator(ReadableStream?) conversion.

@alexcrichton
Copy link
Contributor

Looks good, thanks! For a small amount of extra future-proofing, though, could this perhaps be named futures-core-03-stream?

@alexcrichton alexcrichton merged commit b2fe5d1 into rustwasm:master Jan 5, 2021
@alexcrichton
Copy link
Contributor

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants