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 from/to native streams? #1

Closed
MattiasBuelens opened this issue May 15, 2018 · 5 comments · Fixed by #135
Closed

Convert from/to native streams? #1

MattiasBuelens opened this issue May 15, 2018 · 5 comments · Fixed by #135
Labels

Comments

@MattiasBuelens
Copy link
Owner

MattiasBuelens commented May 15, 2018

Right now, this project is just a polyfill: it implements the streams specification, and that's it. In reality though, you may need to inter-operate with non-polyfilled streams, e.g. when using web APIs that consume or produce native streams. In that case, you need a way to convert between native streams and polyfilled streams.

web-streams-adapter provides a low-level API to do such conversions between any stream implementation. For this polyfill, it might be interesting to provide an easier way to convert from/to a polyfilled stream.

These could be provided as static utility methods:

ReadableStream.from(stream: ReadableStreamLike): ReadableStream;
ReadableStream.toNative(stream: ReadableStream): typeof window.ReadableStream;

or exported separately:

export function fromReadableStream(stream: ReadableStreamLike): ReadableStream;
export function toNativeReadableStream(stream: ReadableStream): typeof window.ReadableStream;

Any thoughts on this API?

@MattiasBuelens
Copy link
Owner Author

Some more thoughts on this...

Right now, the polyfill always replaces the global stream implementations. However, if the native implementation were fully spec-compliant, the polyfill should detect this and simply use the native implementation. (At the moment, Chrome 68 is the closest to full compliance, supporting everything except for readable byte streams.)

If the polyfill would re-export the native implementation, then we can't provide ReadableStream.from without mutating the global ReadableStream. Therefore, we should not use static utility methods.

@jimmywarting
Copy link

Looks like the spec is planing on adding ReadableStream.from(iterable) now as well.

It could serve as a grate utility to convert the polyfill stream to/from a native one (until you are using native streams - #20)

maybe could have two implementation in the meanwhile?

  • window.ReadableStream.from(iterable) // converts iterable to native
  • polyfill.ReadableStream.from(iterable) // converts iterable to polyfilled version

all doe the later one would quickly be obsolete when you are using native streams.
the native ReadableStream would also need asyncIterable to work...

@MattiasBuelens
Copy link
Owner Author

Good idea! 👍

Indeed, if ReadableStream.from() becomes a thing, the polyfill can provide a version which also handles conversions. Aside from accepting an (async) iterable, it could also accept a native ReadableStream (even if the native stream is not async iterable itself).

I'll keep it in mind when I (eventually) get to this. Right now, I'm keeping an eye on the ongoing work for converting the spec to WebIDL, in particular on the functional changes that will be needed for the polyfill... 👀

@jimmywarting
Copy link

Hi, any progress on implementing ReadableStream.from?
I rather prefer using ReadableStream.from(iterable) rather than having to use NodeJS specific stream.Readable.toWeb()

using NodeJS specific things makes my code less cross env compatible with other runtimes.

@MattiasBuelens
Copy link
Owner Author

@jimmywarting Sorry for the long delay! I finally implemented ReadableStream.from(), it'll come in a 3.3.0 release very soon. 😁

As for the next branch, I decided to restart from scratch. I got sidetracked trying to make every polyfill method be compatible with any stream implementation (see #122). And because I got stuck on that, I never actually made a new release in over a year...

So I came up with a new plan:

Hopefully I should get that out of the door in a shorter timeframe. 😅

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

Successfully merging a pull request may close this issue.

2 participants