uhop / stream-chain

Chain functions as transform streams.
BSD 3-Clause "New" or "Revised" License
85 stars 11 forks source link

Unable to create chain with StreamProxy (duplexify) #4

Closed rhodgkins closed 2 years ago

rhodgkins commented 2 years ago

Hi,

I'm trying to use this library in relation to a streamed Google gRPC response that uses StreamProxy (and duplexify), but there the below check when creating a chain that prevents this...

https://github.com/uhop/stream-chain/blob/49b851fa38c0a53f6c62fa3859a0bfbb4f7008be/index.js#L97-L105

Any ideas on a way around this? (Currently wrapping a pointless PassThrough with the StreamProxy in a NodeJS.pipeline to get around it but this isn't ideal obviously!)

uhop commented 2 years ago

The check in question is to make sure that arguments are functions, arrays, or streams. As far as I can tell StreamProxy (and duplexify) are objects, which do not implement stream protocols and are not based on any standard Node stream.

rhodgkins commented 2 years ago

duplexity uses readable-stream which is a package mirroring the standard Node streams implementing all the stream protocols based on the standard Node stream.

uhop commented 2 years ago

Mirroring is not "the same". Obviously, I cannot include all possible class names of all possible third-party packages in my code for practical reasons. Alternatively, you can ask the developers of the third-party streams to base their classes on Node streams.

If you have another reasonable solution, I would love to hear it.

rhodgkins commented 2 years ago

I'd disagree its third party as its ported from NodeJS, anyway what about matching NodeJS checks for streams?

https://github.com/nodejs/node/blob/a82c1a1da710e83bebd4431a0220e45562a310a3/lib/internal/streams/utils.js#L14-L44

There's no need for the instanceof Transform if its a Duplex stream, so check could just become:

if ( 
   isDuplexNodeStream(fn) || 
   (!index && isReadableNodeStream(fn)) || 
   (index === fns.length - 1 && isWritableNodeStream(fn)) 
 ) { 
   return fn; 
 } 
 throw Error('Arguments should be functions, arrays or streams.'); 
uhop commented 2 years ago

Sounds like a plan. I'll add it to the next release.

uhop commented 2 years ago

Released as 2.2.5.