fs: support caller-supplied readFile() buffers by mcollina · Pull Request #63634 · nodejs/node

mcollina · 2026-05-29T08:22:28Z

PR-URL: nodejs#63634 Signed-off-by: Matteo Collina <hello@matteocollina.com>

PR-URL: nodejs#63634 Signed-off-by: Matteo Collina <matteo.collina@gmail.com>

ShogunPanda

LGTM!

Renegade334 · 2026-05-29T08:54:42Z

+  * `buffer` {Buffer|TypedArray|DataView} A buffer to read into.
+  * `getBuffer` {Function} A synchronous function called with the file size and
+    returning the buffer to read into.


Could we not consolidate these two? Plenty of places across the API surface have options with "either a thing or a thing-like function" types.

Agreed. It should be quite cheap to check typeof options.buffer === 'function' at the start.

Renegade334 · 2026-05-29T08:57:14Z

  }
 });

+const validateReadFileBufferOptions = hideStackFrames((options) => {


I would personally advocate for this to throw if both buffer and encoding are provided – directly contradictory patterns should fail validation.

codecov · 2026-05-29T09:50:00Z

Codecov Report

❌ Patch coverage is 96.32353% with 10 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.35%. Comparing base (8d0a3b8) to head (e8edd28).
⚠️ Report is 89 commits behind head on main.

Files with missing lines	Patch %	Lines
lib/internal/fs/promises.js	93.10%	4 Missing ⚠️
lib/fs.js	95.89%	3 Missing ⚠️
lib/internal/fs/read/context.js	95.94%	3 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #63634      +/-   ##
==========================================
+ Coverage   90.29%   90.35%   +0.06%     
==========================================
  Files         730      732       +2     
  Lines      234782   236689    +1907     
  Branches    43953    44582     +629     
==========================================
+ Hits       211993   213862    +1869     
- Misses      14501    14561      +60     
+ Partials     8288     8266      -22

Files with missing lines	Coverage Δ
lib/internal/fs/utils.js	`98.53% <100.00%> (+0.09%)`	⬆️
lib/fs.js	`98.36% <95.89%> (+0.16%)`	⬆️
lib/internal/fs/read/context.js	`94.92% <95.94%> (+0.35%)`	⬆️
lib/internal/fs/promises.js	`92.94% <93.10%> (+0.12%)`	⬆️

... and 65 files with indirect coverage changes

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

PR-URL: nodejs#63634 Signed-off-by: Matteo Collina <matteo.collina@gmail.com>

nodejs-github-bot · 2026-06-01T10:58:54Z

CI: https://ci.nodejs.org/job/node-test-pull-request/73857/

- Remove getBuffer deprecation check from validateReadFileBufferOptions (getBuffer was never released, dead code) - Simplify buffer validation using isArrayBufferView check - Consolidate buffer option description in docs: 'synchronous function called with the file size and returning the buffer to read into' -> 'function called with the file size that returns the buffer' - Add examples for pre-allocated buffer and function variants Co-authored-by: LiviaMedeiros <livia@example.com> Co-authored-by: Renegade334 <renegade334@example.com> Co-authored-by: MattiasBuelens <mattias@example.com> Co-authored-by: jasnell <jasnell@example.com> PR-URL: nodejs#63634 Signed-off-by: Matteo Collina <hello@matteocollina.com>

The getBuffer validation was removed from validateReadFileBufferOptions since getBuffer was never released. Remove the test cases that expected ERR_INVALID_ARG_VALUE for getBuffer. PR-URL: nodejs#63634 Signed-off-by: matteo <matteo@example.com>

PR-URL: nodejs#63634 Signed-off-by: matteo <matteo@example.com>

mcollina · 2026-06-11T15:08:46Z

@LiviaMedeiros @jasnell @MattiasBuelens @ronag PTAL

ShogunPanda

LGTM!

nodejs-github-bot · 2026-06-11T16:32:30Z

CI: https://ci.nodejs.org/job/node-test-pull-request/74060/

nodejs-github-bot · 2026-06-11T18:23:11Z

CI: https://ci.nodejs.org/job/node-test-pull-request/74061/

nodejs-github-bot · 2026-06-12T05:53:54Z

CI: https://ci.nodejs.org/job/node-test-pull-request/74067/

nodejs-github-bot · 2026-06-13T09:01:59Z

Commit Queue failed

- Loading data for nodejs/node/pull/63634 ✔ Done loading data for nodejs/node/pull/63634 ----------------------------------- PR info ------------------------------------ Title fs: support caller-supplied readFile() buffers (#63634) Author Matteo Collina <matteo.collina@gmail.com> (@mcollina) Branch mcollina:fs-readfile-buffer-option -> nodejs:main Labels fs, semver-minor, needs-ci Commits 4 - fs: support caller-supplied readFile() buffers - fs: address PR review comments - test: remove getBuffer tests - doc: add Buffer import to examples to satisfy lint Committers 1 - Matteo Collina <hello@matteocollina.com> PR-URL: https://github.com/nodejs/node/pull/63634 Fixes: https://github.com/nodejs/node/issues/63600 Reviewed-By: Paolo Insogna <paolo@cowtech.it> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: LiviaMedeiros <livia@cirno.name> ------------------------------ Generated metadata ------------------------------ PR-URL: https://github.com/nodejs/node/pull/63634 Fixes: https://github.com/nodejs/node/issues/63600 Reviewed-By: Paolo Insogna <paolo@cowtech.it> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: LiviaMedeiros <livia@cirno.name> -------------------------------------------------------------------------------- ℹ This PR was created on Fri, 29 May 2026 08:22:28 GMT ✔ Approvals: 3 ✔ - Paolo Insogna (@ShogunPanda) (TSC): https://github.com/nodejs/node/pull/63634#pullrequestreview-4478094154 ✔ - James M Snell (@jasnell) (TSC): https://github.com/nodejs/node/pull/63634#pullrequestreview-4398614605 ✔ - LiviaMedeiros (@LiviaMedeiros): https://github.com/nodejs/node/pull/63634#pullrequestreview-4478938701 ✔ Last GitHub CI successful ℹ Last Full PR CI on 2026-06-12T05:53:54Z: https://ci.nodejs.org/job/node-test-pull-request/74067/ - Querying data for job/node-test-pull-request/74067/ ✔ Build data downloaded ✔ Last Jenkins CI successful -------------------------------------------------------------------------------- ✔ No git cherry-pick in progress ✔ No git am in progress ✔ No git rebase in progress -------------------------------------------------------------------------------- - Bringing origin/main up to date... From https://github.com/nodejs/node * branch main -> FETCH_HEAD ✔ origin/main is now up-to-date - Downloading patch for 63634 From https://github.com/nodejs/node * branch refs/pull/63634/merge -> FETCH_HEAD ✔ Fetched commits as 4383f67279f2..e6c1949c90e7 -------------------------------------------------------------------------------- Auto-merging lib/fs.js Auto-merging lib/internal/fs/promises.js Auto-merging lib/internal/fs/utils.js [main aabbda3359] fs: support caller-supplied readFile() buffers Author: Matteo Collina <hello@matteocollina.com> Date: Fri May 29 10:20:46 2026 +0200 7 files changed, 635 insertions(+), 18 deletions(-) create mode 100644 test/parallel/test-fs-promises-readfile-buffer-option.js create mode 100644 test/parallel/test-fs-readfile-buffer-option.js Auto-merging lib/internal/fs/utils.js [main a395990cbd] fs: address PR review comments Author: Matteo Collina <hello@matteocollina.com> Date: Wed Jun 10 13:14:54 2026 +0000 2 files changed, 88 insertions(+), 22 deletions(-) [main 5386f6d85d] test: remove getBuffer tests Author: Matteo Collina <hello@matteocollina.com> Date: Wed Jun 10 13:42:35 2026 +0000 2 files changed, 12 deletions(-) [main 928677a24f] doc: add Buffer import to examples to satisfy lint Author: Matteo Collina <hello@matteocollina.com> Date: Wed Jun 10 16:15:07 2026 +0000 1 file changed, 6 insertions(+) ✔ Patches applied There are 4 commits in the PR. Attempting autorebase. (node:478) [DEP0190] DeprecationWarning: Passing args to a child process with shell option true can lead to security vulnerabilities, as the arguments are not escaped, only concatenated. (Use `node --trace-deprecation ...` to show where the warning was created) Rebasing (2/8) Executing: git node land --amend --yes --------------------------------- New Message ---------------------------------- fs: support caller-supplied readFile() buffers

Signed-off-by: Matteo Collina <matteo.collina@gmail.com>
PR-URL: #63634
Fixes: #63600
Reviewed-By: Paolo Insogna <paolo@cowtech.it>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: LiviaMedeiros <livia@cirno.name>

[detached HEAD 6051ce1c89] fs: support caller-supplied readFile() buffers
Author: Matteo Collina <hello@matteocollina.com>
Date: Fri May 29 10:20:46 2026 +0200
7 files changed, 635 insertions(+), 18 deletions(-)
create mode 100644 test/parallel/test-fs-promises-readfile-buffer-option.js
create mode 100644 test/parallel/test-fs-readfile-buffer-option.js
Rebasing (3/8)
Rebasing (4/8)
Executing: git node land --amend --yes
--------------------------------- New Message ----------------------------------
fs: address PR review comments

Remove getBuffer deprecation check from validateReadFileBufferOptions
(getBuffer was never released, dead code)
Simplify buffer validation using isArrayBufferView check
Consolidate buffer option description in docs:
'synchronous function called with the file size and returning
the buffer to read into' -> 'function called with the file size
that returns the buffer'
Add examples for pre-allocated buffer and function variants

Co-authored-by: LiviaMedeiros <livia@example.com>
Co-authored-by: Renegade334 <renegade334@example.com>
Co-authored-by: MattiasBuelens <mattias@example.com>
Co-authored-by: jasnell <jasnell@example.com>

Signed-off-by: Matteo Collina <hello@matteocollina.com>
PR-URL: #63634
Fixes: #63600
Reviewed-By: Paolo Insogna <paolo@cowtech.it>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: LiviaMedeiros <livia@cirno.name>

[detached HEAD 089d6f919c] fs: address PR review comments
Author: Matteo Collina <hello@matteocollina.com>
Date: Wed Jun 10 13:14:54 2026 +0000
2 files changed, 88 insertions(+), 22 deletions(-)
Rebasing (5/8)
Rebasing (6/8)
Executing: git node land --amend --yes
--------------------------------- New Message ----------------------------------
test: remove getBuffer tests

The getBuffer validation was removed from validateReadFileBufferOptions
since getBuffer was never released. Remove the test cases that expected
ERR_INVALID_ARG_VALUE for getBuffer.

Signed-off-by: matteo <matteo@example.com>
PR-URL: #63634
Fixes: #63600
Reviewed-By: Paolo Insogna <paolo@cowtech.it>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: LiviaMedeiros <livia@cirno.name>

[detached HEAD 59227a2aeb] test: remove getBuffer tests
Author: Matteo Collina <hello@matteocollina.com>
Date: Wed Jun 10 13:42:35 2026 +0000
2 files changed, 12 deletions(-)
Rebasing (7/8)
Rebasing (8/8)
Executing: git node land --amend --yes
--------------------------------- New Message ----------------------------------
doc: add Buffer import to examples to satisfy lint

Signed-off-by: matteo <matteo@example.com>
PR-URL: #63634
Fixes: #63600
Reviewed-By: Paolo Insogna <paolo@cowtech.it>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: LiviaMedeiros <livia@cirno.name>

[detached HEAD 93ce6710f2] doc: add Buffer import to examples to satisfy lint
Author: Matteo Collina <hello@matteocollina.com>
Date: Wed Jun 10 16:15:07 2026 +0000
1 file changed, 6 insertions(+)
Successfully rebased and updated refs/heads/main.

ℹ Add commit-queue-squash label to land the PR as one commit, or commit-queue-rebase to land as separate commits.

https://github.com/nodejs/node/actions/runs/27462268368

nodejs-github-bot · 2026-06-13T10:08:07Z

Landed in de67c5c

Signed-off-by: Matteo Collina <matteo.collina@gmail.com> PR-URL: #63634 Fixes: #63600 Reviewed-By: Paolo Insogna <paolo@cowtech.it> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: LiviaMedeiros <livia@cirno.name>

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 May 29, 2026

mcollina requested review from LiviaMedeiros, MattiasBuelens and ronag and removed request for MattiasBuelens May 29, 2026 08:22

mcollina assigned jasnell May 29, 2026

mcollina marked this pull request as ready for review May 29, 2026 08:24

mcollina added a commit to mcollina/node that referenced this pull request May 29, 2026

fs: support caller-supplied readFile() buffers

e04e121

PR-URL: nodejs#63634 Signed-off-by: Matteo Collina <hello@matteocollina.com>

mcollina force-pushed the fs-readfile-buffer-option branch from 50d050d to e04e121 Compare May 29, 2026 08:24

mcollina added a commit to mcollina/node that referenced this pull request May 29, 2026

fs: support caller-supplied readFile() buffers

2f1e60f

PR-URL: nodejs#63634 Signed-off-by: Matteo Collina <matteo.collina@gmail.com>

mcollina force-pushed the fs-readfile-buffer-option branch from e04e121 to 2f1e60f Compare May 29, 2026 08:26

ShogunPanda approved these changes May 29, 2026

View reviewed changes

Renegade334 reviewed May 29, 2026

View reviewed changes

fs: support caller-supplied readFile() buffers

e8edd28

PR-URL: nodejs#63634 Signed-off-by: Matteo Collina <matteo.collina@gmail.com>

mcollina force-pushed the fs-readfile-buffer-option branch from 2f1e60f to e8edd28 Compare May 30, 2026 11:01

LiviaMedeiros reviewed May 30, 2026

View reviewed changes

Comment thread lib/internal/fs/utils.js Outdated

Comment thread lib/internal/fs/utils.js Outdated

jasnell reviewed Jun 1, 2026

View reviewed changes

Comment thread doc/api/fs.md Outdated

jasnell approved these changes Jun 1, 2026

View reviewed changes

mcollina added semver-minor PRs that contain new features and should be released in the next minor version. request-ci Add this label to start a Jenkins CI on a PR. labels Jun 1, 2026

github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jun 1, 2026

mcollina added 3 commits June 10, 2026 13:14

test: remove getBuffer tests

ce65c0b

The getBuffer validation was removed from validateReadFileBufferOptions since getBuffer was never released. Remove the test cases that expected ERR_INVALID_ARG_VALUE for getBuffer. PR-URL: nodejs#63634 Signed-off-by: matteo <matteo@example.com>

doc: add Buffer import to examples to satisfy lint

e6c1949

PR-URL: nodejs#63634 Signed-off-by: matteo <matteo@example.com>

ShogunPanda approved these changes Jun 11, 2026

View reviewed changes

mcollina added the request-ci Add this label to start a Jenkins CI on a PR. label Jun 11, 2026

github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jun 11, 2026

LiviaMedeiros approved these changes Jun 11, 2026

View reviewed changes

mcollina added the commit-queue Add this label to land a pull request using GitHub Actions. label Jun 13, 2026

nodejs-github-bot added commit-queue-failed An error occurred while landing this pull request using GitHub Actions. and removed commit-queue Add this label to land a pull request using GitHub Actions. labels Jun 13, 2026

nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Jun 13, 2026

nodejs-github-bot merged commit de67c5c into nodejs:main Jun 13, 2026
81 of 82 checks passed

Uh oh!

Conversation

mcollina commented May 29, 2026

Uh oh!

ShogunPanda left a comment

Choose a reason for hiding this comment

Uh oh!

Renegade334 May 29, 2026

Choose a reason for hiding this comment

Uh oh!

MattiasBuelens May 29, 2026

Choose a reason for hiding this comment

Uh oh!

Renegade334 May 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

codecov Bot commented May 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

Uh oh!

Uh oh!

nodejs-github-bot commented Jun 1, 2026

Uh oh!

mcollina commented Jun 11, 2026

Uh oh!

ShogunPanda left a comment

Choose a reason for hiding this comment

Uh oh!

nodejs-github-bot commented Jun 11, 2026

Uh oh!

nodejs-github-bot commented Jun 11, 2026

Uh oh!

nodejs-github-bot commented Jun 12, 2026

Uh oh!

nodejs-github-bot commented Jun 13, 2026

Signed-off-by: Matteo Collina <matteo.collina@gmail.com> PR-URL: #63634 Fixes: #63600 Reviewed-By: Paolo Insogna <paolo@cowtech.it> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: LiviaMedeiros <livia@cirno.name>

Signed-off-by: Matteo Collina <hello@matteocollina.com> PR-URL: #63634 Fixes: #63600 Reviewed-By: Paolo Insogna <paolo@cowtech.it> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: LiviaMedeiros <livia@cirno.name>

Signed-off-by: matteo <matteo@example.com> PR-URL: #63634 Fixes: #63600 Reviewed-By: Paolo Insogna <paolo@cowtech.it> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: LiviaMedeiros <livia@cirno.name>

Signed-off-by: matteo <matteo@example.com> PR-URL: #63634 Fixes: #63600 Reviewed-By: Paolo Insogna <paolo@cowtech.it> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: LiviaMedeiros <livia@cirno.name>

[detached HEAD 93ce6710f2] doc: add Buffer import to examples to satisfy lint Author: Matteo Collina <hello@matteocollina.com> Date: Wed Jun 10 16:15:07 2026 +0000 1 file changed, 6 insertions(+) Successfully rebased and updated refs/heads/main.

Uh oh!

Uh oh!

nodejs-github-bot commented Jun 13, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

Renegade334 May 29, 2026 •

edited

Loading

codecov Bot commented May 29, 2026 •

edited

Loading

Signed-off-by: Matteo Collina <matteo.collina@gmail.com>
PR-URL: #63634
Fixes: #63600
Reviewed-By: Paolo Insogna <paolo@cowtech.it>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: LiviaMedeiros <livia@cirno.name>

Signed-off-by: Matteo Collina <hello@matteocollina.com>
PR-URL: #63634
Fixes: #63600
Reviewed-By: Paolo Insogna <paolo@cowtech.it>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: LiviaMedeiros <livia@cirno.name>

Signed-off-by: matteo <matteo@example.com>
PR-URL: #63634
Fixes: #63600
Reviewed-By: Paolo Insogna <paolo@cowtech.it>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: LiviaMedeiros <livia@cirno.name>

Signed-off-by: matteo <matteo@example.com>
PR-URL: #63634
Fixes: #63600
Reviewed-By: Paolo Insogna <paolo@cowtech.it>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: LiviaMedeiros <livia@cirno.name>

[detached HEAD 93ce6710f2] doc: add Buffer import to examples to satisfy lint
Author: Matteo Collina <hello@matteocollina.com>
Date: Wed Jun 10 16:15:07 2026 +0000
1 file changed, 6 insertions(+)
Successfully rebased and updated refs/heads/main.