decodeStrings option for Writable streams doesn't decode incoming data to Strings

[dgholz]

• Version: v11.2.0
• Platform: MacOS
• Subsystem: streams2

I am implementing a Writable stream to process output from spawned processes. Since I know that the processes will write utf8-encoded strings, I wanted to implement the stream so the incoming data would arrive as Strings. I was delighted to see decodeStrings mentioned under writable._write:

chunk <Buffer> | <string> | <any> The chunk to be written. Will always be a buffer unless the decodeStrings option was set to false or the stream is operating in object mode.

I didn't quite understand how setting decodeStrings to false would result in the stream not getting chunks as Buffers, but I tried it. It didn't work the way I interpreted the docs, so I was quite surprised to see the chunks still arriving as Buffers.

After reading the Constructor docs:

decodeStrings <boolean> Whether or not to encode strings as Buffers before passing them to stream._write(), using the encoding specified in the stream.write() call. Default: true.

and the actual implementation of decodeChunk, I realised that the option name didn't match the behavior, and it would be more accurate to call it encodeStrings (to match the nomenclature used in other languages/libraries when they refer to the act of taking an internal string representation & converting it to a byte sequence using a specified encoding).

The Constructor docs confused me, because the option said decode and the description said encode and I knew they couldn't both be accurate, and so I had to choose between assuming the option name was right (and Strings were decoded from Buffers), or the description was right (and Strings were encoding into Buffers).

I'm filing this issue because I'm grumpy about the time it took to understand the behaviour and feel that I would have spent less time to work out my use-case wasn't supported if there was a clearer option name, or docs which made it clear that the option name didn't reflect the behaviour. Something like:

chunk <Buffer> | <string> | <any> A Buffer converted from the String passed to stream.write(). If the stream is operating in object mode or the stream's decodeStrings option is false, chunk will not be converted & will be whatever was passed to stream.write().

decodeStrings <boolean> Convert Strings passed to stream.write() to Buffers (using the encoding given to the stream.write() call) before passing them to writable._write. Other types of data are not converted. Setting to false will pass Strings through without conversion. Default: true.

I now know I have to implement something to handle my original problem (decode incoming Buffers into Strings) myself. I'm happy to do that, but if you think the Writable's decodeStrings feature could be more configurable, I'd be very happy to use it instead.

[addaleax]

The Constructor docs confused me, because the option said decode and the description said encode and I knew they couldn't both be accurate

That’s because we’re stuck with the wrong option name for backwards compatibility, but we can adjust its documentation. 😕

Would you like to open a PR against this repo to improve the documentation along the lines of your suggestions?

I'm happy to do that, but if you think the Writable's decodeStrings feature could be more configurable, I'd be very happy to use it instead.

I’d also like it if we could have an equivalent of readable stream’s setEncoding – I made a PR a while back in #7425. If you want, you can feel free to resurrect it and see if you can push it over the line. :)

[dgholz]

I certainly make a PR for a doc change. I can't commit to helping out with #7425, I don't know about node performance to improve it or argue why a change to make devs lives easier is worth a performance hit.

That branch looks great, just what I wanted