FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

fs: add stream utilities to `FileHandle` by aduh95 · Pull Request #40009 · nodejs/node · GitHub

/ node Public

fs: add stream utilities to FileHandle - #40009

Closed
aduh95 wants to merge 6 commits into
nodejs:masterfrom
aduh95:fs-fh-nodejs-streams
Closed

fs: add stream utilities to FileHandle#40009
aduh95 wants to merge 6 commits into
nodejs:masterfrom
aduh95:fs-fh-nodejs-streams

Conversation

aduh95 commented Sep 5, 2021
edited
Loading

Copy link
Copy Markdown
Contributor

Adding filehandle.createReadStream and filehandle.createWriteStream methods to FileHandle class. This is useful to be able create ReadStream or WriteStream from fs/promises.

import { open } from 'node:fs/promises';
import { stdout } from 'node:process';

try {
  const textFile = await open(new URL('./file.txt', import.meta.url));
  textFile.createReadStream().pipe(stdout);
} catch (err) {
  // error while opening the file
  // ...
}

It also accept the same functions as fs.createReadStream and fs.createWriteStream respectively:

import { open } from 'node:fs/promises';
import { stdout } from 'node:process';
import { finished } from 'node:stream/promises';

let textFile;
try {
  textFile = await open(new URL('./file.txt', import.meta.url));
  await finished(textFile.createReadStream({ autoClose: false, encoding: 'utf8' }).pipe(stdout));
} catch (err) {
  // error while opening the file or while streaming the file
  // ...
} finally {
  await textFile?.close();
}

aduh95 added the semver-minor PRs that contain new features and should be released in the next minor version. label Sep 5, 2021
nodejs-github-bot added fs Issues and PRs related to the fs subsystem / file system. needs-ci PRs that need a full CI run. labels Sep 5, 2021
Comment thread lib/internal/fs/promises.js Outdated
aduh95 added author ready PRs that have at least one approval, no outstanding review comments, and a CI started. request-ci Add this label to start a Jenkins CI on a PR. labels Sep 6, 2021
github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Sep 6, 2021

Copy link
Copy Markdown
Collaborator

Copy link
Copy Markdown
Collaborator

Comment thread doc/api/fs.md Outdated
Comment thread doc/api/fs.md Outdated
Comment thread doc/api/fs.md Outdated
Comment thread doc/api/fs.md Outdated
Comment thread doc/api/fs.md Outdated
Comment thread doc/api/fs.md Outdated
Comment thread lib/internal/fs/promises.js Outdated
Comment thread lib/internal/fs/promises.js Outdated
aduh95 removed the author ready PRs that have at least one approval, no outstanding review comments, and a CI started. label Sep 6, 2021
aduh95 force-pushed the fs-fh-nodejs-streams branch from 207df82 to ab59886 Compare September 8, 2021 10:28
* @param {{
* encoding?: string;
* autoClose?: boolean;
* emitClose?: boolean;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Can we remove autoClose & emitClose from here?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Could you explain why?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

They are legacy. The only reason we have still have them is to avoid breakage. Since this is a new API they are not necessary.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

It’s a new way for exposing the same old API, removing it would be harder than keeping it – also I don’t think they’re documented as legacy anywhere, so that’s probably best to leave that discussion for another PR.

* @param {{
* encoding?: string;
* autoClose?: boolean;
* emitClose?: boolean;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Can we remove autoClose & emitClose from here?

aduh95 added author ready PRs that have at least one approval, no outstanding review comments, and a CI started. request-ci Add this label to start a Jenkins CI on a PR. labels Sep 16, 2021
github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Sep 16, 2021

nodejs-github-bot commented Sep 16, 2021
edited by jasnell
Loading

Copy link
Copy Markdown
Collaborator

aduh95 commented Sep 22, 2021

Copy link
Copy Markdown
Contributor Author

/cc @nodejs/fs

jasnell pushed a commit that referenced this pull request Sep 25, 2021
PR-URL: #40009
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>

jasnell commented Sep 25, 2021

Copy link
Copy Markdown
Member

Landed in 026bd82

jasnell closed this Sep 25, 2021
danielleadams pushed a commit that referenced this pull request Oct 4, 2021
PR-URL: #40009
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>

Copy link
Copy Markdown
Contributor

@aduh95 do you mind opening a backport PR to v16.x-staging? The PR broke the lint-js script. Thanks!

aduh95 deleted the fs-fh-nodejs-streams branch October 5, 2021 08:01
aduh95 added a commit to aduh95/node that referenced this pull request Oct 6, 2021
PR-URL: nodejs#40009
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
danielleadams pushed a commit that referenced this pull request Oct 7, 2021
PR-URL: #40009
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>

Backport-PR-URL: #40329
Reviewed-By: Danielle Adams <adamzdanielle@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

author ready PRs that have at least one approval, no outstanding review comments, and a CI started. fs Issues and PRs related to the fs subsystem / file system. needs-ci PRs that need a full CI run. semver-minor PRs that contain new features and should be released in the next minor version.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants


Back | FazBrowse Home | New Git URL