Node: fs: promises have undocumented write(string,…) method

[ChALkeR]

• Version: 10.0.0, master
• Subsystem: fs

Non-promises API has two variants documented: Buffer, string.

Promises API has only Buffer variants documented: method, class method.

This code works, but it's undocumented behavior:

const fs = require('fs/promises');
const fd = await fs.open('temp.txt', 'w+');
fd.write('test\n');

[ChALkeR]

Not sure if this is a documentation issue. Should we keep that API in such form?

[BridgeAR]

Without looking into it further: as far as I know the promises API should be identical to the non-promise one besides returning promises. So it sounds like a documentation issue.

[vsemozhetbyt]

Refs: https://github.com/nodejs/node/pull/18297#issuecomment-359625914 and next comment:

Yes, the omission of the fs.write(fd, string, position, encoding, callback) variant is intentional.

so cc @jasnell

[ChALkeR]

It's undocumented, but present and _partially_ broken — see #20407.

[ChALkeR]

Note that according to coverage report, that variant is called once, which means that there is a test that depends on it being present.

[jasnell]

Doc omission. The variant should be doc'd.

[jasnell]

It was a to-do that I never went back to. Although if I recall correctly there may need to be some reconcilation still with the non-promise version

[jasnell]

@BridgeAR ... To be certain the promise certain is not identical. There are intended differences... Such as the use of the FileHandle object rather than numeric fd.

[BridgeAR]

That does not sound intuitive to me at all. I would have expected it is fine to switch to promises 1-to-1. Can you outline the specific differences / where there a lot of these?

[ChALkeR]

@BridgeAR The ones I noticed:

1. callbacks → promises
2. fd → filehandle
3. constants not there (fs.constants, fs.*_OK),
4. classes not there (fs.Stats, fs.WriteStream, etc.),
5. some methods are not there, see #20548 — close, *Stream, *watch*, *Sync, exists. #20559 should remove even more.
6. return values is different

Example:

> await (await fsp.open('test.txt', 'w')).write('test')
{ bytesWritten: 4, buffer: 'test' }
> fs.writeSync(fs.openSync('test.txt', 'w'), 'test')
4

[BridgeAR]

@ChALkeR thanks for pointing these things out.

[ChALkeR]

@BridgeAR I actually think that we could merge fs.promises into fs, after removing all fd-using methods from fs.promises. It shouldn't be that hard, though different return values might confuse people a bit, but I don't expect that to be a large problem with proper documentation.

[jasnell]

Big -1 on merging the APIs. Polymorphic returns are awful and it's not something we can do consistently across the core API. fs.promises should remain a separate API path.

[tjcrowder]

@jasnell - Currently, the implementation of filehandle.write doesn't accept an (optional) encoding parameter, so is this a documentation-only issue or does it need the param (and the attendant shuffling around of params)? FWIW, since this is a new API, my complete-outsider view would be don't bother with an encoding parameter, just say (and ensure) that if you provide a string, UTF-8 will be used, and if you want something else provide a buffer instead. :-) (I'm happy to take this issue and do a PR if it's doc-only, or perhaps even if adding the param is required if I can ping you with the occasional question as I ramp up.)

[ChALkeR]

@nodejs/collaborators :+1: for good-first-issue (doc omission for .write(string).

[darahayes]

@ChALkeR @jasnell Hey folks, I just submitted a docs PR for this issue. I hope it helps in some way. I've raised some questions in there. Cheers!