stream.compose documentation could be better

[JoakimCh]

Version

17.1.0

Platform

Linux

Subsystem

stream

What steps will reproduce the bug?

No response

How often does it reproduce? Is there a required condition?

No response

What is the expected behavior?

No response

What do you see instead?

No response

Additional information

const duplexStream = duplexer3(subprocess.stdin, subprocess.stdout) // this one works
const duplexStream =   compose(subprocess.stdin, subprocess.stdout) // this one doesn't

Is it not supposed to work the same as duplexer3?

The error I get:
The argument 'streams[0]' must be readable.

EDIT:
This was a misunderstanding on my behalf, I tried to find an alternative to duplexer3 and mistook stream.compose for having the same functionality. Instead stream.Duplex.from({writable, readable}) actually has the functionality I was looking for!

Further down I figured out my mistake and suggested which changes could be done to the documentation to make it clearer.

[ronag]

Can you try:

const duplexStream = compose({ writable: subprocess.stdin, readable: subprocess.stdout })

[JoakimCh]

Can you try:

const duplexStream = compose({ writable: subprocess.stdin, readable: subprocess.stdout })

Thanks.
This resulted in another error though:

Error [ERR_STREAM_WRITE_AFTER_END]: write after end
    at new NodeError (node:internal/errors:371:5)
    at _write (node:internal/streams/writable:320:11)
    at Duplexify.Writable.write (node:internal/streams/writable:335:10)
    at ReadStream.ondata (node:internal/streams/readable:777:22)
    at ReadStream.emit (node:events:390:28)
    at addChunk (node:internal/streams/readable:324:12)
    at readableAddChunk (node:internal/streams/readable:297:9)
    at ReadStream.Readable.push (node:internal/streams/readable:234:10)
    at node:internal/fs/streams:275:14
    at FSReqCallback.wrapper [as oncomplete] (node:fs:660:5) {
  code: 'ERR_STREAM_WRITE_AFTER_END'
}

[ronag]

Can you provide a full repro example?

[JoakimCh]

Can you provide a full repro example?

Took my time to do this now and the last error message was because I piped it to stdout (which closes it when done) and tried to print to stdout after that. So the only bug then is having to specify the readable/writeable for it to work.

[ronag]

That's more of a feature request rather than a bug. I'm not sure I see the value of it. { writable, readable } is more explicit.

[JoakimCh]

No, it's a bug because the doc says I can use it the way I tried to use it and that didn't work:
Combines two or more streams into a Duplex stream that writes to the first stream and reads from the last.

[ronag]

Not sure how it improve that wording. PR is welcome.

[JoakimCh]

The wording in the documentation is fine, this is not a documentation bug.

This is what's documented stream.compose(...streams) where streams can be: <Stream[]> | <Iterable[]> | <AsyncIterable[]> | <Function[]>.

Your syntax { writable, readable } is not documented... I guess that creates an object which is usable as a duplex stream by compose?

Btw, I don't submit PR's, that's why I wrote a bug report. Someone else will need to fix them.

[ronag]

this is not a documentation bug.

I disagree.

Your syntax { writable, readable } is not documented.

This needs to be documented. compose uses Duplex.from.

Btw, I don't submit PR's, that's why I wrote a bug report. Someone else will need to fix them.

Let's hope someone will fix it then.

[JoakimCh]

This needs to be documented. compose uses Duplex.from.

Then the differences between stream.compose and stream.Duplex.from needs to be documented as well, if there is any. If no differences then stream.compose should be deprecated in my opinion. Edit: I guess the difference is that compose can take many streams; where the first must be writeable and the last readable. Hence why my original code should not fail btw.

[ronag]

Edit: I guess the difference is that compose can take many streams;

Exactly, compose takes many.

where the first must be writeable and the last readable.

This is not true. If there are multiple streams then all but the last must be readable. Otherwise we can't connect them.

[JoakimCh]

This is not true. If there are multiple streams then all but the last must be readable. Otherwise we can't connect them.

I'm very confused by your statement. How can you compose them into a new duplex stream if the first is not writeable and the last is not readable?

I didn't specify writeable or readable in an exclusive form.

EDIT:
Nevermind, I guess I understand what you meant... I must have misunderstood the use case of compose. What I want to do is covered by stream.Duplex.from and I shouldn't have confused compose being able to do the same. So it is a documentation bug then; because it's what lead to my confusement.

So if I try to re-understand compose by what you've told me the streams all need to be duplex streams. Because the first one is written to and must be readable so it can be fed into the second one and so on. But no, honestly I'm lost... Why wouldn't the last one also need to be a (readable) duplex stream then? The composed duplex needs a readable input from somewhere...

Can you explain how compose is supposed to be used together with streams according to your description (all but the last must be readable) of it?

[JoakimCh]

I'm sorry, I edited again after your thumb up... I am having a brain meltdown I guess.

If documentation must be fixed, it needs to avoid others getting melted brains. Hence why I want a better understanding of the use case.

[JoakimCh]

So I'm thinking that the documentation could be changed to something like this (my changes in bold):

Combines two or more Duplex streams (e.g. transforms) into a Duplex stream that writes to the first stream and reads from the last. Each provided stream is piped into the next, using stream.pipeline. If any of the streams error then all are destroyed, including the outer Duplex stream.

It should not be confused with stream.Duplex.from({writable, readable}) which can combine a write-only stream with a read-only stream to create a Duplex (where the streams doesn't get piped).

Because stream.compose returns a new stream that in turn can (and should) be piped into other streams, it enables composition. In contrast, when passing streams to stream.pipeline, typically the first stream is a readable stream and the last a writable stream, forming a closed circuit.

If passed a Function it must be a factory method taking a source Iterable.

[ronag]

Good job! Sounds great.

[debadree25]

I think this issue is still relevant? and the PR linked to it was closed without landing, should we maybe reland the previous PR or a fresh one?