highland icon indicating copy to clipboard operation
highland copied to clipboard

WHATWG Streams compatibility

Open creatorrr opened this issue 8 years ago • 6 comments

Hey folks,

WHATWG seems to have finished a the browser streams spec along with a working implementation. There are some minor differences between node Streams and web Streams, like these.

It should be trivial to wrap the latter around to have similar APIs, while WHATWG and Node battle it out.

It would then allow us to NOT bundle the node streams module through browserify into the library. We can then ask browser users to bring their own stream polyfill (which in time would become unnecessary). This would greatly reduce library footprint. This might also make splitting into modules ala #3 a lot easier, @caolan

If any of this sound good, happy to PR.

creatorrr avatar Mar 01 '16 20:03 creatorrr

I'm a bit confused about what you're proposing here. We don't currently bundle node streams with the browser library. Node streams aren't lazy enough for our uses. Highland depends on EventEmitter and StringDecoder, neither of which depends on node streams, as far as I know.

I think it makes sense to provide some sort of interop with web streams, similar to how we provide limited interop with Node streams. But we may want to wait until the spec stabilizes more though (specifically the WHATWG Writable spec).

An aside: I skimmed the spec, and it's quite good. It may be possible to build Highland on top of it...

vqvu avatar Mar 02 '16 00:03 vqvu

Hmm, I think misunderstood the node streams interop. Looking through, your point makes sense. What do you think of adding compatibility for web streams?

creatorrr avatar Mar 02 '16 15:03 creatorrr

Could this work with #148 ?

creatorrr avatar Mar 02 '16 18:03 creatorrr

Do you mean add interop support in a module? Depending on what the support looks like, that could be done. For instance, if the interop involved adding pipeTo and pipeThrough methods or a toWebStream converter method, then sure. But we may want to be able to wrap a Highland stream around a web stream via the contructor (i.e., _(webStream)) like we can currently do with node Readables. That can't be added as a module. Side note, I think the proposals in #148 is a bit old. #337 is the implementation currently in 3.x.

I think it's a good idea to add some interop with web streams. I'm just not sure how far we should go. Some options:

  1. Implement consumption. Just a _(webStream) operator.
  2. Extend through to work with web streams.
  3. Implement pipeTo and/or pipeThrough. Or extend pipe to work with web streams.
  4. Implement the full Readable prototype so we ducktype as a web Readable.
  5. Simply provide a toWebStream() that returns a web Readable view of the Highland stream.

One thing I've noticed is that web streams make heavy use of promises throughout their API, so implementing some of this may require a dependency on a promise library. This may be a good reason for putting the interop functionality into a separate module. Web users can polyfill in a Promise library if they need to, and node users can provide one when they load the module.

Anyway, I'd like to hear thoughts on how far we should go, and whether we should do it now or wait until the spec stabilizes.

@jeromew, @quarterto, @svozza, @LewisJEllis?

My current thoughts:

  • (1) is almost certainly needed in some form.
  • Ditto for 2. Users will likely want to pipe through a TransformStream and get back a Highland stream.
  • Implementing pipeTo or pipeThrough (rather than extending pipe) would pollute the Highland prototype too much.
  • (4) is is even worse, but it at least has the benefit of becoming a full-fledged web stream. Could be worth it.
  • (5) is the least impactful and a decent alternative if we don't want to pollute the Highland namespace. This would likely be implemented in a module.

vqvu avatar Mar 03 '16 02:03 vqvu

I agree that (1) and (2) are needed and we should start on an implementation. I am against (3), no point in excessive prototype pollution.

I think we need a lot more discussion on (4) before it can be considered.

(5) should be simple enough to implement.

creatorrr avatar Mar 03 '16 16:03 creatorrr

I think 1 and 2 are definitely worth doing but I don't like option 3 at all: let's leave pipe for Node streams, people already use it when they should be using through so let's not encourage that even more. Out of option 4 or 5, I would favour 5, as you say, it's the least impactful and tbh such an invasive changes as option 4 for a standard that may or may not become commonplace seems a bit premature.

svozza avatar Mar 11 '16 20:03 svozza