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

fs: add stream utilities to FileHandle #40009

Closed
wants to merge 6 commits into from

Conversation

aduh95
Copy link
Contributor

@aduh95 aduh95 commented Sep 5, 2021

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 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 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
lib/internal/fs/promises.js Outdated Show resolved Hide resolved
@aduh95 aduh95 added author ready PRs that have at least one approval, no pending requests for changes, and a CI started. request-ci Add this label to start a Jenkins CI on a PR. labels Sep 6, 2021
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Sep 6, 2021
@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

doc/api/fs.md Outdated Show resolved Hide resolved
doc/api/fs.md Outdated Show resolved Hide resolved
doc/api/fs.md Outdated Show resolved Hide resolved
doc/api/fs.md Outdated Show resolved Hide resolved
doc/api/fs.md Outdated Show resolved Hide resolved
doc/api/fs.md Outdated Show resolved Hide resolved
lib/internal/fs/promises.js Outdated Show resolved Hide resolved
lib/internal/fs/promises.js Outdated Show resolved Hide resolved
@aduh95 aduh95 removed the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Sep 6, 2021
* @param {{
* encoding?: string;
* autoClose?: boolean;
* emitClose?: boolean;
Copy link
Member

Choose a reason for hiding this comment

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

Can we remove autoClose & emitClose from here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Could you explain why?

Copy link
Member

Choose a reason for hiding this comment

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

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
Contributor Author

Choose a reason for hiding this comment

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

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
Member

Choose a reason for hiding this comment

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

Can we remove autoClose & emitClose from here?

@aduh95 aduh95 added author ready PRs that have at least one approval, no pending requests for changes, and a CI started. request-ci Add this label to start a Jenkins CI on a PR. labels Sep 16, 2021
@github-actions 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
Copy link
Collaborator

nodejs-github-bot commented Sep 16, 2021

@aduh95
Copy link
Contributor Author

aduh95 commented Sep 22, 2021

/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
Copy link
Member

jasnell commented Sep 25, 2021

Landed in 026bd82

@jasnell 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>
@danielleadams
Copy link
Member

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

@aduh95 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 5, 2021
PR-URL: nodejs#40009
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
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>
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 pending requests for changes, 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.

None yet

7 participants