Add stdin: ReadableStream and stdout|stderr: WritableStream options

[ehmicky]

As discussed in #593 (comment)

We should support web streams for the stdin/stdout/stderr options.

We can implement this by converting them to Node.js streams using stream.Readable.fromWeb() and stream.Writable.fromWeb()

[aaronccasanova]

Definitely a feature I would leverage!

We can implement this by converting them to Node.js streams using stream.Readable.fromWeb() and stream.Writable.fromWeb()

Is Stability: 1 - Experimental a concern? I haven't been using the Web Stream utils in production for that reason. Might be worth opening a request in Node.js to mark as stable. (At least that's the process I'm familiar with following along with the util.parseArgs stability)

[ehmicky]

That's a very good point @aaronccasanova.

My concern would be to end up re-implementing the same logic, but without all the tests and insight knowledge that James Snell has (see original PR in nodejs/node#39134). From checking the history, it seems rather stable.

For reference, links to the code of stream.Writable.fromWeb() and stream.Readable.fromWeb(). It's quite intricate.

We could also wait until those APIs become stable.

What are your thoughts on this @sindresorhus?

[sindresorhus]

I think we should do it and use the built-in APIs, but we can mention in our docs too that the web streams stuff is experimental.

[ehmicky]

I gave it a go and ran into the following problem: child_process.spawn() stdio option only accepts streams that have an underlying file descriptor.

https://nodejs.org/api/child_process.html#optionsstdio

<Stream> object: Share a readable or writable stream that refers to a tty, file, socket, or a pipe with the child process. 
The stream's underlying file descriptor is duplicated in the child process to the fd that corresponds to the index in the stdio array. 
The stream must have an underlying descriptor (file streams do not until the 'open' event has occurred).

The problem is: this is currently checked by verifying if the stream is an instanceof the following classes: Pipe, TTY, TCP and UDP. Those classes are implemented in C++ and are not accessible userland.

https://github.com/nodejs/node/blob/5ac658102d8f84a4c3e00e3c54e89dc9784c3394/lib/internal/child_process.js#L340-L347

https://github.com/nodejs/node/blob/5ac658102d8f84a4c3e00e3c54e89dc9784c3394/lib/internal/child_process.js#L1060-L1071

Unfortunately, when passing a web stream to stream.Readable.fromWeb() or stream.Writable.fromWeb(), the return value is never an instance of Pipe/TTY/TCP/UDP . Even when the web stream technically has an underlying file descriptor, for example the response.body of fetch().

I do not see a way to go around this problem. If you do, please let me know!

[ehmicky]

Update: there is a solution. When a web stream is passed to the stdin/stdout/stderr option, pass 'pipe' instead. Once the process is spawned, pipe the web stream to process.stdin|stdout|stderr. I tried this approach already in #604.

[ehmicky]

stdin done at #615.

[ehmicky]

stdout and stderr done at #619.