-
Notifications
You must be signed in to change notification settings - Fork 30.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Documentation should mention encoding
instead of defaultEncoding
as options property for fs.createWriteStream
#14611
Comments
/cc @nodejs/streams @mcollina |
I would say we should update the docs to use |
Sure, but will go through And I'm going to update TS type definitions too (I see it has been noticed already DefinitelyTyped/DefinitelyTyped#14981). |
the fix will go to master, and then we will backport to 8 and 6. |
The documentation for `fs.createWriteStream()` references a `defaultEncoding` as possible options property, but in reality `encoding` property is expected and properly handled. This fix updates the documentation to rename the `defaultEncoding` property to `encoding`. PR-URL: #14867 Fixes: #14611 Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Refael Ackermann <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: James M Snell <[email protected]>
The documentation for `fs.createWriteStream()` references a `defaultEncoding` as possible options property, but in reality `encoding` property is expected and properly handled. This fix updates the documentation to rename the `defaultEncoding` property to `encoding`. PR-URL: #14867 Fixes: #14611 Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Refael Ackermann <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: James M Snell <[email protected]>
Documentation says that
defaultEncoding
is the right options property forfs.createWriteStream
. But if we look into source code [1]options
are directly passed toWriteStream
that expectsoptions.encoding
[2]. At the same timeoptions
are passed toWritable->WritableState
[3] that expectsoptions.defaultEncoding
instead [4].For example this inconsistency reveals when you pass "wrong" encoding. Both following examples throw
TypeError: Unknown encoding: unknown
:and
But the following example doesn't throw anything:
I see there was a similar issue a while ago: nodejs/node-v0.x-archive#7710. So at least it's confusing.
[1] https://github.com/nodejs/node/blob/v6.x/lib/fs.js#L2059
[2] https://github.com/nodejs/node/blob/v6.x/lib/fs.js#L2100
[3] https://github.com/nodejs/node/blob/v6.x/lib/fs.js#L2077
[4] https://github.com/nodejs/node/blob/v6.x/lib/_stream_writable.js#L64
The text was updated successfully, but these errors were encountered: