fix(std/node): fix readFile types, add encoding types (denoland/deno#6451)

Author: balupton

node/_fs/_fs_appendFile.ts

@@ -1,5 +1,6 @@
 // Copyright 2018-2020 the Deno authors. All rights reserved. MIT license.
 import {
+  Encodings,
   WriteFileOptions,
   isFileOptions,
   CallbackWithError,
@@ -15,13 +16,13 @@ import { fromFileUrl } from "../path.ts";
 export function appendFile(
   pathOrRid: string | number | URL,
   data: string,
-  optionsOrCallback: string | WriteFileOptions | CallbackWithError,
+  optionsOrCallback: Encodings | WriteFileOptions | CallbackWithError,
   callback?: CallbackWithError
 ): void {
   pathOrRid = pathOrRid instanceof URL ? fromFileUrl(pathOrRid) : pathOrRid;
   const callbackFn: CallbackWithError | undefined =
     optionsOrCallback instanceof Function ? optionsOrCallback : callback;
-  const options: string | WriteFileOptions | undefined =
+  const options: Encodings | WriteFileOptions | undefined =
     optionsOrCallback instanceof Function ? undefined : optionsOrCallback;
   if (!callbackFn) {
     throw new Error("No callback function supplied");
@@ -79,7 +80,7 @@ function closeRidIfNecessary(isPathString: boolean, rid: number): void {
 export function appendFileSync(
   pathOrRid: string | number | URL,
   data: string,
-  options?: string | WriteFileOptions
+  options?: Encodings | WriteFileOptions
 ): void {
   let rid = -1;
 
@@ -115,7 +116,7 @@ export function appendFileSync(
 }
 
 function validateEncoding(
-  encodingOption: string | WriteFileOptions | undefined
+  encodingOption: Encodings | WriteFileOptions | undefined
 ): void {
   if (!encodingOption) return;
 

node/_fs/_fs_appendFile_test.ts

@@ -23,6 +23,7 @@ Deno.test({
   fn() {
     assertThrows(
       () => {
+        // @ts-expect-error Type '"made-up-encoding"' is not assignable to type
         appendFile("some/path", "some data", "made-up-encoding", () => {});
       },
       Error,
@@ -33,6 +34,7 @@ Deno.test({
         appendFile(
           "some/path",
           "some data",
+          // @ts-expect-error Type '"made-up-encoding"' is not assignable to type
           { encoding: "made-up-encoding" },
           () => {}
         );
@@ -41,13 +43,15 @@ Deno.test({
       "Only 'utf8' encoding is currently supported"
     );
     assertThrows(
+      // @ts-expect-error Type '"made-up-encoding"' is not assignable to type
       () => appendFileSync("some/path", "some data", "made-up-encoding"),
       Error,
       "Only 'utf8' encoding is currently supported"
     );
     assertThrows(
       () =>
         appendFileSync("some/path", "some data", {
+          // @ts-expect-error Type '"made-up-encoding"' is not assignable to type
           encoding: "made-up-encoding",
         }),
       Error,

node/_fs/_fs_common.ts

@@ -4,11 +4,32 @@ import { notImplemented } from "../_utils.ts";
 
 export type CallbackWithError = (err?: Error | null) => void;
 
+export type TextEncodings =
+  | "ascii"
+  | "utf8"
+  | "utf-8"
+  | "utf16le"
+  | "ucs2"
+  | "ucs-2"
+  | "base64"
+  | "latin1"
+  | "hex";
+export type BinaryEncodings = "binary";
+export type Encodings = TextEncodings | BinaryEncodings;
+
 export interface FileOptions {
-  encoding?: string;
+  encoding?: Encodings;
   flag?: string;
 }
 
+export type TextOptionsArgument =
+  | TextEncodings
+  | ({ encoding: TextEncodings } & FileOptions);
+export type BinaryOptionsArgument =
+  | BinaryEncodings
+  | ({ encoding: BinaryEncodings } & FileOptions);
+export type FileOptionsArgument = Encodings | FileOptions;
+
 export interface WriteFileOptions extends FileOptions {
   mode?: number;
 }
@@ -26,8 +47,8 @@ export function isFileOptions(
 }
 
 export function getEncoding(
-  optOrCallback?: FileOptions | WriteFileOptions | Function | string
-): string | null {
+  optOrCallback?: FileOptions | WriteFileOptions | Function | Encodings | null
+): Encodings | null {
   if (!optOrCallback || typeof optOrCallback === "function") {
     return null;
   }
@@ -38,13 +59,15 @@ export function getEncoding(
   return encoding;
 }
 
-export function checkEncoding(encoding: string | null): string | null {
+export function checkEncoding(encoding: Encodings | null): Encodings | null {
   if (!encoding) return null;
   if (encoding === "utf8" || encoding === "utf-8") {
     return "utf8";
   }
-  if (encoding === "buffer") {
-    return "buffer";
+  if (encoding === "binary") {
+    return "binary";
+    // before this was buffer, however buffer is not used in Node
+    // node -e "require('fs').readFile('../world.txt', 'buffer', console.log)"
   }
 
   const notImplementedEncodings = [
@@ -53,7 +76,6 @@ export function checkEncoding(encoding: string | null): string | null {
     "base64",
     "hex",
     "ascii",
-    "binary",
     "ucs2",
   ];
 

node/_fs/_fs_readFile.ts

@@ -1,30 +1,58 @@
 // Copyright 2018-2020 the Deno authors. All rights reserved. MIT license.
-import { intoCallbackAPIWithIntercept, MaybeEmpty } from "../_utils.ts";
-import { getEncoding, FileOptions } from "./_fs_common.ts";
+import {
+  Encodings,
+  getEncoding,
+  FileOptionsArgument,
+  TextOptionsArgument,
+  BinaryOptionsArgument,
+  TextEncodings,
+  BinaryEncodings,
+} from "./_fs_common.ts";
 import { Buffer } from "../buffer.ts";
 import { fromFileUrl } from "../path.ts";
 
-type ReadFileCallback = (
-  err: MaybeEmpty<Error>,
-  data: MaybeEmpty<string | Buffer>
-) => void;
-
+function maybeDecode(data: Uint8Array, encoding: TextEncodings): string;
+function maybeDecode(
+  data: Uint8Array,
+  encoding: BinaryEncodings | null
+): Buffer;
 function maybeDecode(
   data: Uint8Array,
-  encoding: string | null
+  encoding: Encodings | null
 ): string | Buffer {
   const buffer = new Buffer(data.buffer, data.byteOffset, data.byteLength);
-  if (encoding) return buffer.toString(encoding);
+  if (encoding && encoding !== "binary") return buffer.toString(encoding);
   return buffer;
 }
 
+type TextCallback = (err: Error | null, data?: string) => void;
+type BinaryCallback = (err: Error | null, data?: Buffer) => void;
+type GenericCallback = (err: Error | null, data?: string | Buffer) => void;
+type Callback = TextCallback | BinaryCallback | GenericCallback;
+
+export function readFile(
+  path: string | URL,
+  options: TextOptionsArgument,
+  callback: TextCallback
+): void;
+export function readFile(
+  path: string | URL,
+  options: BinaryOptionsArgument,
+  callback: BinaryCallback
+): void;
+export function readFile(
+  path: string | URL,
+  options: null | undefined | FileOptionsArgument,
+  callback: BinaryCallback
+): void;
+export function readFile(path: string | URL, callback: BinaryCallback): void;
 export function readFile(
   path: string | URL,
-  optOrCallback: ReadFileCallback | FileOptions | string | undefined,
-  callback?: ReadFileCallback
+  optOrCallback?: FileOptionsArgument | Callback | null | undefined,
+  callback?: Callback
 ): void {
   path = path instanceof URL ? fromFileUrl(path) : path;
-  let cb: ReadFileCallback | undefined;
+  let cb: Callback | undefined;
   if (typeof optOrCallback === "function") {
     cb = optOrCallback;
   } else {
@@ -33,18 +61,39 @@ export function readFile(
 
   const encoding = getEncoding(optOrCallback);
 
-  intoCallbackAPIWithIntercept<Uint8Array, string | Buffer>(
-    Deno.readFile,
-    (data: Uint8Array): string | Buffer => maybeDecode(data, encoding),
-    cb,
-    path
-  );
+  const p = Deno.readFile(path);
+
+  if (cb) {
+    p.then((data: Uint8Array) => {
+      if (encoding && encoding !== "binary") {
+        const text = maybeDecode(data, encoding);
+        return (cb as TextCallback)(null, text);
+      }
+      const buffer = maybeDecode(data, encoding);
+      (cb as BinaryCallback)(null, buffer);
+    }).catch((err) => cb && cb(err));
+  }
 }
 
 export function readFileSync(
   path: string | URL,
-  opt?: FileOptions | string
+  opt: TextOptionsArgument
+): string;
+export function readFileSync(
+  path: string | URL,
+  opt?: BinaryOptionsArgument
+): Buffer;
+export function readFileSync(
+  path: string | URL,
+  opt?: FileOptionsArgument
 ): string | Buffer {
   path = path instanceof URL ? fromFileUrl(path) : path;
-  return maybeDecode(Deno.readFileSync(path), getEncoding(opt));
+  const data = Deno.readFileSync(path);
+  const encoding = getEncoding(opt);
+  if (encoding && encoding !== "binary") {
+    const text = maybeDecode(data, encoding);
+    return text;
+  }
+  const buffer = maybeDecode(data, encoding);
+  return buffer;
 }

node/_fs/_fs_writeFile.ts

@@ -3,6 +3,7 @@ import { notImplemented } from "../_utils.ts";
 import { fromFileUrl } from "../path.ts";
 
 import {
+  Encodings,
   WriteFileOptions,
   CallbackWithError,
   isFileOptions,
@@ -14,12 +15,12 @@ import {
 export function writeFile(
   pathOrRid: string | number | URL,
   data: string | Uint8Array,
-  optOrCallback: string | CallbackWithError | WriteFileOptions | undefined,
+  optOrCallback: Encodings | CallbackWithError | WriteFileOptions | undefined,
   callback?: CallbackWithError
 ): void {
   const callbackFn: CallbackWithError | undefined =
     optOrCallback instanceof Function ? optOrCallback : callback;
-  const options: string | WriteFileOptions | undefined =
+  const options: Encodings | WriteFileOptions | undefined =
     optOrCallback instanceof Function ? undefined : optOrCallback;
 
   if (!callbackFn) {
@@ -71,7 +72,7 @@ export function writeFile(
 export function writeFileSync(
   pathOrRid: string | number | URL,
   data: string | Uint8Array,
-  options?: string | WriteFileOptions
+  options?: Encodings | WriteFileOptions
 ): void {
   pathOrRid = pathOrRid instanceof URL ? fromFileUrl(pathOrRid) : pathOrRid;
 

node/_fs/_fs_writeFile_test.ts

@@ -24,6 +24,7 @@ Deno.test("Callback must be a function error", function fn() {
 Deno.test("Invalid encoding results in error()", function testEncodingErrors() {
   assertThrows(
     () => {
+      // @ts-expect-error Type '"made-up-encoding"' is not assignable to type
       writeFile("some/path", "some data", "made-up-encoding", () => {});
     },
     Error,
@@ -32,6 +33,7 @@ Deno.test("Invalid encoding results in error()", function testEncodingErrors() {
 
   assertThrows(
     () => {
+      // @ts-expect-error Type '"made-up-encoding"' is not assignable to type
       writeFileSync("some/path", "some data", "made-up-encoding");
     },
     Error,
@@ -44,6 +46,7 @@ Deno.test("Invalid encoding results in error()", function testEncodingErrors() {
         "some/path",
         "some data",
         {
+          // @ts-expect-error Type '"made-up-encoding"' is not assignable to type
           encoding: "made-up-encoding",
         },
         () => {}
@@ -56,6 +59,7 @@ Deno.test("Invalid encoding results in error()", function testEncodingErrors() {
   assertThrows(
     () => {
       writeFileSync("some/path", "some data", {
+        // @ts-expect-error Type '"made-up-encoding"' is not assignable to type
         encoding: "made-up-encoding",
       });
     },

node/_fs/promises/_fs_readFile.ts

@@ -1,16 +1,28 @@
 // Copyright 2018-2020 the Deno authors. All rights reserved. MIT license.
-import { FileOptions } from "../_fs_common.ts";
-import { MaybeEmpty } from "../../_utils.ts";
-
+import {
+  FileOptionsArgument,
+  BinaryOptionsArgument,
+  TextOptionsArgument,
+} from "../_fs_common.ts";
 import { readFile as readFileCallback } from "../_fs_readFile.ts";
 
 export function readFile(
   path: string | URL,
-  options?: FileOptions | string
-): Promise<MaybeEmpty<string | Uint8Array>> {
+  options: TextOptionsArgument
+): Promise<string>;
+export function readFile(
+  path: string | URL,
+  options?: BinaryOptionsArgument
+): Promise<Uint8Array>;
+export function readFile(
+  path: string | URL,
+  options?: FileOptionsArgument
+): 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);
     });
   });