-
-
Couldn't load subscription status.
- Fork 33.6k
v7.x backport: fs: support Uint8Array input to methods #10593
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
Conversation
Allow `fs.read`, `fs.write` and `fs.writeFile` to take `Uint8Array` arguments. PR-URL: nodejs#10382 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Italo A. Casas <me@italoacasas.com>
| 0, | ||
| expected.length, | ||
| 0, | ||
| common.mustCall((err, bytesRead) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you assert the error here?
| fs.closeSync(fd); | ||
|
|
||
| const found = fs.readFileSync(filename, 'utf8'); | ||
| assert.deepStrictEqual(expected.toString(), found); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Strings don't need deep comparison, right?
|
@thefourtheye I’ll be happy to address your suggestions, but I’d prefer to do that on |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@addaleax Sure, no problem. LGTM, if the CI is happy.
|
@evanlucas I guess whether you want to have this in #10589 is your call? I don’t want to step on anybody’s toes by landing this at the wrong time |
|
@addaleax feel free to land it. I was planning on bringing it any anyways :] |
Allow
fs.read,fs.writeandfs.writeFileto takeUint8Arrayarguments.PR-URL: #10382
The only difference from the original commit is the added line in
node_util.cc/cc @evanlucas