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

Stream pipe documentation is confusing #20635

Closed
jplaisted opened this issue May 9, 2018 · 3 comments
Closed

Stream pipe documentation is confusing #20635

jplaisted opened this issue May 9, 2018 · 3 comments
Labels
doc Issues and PRs related to the documentations. stream Issues and PRs related to the stream subsystem. zlib Issues and PRs related to the zlib subsystem.

Comments

@jplaisted
Copy link

  • Version: 10
  • Platform: n/a
  • Subsystem: stream

Newbie to using NodeJS + NodeJS streams and I find the documentation very confusing, especially around pipes. I get the idea of piping streams but the documentation on pipe says it is on Readable, accepts a Writable, and returns a Writable and then mentions that it is possible to chain calls to pipe (e.g. r.pipe(z).pipe(w);). However it makes zero mention of how this is possible in the document. pipe returns a Writable, yet according to the document Writable has no pipe method. So how can they be chained?

Fiddling around in node inspecting things I see that the stream package is a class that both Readable and Writable inherit from. Perhaps having the documentation actually show this hierarchy and have more "standard" class documentation would help?

Maybe this isn't shown because piping a Writable stream to another stream makes no sense. What are you piping? You can't read from it. Unless you actually can and the idea of a Writable stream is a lie and everything is a Duplex? Or does it just pass the data over that writable and on to the next stream? Maybe this just a general design issue with Node streams? Or maybe there's an undocumented requirement that in order to chain pipe the argument to pipe must be a Duplex? (Digging further that appears to be the case - Writable has a pipe method that throws when called because it is not readable).

Site note: The example in pipe pipes through a Gzip object. But the documentation for Gzip is non existent and makes no mention that is a stream of any kind except in the small examples at the top of the zlib page. It seems pretty important to me to know that it is not only a stream, but a Transform that zips the input stream and outputs the zipped data.

I'm not looking for help using streams or gzip or anything else. Just noting that as someone totally new the documentation is in need of improvement. Especially for something that is presumably very core to Node as streams. Basically clearer documentation around pipe and its requirements (chaining requires streams be Duplexs) and what classes are streams - specifically what kind of stream (like Gzip is a Transform). More generally I'd say the "classic" documentation of documenting classes, methods, and hierarchies is more useful than entirely example based documentation.

@addaleax addaleax added doc Issues and PRs related to the documentations. stream Issues and PRs related to the stream subsystem. zlib Issues and PRs related to the zlib subsystem. labels May 9, 2018
@mscdex
Copy link
Contributor

mscdex commented May 9, 2018

As far as I can see, the only real error here is in the 'Returns' section for readable.pipe(). Instead of what is currently shown:

Returns: <stream.Writable> making it possible to set up chains of piped streams

it's probably meant to instead say:

Returns: destination, making it possible to set up chains of piped streams

since if you read further into the readable.pipe() description it notes (emphasis mine):

The readable.pipe() method returns a reference to the destination stream making it possible to set up chains of piped streams

@addaleax
Copy link
Member

addaleax commented May 9, 2018

I think we should also at least link back from the zlib reference docs to stream.Transform.

@HarshithaKP
Copy link
Member

Looks like this is fixed in #22354

@BridgeAR BridgeAR closed this as completed Jan 3, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
doc Issues and PRs related to the documentations. stream Issues and PRs related to the stream subsystem. zlib Issues and PRs related to the zlib subsystem.
Projects
None yet
Development

No branches or pull requests

5 participants