Change the way you indicate you want a Buffer

[sindresorhus]

Setting {encoding: null} is such a weird API I should never have inherited from childProcess.

We can introduce a new way while still supporting the old way.

Some possible solutions:

1. {encoding: 'buffer'} (It's not technically an encoding, but who cares?)
2. {returnValue: 'string' | 'buffer'}

In Got, we went with {buffer: Boolean}, but we already use that option name for something else.

[ehmicky]

I just found out that {encoding: 'buffer'} is actually supported by child_process and documented:

If encoding is 'buffer', or an unrecognized character encoding, Buffer objects will be passed to the callback instead.

https://github.com/nodejs/node/blob/64a5c01b99e97854ae933d2210555ba6ee54db14/lib/internal/child_process.js#L1112

> child_process.execSync('echo oh', {encoding: 'buffer'})
<Buffer 6f 68 0a>

Unrecognized character encodings do behave the same, but only with exec() and execFile(). With spawnSync(), execSync() and execFileSync(), the following is thrown instead:

Uncaught TypeError [ERR_UNKNOWN_ENCODING]: Unknown encoding: exampleEncoding
    at __node_internal_captureLargerStackTrace (node:internal/errors:496:5)
    at new NodeError (node:internal/errors:405:5)
    at Uint8Array.toString (node:buffer:851:11)
    at Object.spawnSync (node:internal/child_process:1116:43)
    at spawnSync (node:child_process:873:24)
    at Object.execSync (node:child_process:954:15)

spawn() does not have the encoding option, and ignores it, since it does not buffer the value.

Interestingly enough, the 'buffer' encoding does not work with any Buffer method. For example, Buffer.from(myString, 'buffer') and myBuffer.toString('buffer') throw. The null/'buffer' encoding seems to be specific to child_process.

---

It turns out we accidentally supported encoding: 'buffer' and broke this with today's major release 8.0.0. That's because the new get-stream version is not handling the encoding anymore, and we now use myBuffer.toString(encoding) instead, which throws with 'buffer'.

I do think a boolean option like binary would be a better API. However, since {encoding: 'buffer'} is supported by Node.js child_process itself, it seems to make sense to use that instead.

I just fixed this with #572.

[sindresorhus]

I do think a boolean option like binary would be a better API. However, since {encoding: 'buffer'} is supported by Node.js child_process itself, it seems to make sense to use that instead.

Agreed. Although, I would prefer to remove support for null at some point.

[ehmicky]

Update: due to discussion #586, we are now considering removing encoding: null.