-
Notifications
You must be signed in to change notification settings - Fork 663
feat(io): re-introduce IO functions #4128
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
Conversation
| * file.close(); | ||
| * ``` | ||
| */ | ||
| export async function readAll(reader: Reader): Promise<Uint8Array> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This function differs from readAll() from std/streams in that it doesn't depend on Buffer() from std/io. It's simpler, only relying on concat(). This should be a non-breaking change.
| * | ||
| * @example | ||
| * ```ts | ||
| * import { readAll } from "https://deno.land/std@$STD_VERSION/io/read_all.ts"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note: Buffer() is not imported here as it's deprecated.
| Deno.test("readAll()", async () => { | ||
| const testBytes = init(); | ||
| assert(testBytes); | ||
| const reader = new Buffer(testBytes.buffer); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We rely on Buffer() here but should work on removing that dependency in the future (in a PR).
| * const fileStream = toReadableStream(file); | ||
| * ``` | ||
| */ | ||
| export function toReadableStream( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note: renamed from readableStreamfFromReader().
| * .pipeTo(toWritableStream(file)); | ||
| * ``` | ||
| */ | ||
| export function toWritableStream( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note: renamed from writableStreamFromWriter().
| * {@linkcode ReadableStream.pipeTo} instead. | ||
| * @deprecated (will be removed in 0.214.0) Import from {@link https://deno.land/std/io/copy.ts} instead. | ||
| */ | ||
| export async function copy( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note: a copySync() could be implemented. But there's never been any demand for it. I also wonder whether there's demand for readAllSync() and writeAllSync().
We could also name this copyAll() to align it with readAll() and writeAll().
kt3k
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
This change re-introduces some of the previously deprecated and I/O-related functions and types. In short, these functions are moved from
std/streamstostd/io, which makes a lot more sense, in my opinion. These changes also include a few minor non-breaking (hopefully) tweaks.The refreshed
std/ioAPI aims to be as basic and minimal as possible while still covering the basic I/O modalities such as reading (readAll()), writing (writeAll()) and transferring (copy()), as well as conversion utilities to the Streams API's interfaces for more advanced use. E.g.TextLineStream(). There's an argument to be made that storage (Buffer) should be included in these primary modalities, but users may be fine without it.Closes #4120
Towards denoland/deno#9795