bug(std/node): readFile needs generics for correct types

[balupton]

There is an incompatibility on https://deno.land/std/node/_fs/_fs_readFile.ts

prep

echo 'hello' > 'world.txt'

deno

deno eval "import { promises as fsPromises } from 'https://deno.land/std/node/fs.ts'; const str: string = await fsPromises.readFile('./world.txt', 'utf8'); console.log(str)"

Compile file:///Users/balupton/Projects/active/valid-module/__$deno$eval.ts
error: TS2322 [ERROR]: Type 'MaybeEmpty<string | Uint8Array>' is not assignable to type 'string'.
  Type 'undefined' is not assignable to type 'string'.
import { promises as fsPromises } from 'https://deno.land/std/node/fs.ts'; const str: string = await fsPromises.readFile('./package.json', 'utf8');
                                                                                 ~~~
    at file:///Users/balupton/Projects/active/valid-module/__$deno$eval.ts:1:82

node

node --input-type=module -e "import { promises as fsPromises } from 'fs'; (async () => { const str = await fsPromises.readFile('./world.txt', 'utf8'); console.log(str) })()"

evaluation

Problem seems fixable by overloading readFile where instead of err, and data being maybe empty, they are conditionally empty based on the non-presence of the other.

However, it may be simpler to just use the Node types:
https://github.com/DefinitelyTyped/DefinitelyTyped/blob/64ca7c2a2947369edda83b1e8e1235a116ab352a/types/node/fs/promises.d.ts#L67-L89
https://github.com/DefinitelyTyped/DefinitelyTyped/blob/64ca7c2a2947369edda83b1e8e1235a116ab352a/types/node/fs.d.ts#L1450-L1467

Which it seems always provides a string or Buffer for data - however, the types could be wrong for Node.js, testing now.

Yeah, the types are wrong for Node.js:

> node --input-type=module -e "import { readFile } from 'fs'; readFile('aasdasdad', (...args) => console.log(args))"
[
  [Error: ENOENT: no such file or directory, open 'aasdasdad'] {
    errno: -2,
    code: 'ENOENT',
    syscall: 'open',
    path: 'aasdasdad'
  }
]

However, there is still a need to fix this for Deno. In short:

1. callback has error: node.js is wrong, deno is right; i.e. there should be no data argument provided to the callback
2. otherwise callback has data: error should be empty, node.js is right, deno is wrong

Deno is wrong on item 2, because it thinks there could be a 3rd case, where both error, and data are empty.

---

Ignoring the incorrect readFile for now, which seems to be too complicated to resolve, instead one can focus on the more simple promises/readFile

I came up with:

// Copyright 2018-2020 the Deno authors. All rights reserved. MIT license.
import { FileOptions } from "../_fs_common.ts";
import { MaybeEmpty } from "../../_utils.ts";

import { readFile as readFileCallback } from "../_fs_readFile.ts";

export function readFile(
  path: string | URL,
  options?: FileOptions | string
): Promise<string | Uint8Array> {
  return new Promise((resolve, reject) => {
    readFileCallback(path, options, (err, data): void => {
      if (err) return reject(err);
      if (data == null)
        return reject(new Error("Invalid state: data missing, but no error"));
      resolve(data);
    });
  });
}

However it still fails with:

Compile file:///Users/balupton/Projects/active/valid-module/__$deno$eval.ts
error: TS2322 [ERROR]: Type 'string | Uint8Array' is not assignable to type 'string'.
  Type 'Uint8Array' is not assignable to type 'string'.
    at file:///Users/balupton/Projects/active/valid-module/__$deno$eval.ts:1:72

So there also needs to be type conditionals on the return type based on the input encoding.