Pass options to createReadStream

[mike-marcacci]

I updated fs-capacitor to pass through the stream options highWaterMark and encoding which is now published as version 6. The API of version 5 was identical to version 4, but without support for node versions < 10. Now that the node 8 LTS window is expired (Dec 31st) we can drop support for it here.

This fixes #177.

[jaydenseric]

What exactly does the encoding option do, in what situations would you use it? The underlying Node.js stream.Writable constructor has a defaultEncoding option, is it directly related; is that a better name?

1. There needs to be a proper JSDoc typedef for the createReadStream function and its parameters, with links in the parameter descriptions to relevant fs-capacitor and/or Node.js docs.
2. Also, there should be tests ensuring the parameters are passed through and work.

[jaydenseric]

@mike-marcacci I added a FileUploadCreateReadStream typedef, but the options descriptions still need to be added.

[mike-marcacci]

I 100% agree that this needs tests – the encoding parameter is just passed through to the filesystem read calls, and allows the user to specify the encoding (like utf8, etc) to read the contents into a stream that emits strings instead of buffers. Because may of these encodings can use variable-byte characters, this can be a really useful feature in general. While I don't have a need for this in graphql-upload I figured I'd add it here since I started supporting it along with the highWaterMark option in fs-capacitor. I'm perfectly fine if you'd rather leave this option off until we get a feature request for it.

[jaydenseric]

Does encoding have to match the encoding of what is buffered to the FS by fs-capacitor, or the original upload’s encoding for that matter? Or does it safely convert whatever the original encoding is, on the fly? What are valid encoding values?

None of the createReadStream options are documented in fs-capacitor, it would be great if that could be fixed:

https://github.com/mike-marcacci/fs-capacitor#createreadstream-readstream

I'm perfectly fine if you'd rather leave this option off until we get a feature request for it.

I'm not objecting to adding this feature to the API, I'm just trying to understand what it does so I can quality control the documentation and be sure its naming makes sense.

[mike-marcacci]

Hey @jaydenseric I've updated the docs in fs-capacitor (along with some other small improvements) which are pending release. Can you look over that and see if that helps clear things up? I mostly just referred to the node docs, since these are advanced options of most streams, which are critical if you need them, but typically don't need to be changed from the defaults.

If you see anything that should change in the README there I'll get those changes before cutting the release.

I'll also try to add tests and docs to this PR tomorrow.

[mike-marcacci]

OK, I added tests and some docs (partly copied from, partly referencing the new docs in fs-capacitor). As I mentioned, these are pretty advanced stream options, and we're essentially just "passing them through" to node here... there's just fairly standard options, so it's good to pass them along.

The primary use-case for encoding is when someone wants to stream something encoded with variable-byte characters like utf8 and doesn't want to handle a chunk that ends partway through a character.

[jaydenseric]

I've pulled in master branch and fixed up the tests, but there is some more work to be done on the documentation as it's currently incorrect.

I'll take it from here.