Deno: Deno.readFileSync should accept an option to return a utf8 encoded string instead of Uint8Array

[ry]

(same for Deno.readFile, Deno.writeFile, Deno.writeFileSync)

[shortdiv]

I'd like to take a crack at this one! ✋

[crabmusket]

Suggestion: should it accept a string like utfLabel of the TextDecoder constructor? E.g.

Deno.readFileSync("./blah", "utf8");

[lucacasonato]

Isn't this what std is for? https://doc.deno.land/https/deno.land/std/fs/mod.ts#readFileStrSync provides this exact functionality. Shouldn't the Deno bindings be minimal?

[shortdiv]

It seems that there's a current encoding for utf8 in std, so you could do something like:

Deno.readFileSync("./blah", { encoding: "utf8" });

[hayd]

👍 to using std.

Returning mixed types seems like a bad idea.

[ry]

This is a common enough task, and simple enough to implement, that it seems unnecessary to not include it.

TypeScript deals fine with mixed return types - we have several places where we overload function signatures in the runtime. For example, Deno.listen

https://doc.deno.land/https/github.com/denoland/deno/releases/latest/download/lib.deno.d.ts#Deno.listen

[hayd]

What's the benefit of overloading over simply moving readFileStr etc. to Deno namespace?

Defining mixed types is fine, it's using them that's a problem (you'd need to assert type after use?)... which is more than the 3 chars you save by not writing Str.

[Caesar2011]

You are doubling the amount of functions instead of using the existing ones. The existing functions are doing the correct thing, but with a different output.

It is also simpler to add more outputs than utf8 if needed. Base64 for example...

[kitsonk]

Just a note, it should accept an options bag, instead of a single option... there are a lot of times where you think "oh, I will only ever need a single option" and then it goes horribly wrong. It is something that can easily be sorted out in overloads, to augment the return type in TypeScript, and I agree with Ry, it is a very very common pattern. Forcing people to use std or their own text decoder is mean. It does mean some std stuff would become redundant, but hey, that is life.

[hayd]

So I am mistaken that you would have to assert type guard afterwards for typescript to be happy? That was my only concern.

[nayeemrmn]

It seems that there's a current encoding for utf8 in std, so you could do something like:

Deno.readFileSync("./blah", { encoding: "utf8" });

It should be Deno.readFileSync("./blah", { decode: "utf8" }) to make it clear that you're adding a decode step... specifying that the binary is utf8-encoded doesn't responsibly imply that.

@hayd You can write each overload with a particular argument type mapped to a particular return type, so if TS knows the former it can infer the latter without type guards. I think.

--

I still prefer just adding Deno.readFileString() and Deno.writeFileString(). Using options to append type changing steps to a function just isn't logical, in that sense they're still different functions sharing one name. Even if TS handles it well, for JS it's slightly more error-prone. We should use namespaces to group them if that's important.

[kitsonk]

@hayd You can write each overload with a particular argument type mapped to a particular return type, so if TS knows the former it can infer the latter without type guards. I think.

Correct... you would write an overload like:

namespace Deno {
  readFileSync(path: string, options: { encoding: "utf8" }): string;
  readFileSync(path: string, options?: {}): Uint8Array;
}

Or something like that... there are quite a lot of these types of overloads in the TypeScript lib, especially when creating HTML elements.

[nayeemrmn]

We could also patch the byte array with decode() or any other convenient method:

namespace Deno {
  readFileSync(path: string): Uint8Array & { decode(utfLabel?: string): string }
}

Deno.readFileSync("temp.js") // Uint8Array+
Deno.readFileSync("temp.js").decode() // string

More representative of a piped decode step, more naturally extensible (?), more ergonomic (that's the point, right?).

[kitsonk]

Monkey patching built ins... 😦 I personally thing we should avoid adding sugar to something that is built in. It leads to surprises.

[bartlomieju]

Implemented as Deno.readTextFile() and Deno.readTextFileSync()