Skip to content
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

Add support for reading/writing with multiple encodings #441

Closed
Tracked by #501
icidasset opened this issue Dec 13, 2022 · 5 comments
Closed
Tracked by #501

Add support for reading/writing with multiple encodings #441

icidasset opened this issue Dec 13, 2022 · 5 comments
Labels
enhancement New feature or request

Comments

@icidasset
Copy link
Contributor

icidasset commented Dec 13, 2022

With Webnative 0.35 you can only read and write Uint8Arrays.
There should be support for other encoding, such as UTF8 strings.

@matheus23 suggested the encoding option from https://nodejs.org/api/fs.html#filehandlereadfileoptions

We could do something like:

fs.write({ bytes: Uint8Array })
fs.write({ text: "Text", encoding: "utf8" })
@icidasset icidasset added the enhancement New feature or request label Dec 13, 2022
@matheus23
Copy link
Contributor

This may be a duplicate of #269 now.

I'm biased, but I'd vouch for the 2 positional parameters and an optional third "options" parameter that contains the { encoding: "utf8" } object (as I've suggested in that issue).
For read this looks like: fs.read(path, { encoding: "utf8" }).
This means it's less of a breaking change & it's potentially more familiar to JS developers.

For write, I feel like we have the option of "auto-detecting" the encoding. I.e. if a string is passed, we assume UTF8.
However, that may not be ideal: It may be surprising that .write accepts a string but subsequently returns a Uint8Array if the same file is .read.
So perhaps, it'd make sense to only allow passing a string if the encoding parameter is explicitly set to UTF8.

So, personally I'd be in favor of these types:

fs.read(path: FilePath): Promise<Uint8Array>
fs.read(path: FilePath, options: { encoding: "utf8" }): Promise<string>

fs.write(path: FilePath, bytes: Uint8Array)
fs.write(path: FilePath, text: string, options: { encoding: "utf8" })

@icidasset
Copy link
Contributor Author

Oh whoops, forgot about that issue.

fs.write(path: FilePath, bytes: Uint8Array)
fs.write(path: FilePath, text: string, options: { encoding: "utf8" })

That works too yeah, I have no strong preference.

So perhaps, it'd make sense to only allow passing a string if the encoding parameter is explicitly set to UTF8.

Yes exactly, that's what I was trying to achieve as well. I assumed it'd be easier to do in Typescript with my syntax, rather than yours, but I could be wrong. But yeah, your syntax is more familiar.

@matheus23
Copy link
Contributor

matheus23 commented Dec 13, 2022

Yes exactly, that's what I was trying to achieve as well. I assumed it'd be easier to do in Typescript with my syntax, rather than yours, but I could be wrong. But yeah, your syntax is more familiar.

You are correct about that version being harder, but it's still possible. Here's how node.js defines the readFile type:
https://github.com/DefinitelyTyped/DefinitelyTyped/blob/4805c8405fb64154def301a763c40328ccfb612c/types/node/fs.d.ts#L2532-L2554

Essentially it'd boil down to function type overloading. Something like this in our case:

export function read(path: FilePath): Promise<Uint8Array>;
export function read(path: FilePath, options: { encoding: "utf8" }): Promise<string>;
export function read(path: FilePath, options?: { encoding?: string }): Promise<Uint8Array | string> {
  // impl
}

@icidasset
Copy link
Contributor Author

Oh you can do function overloading in Typescript?! How did I not know this 😲
Ok yeah, that's better 👍

@icidasset
Copy link
Contributor Author

Implemented in #500
Part of #501

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants