Node: Explicit pipe events for streams

Created on 26 May 2020  Â·  9Comments  Â·  Source: nodejs/node

Right now it's a bit ... hackish ... to detect piping from/to a stream.
You have to do some tricks with the newListener event to see if a data listener is attached.

Makes me wonder if we shouldn't add more explicit pipeing events such as

  • src.emit('pipe-to', dst) when a readable is being piped somewhere
  • dst.emit('pipe-from', src) when a writable is being piped to.

We do emit pipe atm on the dst i think, but having more explicit events makes it more clear I think.

/cc @mcollina @ronag

We are discussing this in streamx as well which is where this came up: https://github.com/mafintosh/streamx/issues/16

stream

Most helpful comment

gulpjs/to-through seems like a bit of a magical use case for me.

Takes a readableStream as the only argument and returns a through2 stream. If the returned stream is piped before nextTick, the wrapped readableStream will not flow until the upstream is flushed. If the stream is not piped before nextTick, it is ended and flushed (acting as a proper readable).

i.e. an API dependent on timing

I don't think this is a use case which motivates adding more stuff into Readable. Especially things that encourages more of these (for me) rather strange applications.

All 9 comments

I'm not sure I fully understand the use case. I'm not entirely comfortable with adding more stuff to Readable.

You can see the examples linked in the streamx issue I linked.
To avoid code like this: https://github.com/gulpjs/to-through/blob/552d17efd3f9469166bc87832e77e86602850828/index.js#L34-L43

gulpjs/to-through seems like a bit of a magical use case for me.

Takes a readableStream as the only argument and returns a through2 stream. If the returned stream is piped before nextTick, the wrapped readableStream will not flow until the upstream is flushed. If the stream is not piped before nextTick, it is ended and flushed (acting as a proper readable).

i.e. an API dependent on timing

I don't think this is a use case which motivates adding more stuff into Readable. Especially things that encourages more of these (for me) rather strange applications.

The README is out of date in it's description of the implementation, nextTick was how it was done before advice from @mcollina. The current code watches for new data or readable listeners to determine when we should flow the readableStream argument instead of waiting for nextTick.

The use case is that we have:

input1.pipe(toThrough(input2)).pipe(writeable1);
// or
toThrough(input).pipe(writeable2);

This causes writeable1 to get all data from input1 before data from input2. writable2 gets the data from input.

I think the current code in to-through is correct, while the README is incorrect.

Right now it's a bit ... hackish ... to detect piping from/to a stream.
You have to do some tricks with the newListener event to see if a data listener is attached.

@mafintosh I think it's a bit more complex than that. Specifically because we should not just check for pipes, but if the stream is being actively read from or not. One can consume a stream either 'data' or 'readable', which are used by 'pipe' and async iteration respectively.

I'm ok in adding more visibility on the internal state machine, but I'm not convinced adding those specific events will help solve the issue at hand.

Specifically because we should not just check for pipes, but if the stream is being actively read from or not.

I think this illustrates my concern... if we add things like this users will start to make incomplete assumptions and create libraries that break under certain edge conditions.

@mcollina ya, we need a “flowing” event

@mcollina ya, we need a “flowing” event

Hm, that's reasonable idea. I don't like it because of what I wrote above but would not oppose it.

'flowin' is probably not the right term, because we identify a stream as flowing if it has a 'data' handler (not a 'readable').

More bikeshedding? 'start'?

Was this page helpful?
0 / 5 - 0 ratings

Related issues

jonathanong picture jonathanong  Â·  93Comments

addaleax picture addaleax  Â·  146Comments

mikeal picture mikeal  Â·  90Comments

feross picture feross  Â·  208Comments

egoroof picture egoroof  Â·  90Comments