Skip to content
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

fsPromises.readFile ignores encoding option #19286

Closed
Legends opened this issue Mar 11, 2018 · 4 comments
Closed

fsPromises.readFile ignores encoding option #19286

Legends opened this issue Mar 11, 2018 · 4 comments
Assignees
Labels
confirmed-bug Issues with confirmed bugs. fs Issues and PRs related to the fs subsystem / file system. promises Issues and PRs related to ECMAScript promises.

Comments

@Legends
Copy link

Legends commented Mar 11, 2018

  • Version: v10.0.0-nightly20180309099e621648
  • Platform: Win10 x64
  • Subsystem: ?
var fs = require('fs/promises'); 
fs.readFile(path,"utf8").then((data)=>...

data will be of type string, when specifying the encoding in the method call, but with the latest version (v10.0.0-nightly20180309099e621648) I have to data.toString("utf8") again, to get the result as string.

By design?

@vsemozhetbyt vsemozhetbyt added fs Issues and PRs related to the fs subsystem / file system. promises Issues and PRs related to ECMAScript promises. labels Mar 11, 2018
@benjamingr
Copy link
Member

Looks like a bug to me, fs.readFileSync returns a string for the same call.

@benjamingr
Copy link
Member

benjamingr commented Mar 12, 2018

Can reproduce locally

> await fsPromises.readFile('v8_build_config.json', 'utf-8')
<Buffer 7b 22 76 38 5f 74 61 72 67 65 74 5f 63 70 75 22 3a 20 22 78 36 34 22 2c 20 22 69 73 5f 63 66 69 22 3a 20 66 61 6c 73 65 2c 20 22 74 61 72 67 65 74 5f ... >
> fs.readFileSync('v8_build_config.json', 'utf-8')
'{"v8_target_cpu": "x64", "is_cfi": false, "target_cpu": "x64", "is_ubsan_vptr": false, "is_gcov_coverage": false, "dcheck_always_on": false, "is_component_build": false, "is_asan": false, "is_debug": false, "v8_enable_i18n_support": true, "is_msan": false, "is_tsan": false, "v8_use_snapshot": true, "v8_enable_verify_predictable": false}'

Note this works correctly when calling util.promisify on fs.readFile

@benjamingr benjamingr added the confirmed-bug Issues with confirmed bugs. label Mar 12, 2018
@benjamingr
Copy link
Member

Looks like the encoding handling is missing in fs/promises and it is simply ignored https://github.com/nodejs/node/blob/master/lib/fs.js#L403-L405

https://github.com/nodejs/node/blob/master/lib/fs/promises.js#L132 ignores options passed in.

@benjamingr benjamingr self-assigned this Mar 12, 2018
@benjamingr
Copy link
Member

Going to tackle this one

@benjamingr benjamingr changed the title Question: require('fs/promises') fsPromises.readFile ignores encoding Mar 12, 2018
@benjamingr benjamingr changed the title fsPromises.readFile ignores encoding fsPromises.readFile ignores encoding option Mar 12, 2018
benjamingr pushed a commit to benjamingr/io.js that referenced this issue Mar 12, 2018
Use the encoding parameter passed to fsPromises.readFile if it is
passed. Currently the encoding parameter is ignored in fsPromises

PR-URL: nodejs#19296
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Fixes: nodejs#19286
@lpinca lpinca closed this as completed in e06ad5f Mar 24, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
confirmed-bug Issues with confirmed bugs. fs Issues and PRs related to the fs subsystem / file system. promises Issues and PRs related to ECMAScript promises.
Projects
None yet
Development

No branches or pull requests

3 participants