Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

use readable-stream #15

Closed
Raynos opened this Issue · 12 comments

2 participants

@Raynos

I can make a PR for this

@dominictarr
Owner

I'm gonna refactor this to use duplex (instead of through)
If you want to make a pull request, put it on duplex.

To be honest, I'm not particularily excited about streams2,
it's ugly because it's backwards compatible.
and since it's backwards compatible, then this will still work fine.

So, I don't really see what advantage you'll get from making everything streams2.

That time would be better spent building new useful modules,
rather than needlessly reworking things that already work fine.

@Raynos

@dominictarr the advantage is being able to call read on a stream without it failing.

The advantage is being able to hide the complexity of "is this a 0.8 stream or a 0.9 stream" becomes the former is not forward compatible.

If I can just require a module and know it uses 0.9 streams then I don't have to worry about that shit.

Sure if I only use pipe everything is the same but that's a leaky abstraction.

Also the "finish" event on writable streams isn't on 0.8

@dominictarr
Owner

we are gonna be in a is this a "classic stream or a new stream" for a while, because we are still mostly classic streams, why don't you just write a module that converts a classic stream into a new stream?

what does the "finish" event mean?

@Raynos

Emitted by a writable stream when it's finished writing everything. This happens after .end() has been called.

Finished basically means "I have drained my internal buffer and finished writing it". After you call .end() on a writable stream it may still do some asynchronous writes.

@Raynos

@dominictarr agreed. I don't want a module that turns 0.8 stream into 0.9 stream, I want to take modules i use and rewrite them to be 0.9 streams.

I want to solve the "classic stream or new stream" thing by making sure all things I use are new streams and not letting classic streams ever enter my dependency tree.

Which means I can either fork and PR the module or just write my own thing. I prefer to fork and PR rather then to do the NIH thing.

@dominictarr
Owner

right, so it means that the writable side is done.
what is this useful for from a user perspective?

if mux-demux had a finish event it would emit it synchronously when you call .end() because it doesn't actually do any IO.

@Raynos

@dominictarr

source.pipe(mdm.createStream(...))
    .on("finish", function () {
         // finished sending data from here to remote
         // show user something
    })

It's just generally useful.

@dominictarr
Owner

@Raynos okay, so you are creating A LOT of extra work for your self.

And for what end? are you really in "0.8 or 0.9" hell?

New streams arn't even stable yet!

Just write a toNewStream(classicStream) function and get on with your life.
most of the streams I have written don't even do IO, they are just things that you pipe IO into.
very rarely is the stated benefit of new streams -- that you don't need to pipe them in the first tick --
an issue.

New streams are not the "one true way". They are just another way of implementing the underlying
streaming idea, that makes a different set of tradeoffs, you could them many other ways also.

@Raynos

@dominictarr I don't mind the extra work.

@dominictarr
Owner

Okay, so it's simple anyway, because nearly every stream module I write just uses either through or duplex
so, just make pull requests for those that give you a dropin new style stream.

I don't want to make that the default until 0.10, but it would work like this:

var through = require('through').newStyle
var duplex = require('duplex').newStyle

I'm certainly not making new style default until node@0.10

@Raynos

through is a nightmare to port. tests are weird.

I'll just wrap old streams, easier.

@Raynos Raynos closed this
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.