Skip to content

vfs: add minimal node:vfs subsystem#63115

Open
mcollina wants to merge 30 commits into
nodejs:mainfrom
mcollina:vfs-minimal
Open

vfs: add minimal node:vfs subsystem#63115
mcollina wants to merge 30 commits into
nodejs:mainfrom
mcollina:vfs-minimal

Conversation

@mcollina
Copy link
Copy Markdown
Member

@mcollina mcollina commented May 4, 2026

Adds an experimental node:vfs builtin (gated behind --experimental-vfs) with VirtualFileSystem, VirtualProvider, MemoryProvider, and RealFSProvider. No integration with node:fs, the module loader, or SEA those are intended to land in follow-up PRs.

Extracted from: #61478

Approximate line counts: code ~4k / docs ~1k / tests ~5k — total ~10k lines, with tests being the largest share.

@nodejs-github-bot
Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/config
  • @nodejs/loaders
  • @nodejs/startup

@nodejs-github-bot nodejs-github-bot added lib / src Issues and PRs related to general changes in the lib or src directory. needs-ci PRs that need a full CI run. labels May 4, 2026
@bakkot
Copy link
Copy Markdown
Contributor

bakkot commented May 4, 2026

No integration with node:fs

The docs in this PR claim that you can call myVfs.mount('/virtual') and subsequently regular node:fs operations which read from paths beginning with /virtual will read from the VFS. That sounds like integration to me. Are the docs wrong, or am I misunderstanding what "no integration" means?

@mcollina mcollina force-pushed the vfs-minimal branch 2 times, most recently from 30f5755 to 779fc37 Compare May 5, 2026 09:30
@mcollina mcollina marked this pull request as ready for review May 5, 2026 10:04
@mcollina
Copy link
Copy Markdown
Member Author

mcollina commented May 5, 2026

No integration with node:fs

The docs in this PR claim that you can call myVfs.mount('/virtual') and subsequently regular node:fs operations which read from paths beginning with /virtual will read from the VFS. That sounds like integration to me. Are the docs wrong, or am I misunderstanding what "no integration" means?

Fixed, good spot.

@codecov
Copy link
Copy Markdown

codecov Bot commented May 5, 2026

Codecov Report

❌ Patch coverage is 96.67417% with 192 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.22%. Comparing base (131fd84) to head (58da2ed).
⚠️ Report is 51 commits behind head on main.

Files with missing lines Patch % Lines
lib/internal/vfs/providers/memory.js 93.55% 62 Missing and 4 partials ⚠️
lib/internal/vfs/file_system.js 96.68% 38 Missing ⚠️
lib/internal/vfs/watcher.js 95.20% 32 Missing and 1 partial ⚠️
lib/internal/vfs/providers/real.js 96.01% 19 Missing ⚠️
lib/internal/vfs/streams.js 95.46% 16 Missing ⚠️
lib/internal/vfs/provider.js 98.38% 10 Missing ⚠️
lib/internal/vfs/file_handle.js 98.88% 7 Missing and 1 partial ⚠️
lib/internal/vfs/fd.js 97.70% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #63115      +/-   ##
==========================================
- Coverage   91.94%   90.22%   -1.73%     
==========================================
  Files         362      726     +364     
  Lines      155999   231648   +75649     
  Branches    24057    43703   +19646     
==========================================
+ Hits       143429   208994   +65565     
- Misses      12295    14428    +2133     
- Partials      275     8226    +7951     
Files with missing lines Coverage Δ
lib/internal/bootstrap/realm.js 96.28% <100.00%> (+0.69%) ⬆️
lib/internal/modules/cjs/loader.js 98.04% <100.00%> (+19.43%) ⬆️
lib/internal/process/pre_execution.js 98.28% <100.00%> (+15.17%) ⬆️
lib/internal/vfs/dir.js 100.00% <100.00%> (ø)
lib/internal/vfs/errors.js 100.00% <100.00%> (ø)
lib/internal/vfs/stats.js 100.00% <100.00%> (ø)
lib/vfs.js 100.00% <100.00%> (ø)
src/node_builtins.cc 76.32% <100.00%> (ø)
src/node_options.cc 76.65% <100.00%> (ø)
src/node_options.h 98.00% <100.00%> (ø)
... and 8 more

... and 487 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.

@mcollina mcollina added help wanted Issues that need assistance from volunteers or PRs that need help to proceed. request-ci Add this label to start a Jenkins CI on a PR. labels May 6, 2026
@github-actions github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label May 6, 2026
@nodejs-github-bot
Copy link
Copy Markdown
Collaborator

@mcollina mcollina added the request-ci Add this label to start a Jenkins CI on a PR. label May 7, 2026
@github-actions github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label May 7, 2026
@nodejs-github-bot
Copy link
Copy Markdown
Collaborator

Comment thread doc/api/vfs.md Outdated
Comment thread doc/api/vfs.md Outdated
Comment thread lib/internal/bootstrap/realm.js Outdated
@aduh95
Copy link
Copy Markdown
Contributor

aduh95 commented May 7, 2026

Can you add a test for #63158?

@mcollina mcollina added the request-ci Add this label to start a Jenkins CI on a PR. label May 11, 2026
@github-actions github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label May 11, 2026
mcollina and others added 3 commits May 15, 2026 06:52
Co-authored-by: Antoine du Hamel <duhamelantoine1995@gmail.com>
Co-authored-by: Antoine du Hamel <duhamelantoine1995@gmail.com>
The getter exposed the raw real-fs file descriptor of a virtual file
handle, which is a leaky abstraction: it lets user code bypass the
VFS layer. The only consumer was a test that closed the real fd to
trigger the EBADF rejection paths in the async fd ops; those branches
are defensive and unreachable from the public API, so the test block
is dropped along with the getter.

Assisted-by: Claude-Opus4.7
Signed-off-by: Matteo Collina <hello@matteocollina.com>
Comment thread doc/api/vfs.md Outdated
mcollina added 3 commits May 16, 2026 12:03
* file_handle: import SymbolDispose from primordials so the
  SymbolDispose prototype binding doesn't throw at load time.
* providers/real: import getValidatedPath and setOwnProperty that
  the code-review rewrite of the rootPath validator referenced
  without importing.
* test-vfs-real-provider: getValidatedPath rejects non-strings
  with ERR_INVALID_ARG_TYPE (not ERR_INVALID_ARG_VALUE) and now
  accepts the empty string, so the assertions are updated and the
  empty-string case dropped.
* test-vfs-memory-provider-dynamic: the object-literal rewrite
  left a stray ');' instead of '}', which crashed the parser.

Signed-off-by: Matteo Collina <hello@matteocollina.com>
Signed-off-by: Matteo Collina <hello@matteocollina.com>
Tracks the suggestion from review to delegate the async fd ops in
RealFileHandle to FileHandle methods instead of manually wrapping
fs.read/write/fstat/ftruncate/close in Promises. The refactor is
blocked on a way to wrap an existing numeric fd in a FileHandle so a
sync-opened handle can share one underlying handle for async ops.

Signed-off-by: Matteo Collina <hello@matteocollina.com>
@mcollina mcollina added the request-ci Add this label to start a Jenkins CI on a PR. label May 17, 2026
@github-actions github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label May 17, 2026
@nodejs-github-bot
Copy link
Copy Markdown
Collaborator

RealFSProvider.realpath fed the resolved path through StringPrototype
startsWith against the stored rootPath plus a separator. On Windows
fs.realpathSync is a JS implementation that preserves case while
fs.promises.realpath calls the native binding, which canonicalizes the
drive letter (and potentially other components). The async path then
fails the startsWith check and rejects with EACCES, even for valid
paths inside the root.

Use path.relative to test containment instead, which treats the
comparison as case-insensitive on Windows. Both realpathSync and
realpath now share the same helper.

Fixes failure in test-vfs-real-provider-symlinks on
test-binary-windows-js-suites.

Signed-off-by: Matteo Collina <hello@matteocollina.com>
@nodejs-github-bot
Copy link
Copy Markdown
Collaborator

@mcollina mcollina added the request-ci Add this label to start a Jenkins CI on a PR. label May 18, 2026
@github-actions github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label May 18, 2026
@nodejs-github-bot
Copy link
Copy Markdown
Collaborator

Comment thread test/parallel/test-vfs-watchfile.js Outdated
Comment thread test/parallel/test-vfs-watchfile.js Outdated
Comment thread test/parallel/test-vfs-watchfile.js Outdated
Comment thread test/parallel/test-vfs-watchfile.js
Comment thread test/parallel/test-vfs-watchfile.js Outdated
Comment thread test/parallel/test-vfs-watch.js Outdated
Comment thread test/parallel/test-vfs-watch-promises.js
Comment thread test/parallel/test-vfs-watch-promises.js Outdated
Comment thread test/parallel/test-vfs-watch-promises.js Outdated
Comment thread test/parallel/test-vfs-watch-recursive.js Outdated
Comment thread test/parallel/test-vfs-watch-recursive.js Outdated
Comment thread test/parallel/test-vfs-watch-encoding.js Outdated
Comment thread test/parallel/test-vfs-watch-directory.js Outdated
Comment thread test/parallel/test-vfs-watch-directory.js Outdated
Comment thread test/parallel/test-vfs-watch-directory.js Outdated
Comment thread test/parallel/test-vfs-watch-directory.js Outdated
Comment thread test/parallel/test-vfs-watch-abort-signal.js Outdated
Comment thread test/parallel/test-vfs-watch-abort-signal.js
Comment thread test/parallel/test-vfs-watch-abort-signal.js Outdated
Comment thread test/parallel/test-vfs-watch-abort-signal.js Outdated
Comment thread test/parallel/test-vfs-virtual-provider.js Outdated
Comment thread test/parallel/test-vfs-streams.js Outdated
Comment thread test/parallel/test-vfs-streams.js Outdated
Comment thread test/parallel/test-vfs-streams.js Outdated
Comment thread test/parallel/test-vfs-streams.js Outdated
let data = '';

stream.on('open', common.mustCall((fd) => {
assert.ok((fd & 0x40000000) !== 0);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
assert.ok((fd & 0x40000000) !== 0);
assert.notStrictEqual(fd & 0x40000000, 0);

Comment thread test/parallel/test-vfs-stream-errors.js Outdated
Comment on lines +66 to +68
ws.on('error', common.mustCall((err) => {
assert.ok(err);
}));
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
ws.on('error', common.mustCall((err) => {
assert.ok(err);
}));
ws.on('error', common.expectsError());

Comment thread test/parallel/test-vfs-stream-errors.js Outdated
Comment on lines +47 to +49
ws.on('error', common.mustCall((err) => {
assert.ok(err);
}));
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
ws.on('error', common.mustCall((err) => {
assert.ok(err);
}));
ws.on('error', common.expectsError());

Comment thread test/parallel/test-vfs-stream-errors.js Outdated
const fd = myVfs.openSync('/cl.txt');
const rs = myVfs.createReadStream('/cl.txt', { fd, autoClose: true });
myVfs.closeSync(fd);
rs.on('error', common.mustCall());
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
rs.on('error', common.mustCall());
rs.on('error', common.expectsError());

Comment thread test/parallel/test-vfs-stream-errors.js Outdated
Comment on lines +27 to +29
rs.on('error', common.mustCall((err) => {
assert.strictEqual(err.code, 'EBADF');
}));
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
rs.on('error', common.mustCall((err) => {
assert.strictEqual(err.code, 'EBADF');
}));
rs.on('error', common.expectsError({
code: 'EBADF',
}));

Comment thread test/parallel/test-vfs-stream-errors.js Outdated
Comment on lines +16 to +18
stream.on('error', common.mustCall((err) => {
assert.strictEqual(err.code, 'ENOENT');
}));
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
stream.on('error', common.mustCall((err) => {
assert.strictEqual(err.code, 'ENOENT');
}));
stream.on('error', common.expectsError({
code: 'ENOENT',
}));

Co-authored-by: Antoine du Hamel <duhamelantoine1995@gmail.com>
Co-authored-by: Matteo Collina <matteo.collina@gmail.com>
Comment thread test/parallel/test-vfs-real-provider.js Outdated
Comment thread test/parallel/test-vfs-readfile-encoding.js Outdated
Comment thread test/parallel/test-vfs-readfile-async.js Outdated
Comment thread test/parallel/test-vfs-readdir-symlink-recursive.js Outdated
Comment thread test/parallel/test-vfs-promises.js Outdated
Comment thread test/parallel/test-vfs-promises.js Outdated
Comment thread test/parallel/test-vfs-promises.js Outdated
Comment thread test/parallel/test-vfs-promises.js Outdated
Comment on lines +14 to +16
const fh = await myVfs.promises.open('/hello.txt', 'r');
assert.ok(fh);
assert.ok(typeof fh === 'number' || typeof fh === 'object');
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

That's a strangely loose assertion. Why? Do we expect this to change?

Comment thread test/parallel/test-vfs-flag.js Outdated
Comment thread test/parallel/test-vfs-flag.js Outdated
Comment thread test/parallel/test-vfs-flag.js Outdated
Comment thread test/parallel/test-vfs-flag.js Outdated
Comment thread test/parallel/test-vfs-flag.js Outdated
mcollina and others added 3 commits May 18, 2026 16:50
* test-vfs-stream-errors: drop the unused `assert` import.
* test-vfs-streams: capitalize a lowercase comment to satisfy
  capitalized-comments.

Signed-off-by: Matteo Collina <hello@matteocollina.com>
Co-authored-by: Antoine du Hamel <duhamelantoine1995@gmail.com>
spawnSyncAndAssert handles the expected stderr/status, so the
top-level `assert` is no longer referenced.

Signed-off-by: Matteo Collina <hello@matteocollina.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

help wanted Issues that need assistance from volunteers or PRs that need help to proceed. lib / src Issues and PRs related to general changes in the lib or src directory. needs-ci PRs that need a full CI run.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants