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

BufReader's interface is un-idiomatic javascript. #436

Closed
piscisaureus opened this issue May 22, 2019 · 1 comment
Closed

BufReader's interface is un-idiomatic javascript. #436

piscisaureus opened this issue May 22, 2019 · 1 comment

Comments

@piscisaureus
Copy link
Member

piscisaureus commented May 22, 2019

Currently BufReader.readLine(), readSlice(), peek() etc return a Promise resolving to a [result, BufState] tuple, where BufState is non-null if an error condition has occurred.

The exact definition of BufState is:

export type BufState =
  | null
  | "EOF"
  | "BufferFull"
  | "ShortWrite"
  | "NoProgress"
  | Error;

(Note that some of these BufState values aren't applicable to BufReaders; they're only applicable to BufWriters.)

This may be idiomatic in go (which doesn't have exceptions), but it's highly un-idiomatic in javascript.

I'd propose to change these interfaces as such:

  • These functions return a Promise<T | null>
  • On actual errors, the promise is rejected with an actual Error object.
  • On EOF, the promise is resolved with null.
  • Otherwise, the promise is resolved with just an Uint8Array or a string.
  • The BufState type is made private or removed entirely.

cc @ry

@ry
Copy link
Member

ry commented May 23, 2019

SGTM

piscisaureus added a commit to piscisaureus/deno_std that referenced this issue May 29, 2019
io: refactor BufReader/Writer interfaces to be more idiomatic

Thanks Vincent Le Gofff (@zekth) for porting over the CSV
implementation.

Fixes: denoland#436
piscisaureus added a commit to piscisaureus/deno_std that referenced this issue May 29, 2019
io: refactor BufReader/Writer interfaces to be more idiomatic (denoland#444)

Thanks Vincent Le Gofff (@zekth) for porting over the CSV
implementation.

Fixes: denoland#436
piscisaureus added a commit to piscisaureus/deno_std that referenced this issue May 29, 2019
io: refactor BufReader/Writer interfaces to be more idiomatic (denoland#444)

Thanks Vincent Le Goff (@zekth) for porting over the CSV reader
implementation.

Fixes: denoland#436
ry pushed a commit to ry/deno that referenced this issue Oct 9, 2019
…nd/std#444)

Thanks Vincent Le Goff (@zekth) for porting over the CSV reader
implementation.

Fixes: denoland/std#436

Original: denoland/std@0ee6334
caspervonb pushed a commit to caspervonb/deno_std that referenced this issue Jan 24, 2021
…nd#444)

Thanks Vincent Le Goff (@zekth) for porting over the CSV reader
implementation.

Fixes: denoland#436
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants