Skip to content

Conversation

@mcollina
Copy link
Member

@mcollina mcollina commented Jan 22, 2026

A first-class virtual file system module (node:vfs) with a provider-based architecture that integrates with Node.js's fs module and module loader.

Key Features

  • Provider Architecture - Extensible design with pluggable providers:

    • MemoryProvider - In-memory file system with full read/write support
    • SEAProvider - Read-only access to Single Executable Application assets
    • VirtualProvider - Base class for creating custom providers
  • Standard fs API - Uses familiar writeFileSync, readFileSync, mkdirSync instead of custom methods

  • Mount Mode - VFS mounts at a specific path prefix (e.g., /virtual), clear separation from real filesystem

  • Module Loading - require() and import work seamlessly from virtual files

  • SEA Integration - Assets automatically mounted at /sea when running as a Single Executable Application

  • Full fs Support - readFile, stat, readdir, exists, streams, promises, glob, symlinks

Example

const vfs = require('node:vfs');
const fs = require('node:fs');

// Create a VFS with default MemoryProvider
const myVfs = vfs.create();

// Use standard fs-like API
myVfs.mkdirSync('/app');
myVfs.writeFileSync('/app/config.json', '{"debug": true}');
myVfs.writeFileSync('/app/module.js', 'module.exports = "hello"');

// Mount to make accessible via fs module
myVfs.mount('/virtual');

// Works with standard fs APIs
const config = JSON.parse(fs.readFileSync('/virtual/app/config.json', 'utf8'));
const mod = require('/virtual/app/module.js');

// Cleanup
myVfs.unmount();

SEA Usage

When running as a Single Executable Application, bundled assets are automatically available:

const fs = require('node:fs');

// Assets are automatically mounted at /sea - no setup required
const config = fs.readFileSync('/sea/config.json', 'utf8');
const template = fs.readFileSync('/sea/templates/index.html', 'utf8');

Public API

const vfs = require('node:vfs');

vfs.create([provider][, options])  // Create a VirtualFileSystem
vfs.VirtualFileSystem              // The main VFS class
vfs.VirtualProvider                // Base class for custom providers
vfs.MemoryProvider                 // In-memory provider
vfs.SEAProvider                    // SEA assets provider (read-only)

Disclaimer: I've used a significant amount of Claude Code tokens to create this PR. I've reviewed all changes myself.

@nodejs-github-bot
Copy link
Collaborator

Review requested:

  • @nodejs/single-executable
  • @nodejs/test_runner

@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 Jan 22, 2026
@avivkeller avivkeller added fs Issues and PRs related to the fs subsystem / file system. module Issues and PRs related to the module subsystem. semver-minor PRs that contain new features and should be released in the next minor version. notable-change PRs with changes that should be highlighted in changelogs. needs-benchmark-ci PR that need a benchmark CI run. test_runner Issues and PRs related to the test runner subsystem. labels Jan 22, 2026
@github-actions
Copy link
Contributor

The notable-change PRs with changes that should be highlighted in changelogs. label has been added by @avivkeller.

Please suggest a text for the release notes if you'd like to include a more detailed summary, then proceed to update the PR description with the text or a link to the notable change suggested text comment. Otherwise, the commit will be placed in the Other Notable Changes section.

@Ethan-Arrowood
Copy link
Contributor

Nice! This is a great addition. Since it's such a large PR, this will take me some time to review. Will try to tackle it over the next week.

*/
existsSync(path) {
// Prepend prefix to path for VFS lookup
const fullPath = this.#prefix + (StringPrototypeStartsWith(path, '/') ? path : '/' + path);
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 use path.join?

Comment on lines +417 to +435
/**
* Gets the underlying VirtualFileSystem instance.
* @returns {VirtualFileSystem}
*/
get vfs() {
return this.#vfs;
}

/**
* Gets the mount prefix for the mock file system.
* @returns {string}
*/
get prefix() {
return this.#prefix;
}
Copy link
Member

Choose a reason for hiding this comment

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

Do these need to be getters? Why can't we expose the actual values.

If a user overwrites them, they can if they wish?

validateObject(files, 'options.files');
}

const { VirtualFileSystem } = require('internal/vfs/virtual_fs');
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't we import this at the top level / lazy load it at the top level?

ArrayPrototypePush(this.#mocks, {
__proto__: null,
ctx,
restore: restoreFS,
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
restore: restoreFS,
restore: ctx.restore,

nit

* @param {object} [options] Optional configuration
*/
addFile(name, content, options) {
const path = this._directory.path + '/' + name;
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 use path.join?

let entry = current.getEntry(segment);
if (!entry) {
// Auto-create parent directory
const dirPath = '/' + segments.slice(0, i + 1).join('/');
Copy link
Member

Choose a reason for hiding this comment

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

Let's use path.join

let entry = current.getEntry(segment);
if (!entry) {
// Auto-create parent directory
const parentPath = '/' + segments.slice(0, i + 1).join('/');
Copy link
Member

Choose a reason for hiding this comment

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

path.join?

}
}
callback(null, content);
}).catch((err) => {
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
}).catch((err) => {
}, (err) => {

Comment on lines 676 to 677
const bytesToRead = Math.min(length, available);
content.copy(buffer, offset, readPos, readPos + bytesToRead);
Copy link
Member

Choose a reason for hiding this comment

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

Primordials?

}

callback(null, bytesToRead, buffer);
}).catch((err) => {
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
}).catch((err) => {
}, (err) => {

@avivkeller
Copy link
Member

Left an initial review, but like @Ethan-Arrowood said, it'll take time for a more in depth look

@joyeecheung
Copy link
Member

joyeecheung commented Jan 22, 2026

It's nice to see some momentum in this area, though from a first glance it seems the design has largely overlooked the feedback from real world use cases collected 4 years ago: https://github.com/nodejs/single-executable/blob/main/docs/virtual-file-system-requirements.md - I think it's worth checking that the API satisfies the constraints that users of this feature have provided, to not waste the work that have been done by prior contributors to gather them, or having to reinvent it later (possibly in a breaking manner) to satisfy these requirements from real world use cases.

@codecov
Copy link

codecov bot commented Jan 22, 2026

Codecov Report

❌ Patch coverage is 90.78870% with 313 lines in your changes missing coverage. Please review.
✅ Project coverage is 89.79%. Comparing base (1ad04e2) to head (d501c7b).
⚠️ Report is 5 commits behind head on main.

Files with missing lines Patch % Lines
lib/internal/vfs/module_hooks.js 81.83% 108 Missing and 1 partial ⚠️
lib/internal/vfs/virtual_fs.js 91.91% 107 Missing and 2 partials ⚠️
lib/internal/vfs/entries.js 93.44% 23 Missing ⚠️
lib/internal/vfs/streams.js 88.19% 19 Missing ⚠️
lib/internal/vfs/errors.js 88.53% 18 Missing ⚠️
lib/internal/vfs/router.js 91.11% 12 Missing ⚠️
lib/internal/vfs/fd.js 94.57% 9 Missing ⚠️
lib/internal/main/embedding.js 90.00% 6 Missing ⚠️
lib/internal/vfs/stats.js 96.93% 6 Missing ⚠️
lib/internal/vfs/sea.js 97.87% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #61478      +/-   ##
==========================================
- Coverage   89.80%   89.79%   -0.02%     
==========================================
  Files         672      681       +9     
  Lines      203907   207293    +3386     
  Branches    39203    39692     +489     
==========================================
+ Hits       183121   186139    +3018     
- Misses      13113    13475     +362     
- Partials     7673     7679       +6     
Files with missing lines Coverage Δ
lib/fs.js 98.20% <100.00%> (+<0.01%) ⬆️
lib/internal/test_runner/mock/mock.js 98.86% <100.00%> (+0.13%) ⬆️
lib/sea.js 97.93% <100.00%> (+0.16%) ⬆️
lib/internal/vfs/sea.js 97.87% <97.87%> (ø)
lib/internal/main/embedding.js 88.55% <90.00%> (-0.43%) ⬇️
lib/internal/vfs/stats.js 96.93% <96.93%> (ø)
lib/internal/vfs/fd.js 94.57% <94.57%> (ø)
lib/internal/vfs/router.js 91.11% <91.11%> (ø)
lib/internal/vfs/errors.js 88.53% <88.53%> (ø)
lib/internal/vfs/streams.js 88.19% <88.19%> (ø)
... and 3 more

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

@jimmywarting
Copy link

jimmywarting commented Jan 22, 2026

And why not something like OPFS aka whatwg/fs?

const rootHandle = await navigator.storage.getDirectory()
await rootHandle.getFileHandle('config.json', { create: true })
fs.mount('/app', rootHandle) // to make it work with fs
fs.readFileSync('/app/config.json')

OR

const rootHandle = await navigator.storage.getDirectory()
await rootHandle.getFileHandle('config.json', { create: true })

fs.readFileSync('sandbox:/config.json')

fs.createVirtual seems like something like a competing specification

@mcollina mcollina force-pushed the vfs branch 3 times, most recently from 5e317de to 977cc3d Compare January 23, 2026 08:15
@mcollina
Copy link
Member Author

And why not something like OPFS aka whatwg/fs?

I generally prefer not to interleave with WHATWG specs as much as possible for core functionality (e.g., SEA). In my experience, they tend to perform poorly on our codebase and remove a few degrees of flexibility. (I also don't find much fun in working on them, and I'm way less interested in contributing to that.)

On an implementation side, the core functionality of this feature will be identical (technically, it's missing writes that OPFS supports), as we would need to impact all our internal fs methods anyway.

If this lands, we can certainly iterate on a WHATWG-compatible API for this, but I would not add this to this PR.

@juliangruber
Copy link
Member

Small prior art: https://github.com/juliangruber/subfs

@mcollina mcollina force-pushed the vfs branch 2 times, most recently from 8d711c1 to 73c18cd Compare January 23, 2026 13:19
@Qard
Copy link
Member

Qard commented Jan 23, 2026

I also worked on this a bit on the side recently: Qard@73b8fc6

That is very much in chaotic ideation stage with a bunch of LLM assistance to try some different ideas, but the broader concept I was aiming for was to have a VirtualFileSystem type which would actually implement the entire API surface of the fs module, accepting a Provider type to delegate the internals of the whole cluster of file system types to a singular class managing the entire cluster of fs-related types such that the fs module could actually just be fully converted to:

module.exports = new VirtualFileSystem(new LocalProvider())

I intended for it to be extensible for a bunch of different interesting scenarios, so there's also an S3 provider and a zip file provider there, mainly just to validate that the model can be applied to other varieties of storage systems effectively.

Keep in mind, like I said, the current state is very much just ideation in a branch I pushed up just now to share, but I think there are concepts for extensibility in there that we could consider to enable a whole ecosystem of flexible storage providers. 🙂

Personally, I would hope for something which could provide both read and write access through an abstraction with swappable backends of some variety, this way we could pass around these virtualized file systems like objects and let an ecosystem grow around accepting any generalized virtual file system for its storage backing. I think it'd be very nice for a lot of use cases like file uploads or archive management to be able to just treat them like any other readable and writable file system.

@jimmywarting
Copy link

jimmywarting commented Jan 23, 2026

Personally, I would hope for something which could provide both read and write access through an abstraction with swappable backends of some variety, this way we could pass around these virtualized file systems like objects and let an ecosystem grow around accepting any generalized virtual file system for its storage backing. I think it'd be very nice for a lot of use cases like file uploads or archive management to be able to just treat them like any other readable and writable file system.

just a bit off topic... but this reminds me of why i created this feature request:
Blob.from() for creating virtual Blobs with custom backing storage

Would not lie, it would be cool if NodeJS also provided some type of static Blob.from function to create virtual lazy blobs. could live on fs.blobFrom for now...

example that would only work in NodeJS (based on how it works internally)

const size = 26

const blobPart = BlobFrom({
  size,
  stream (start, end) {
    // can either be sync or async (that resolves to a ReadableStream)
    // return new Response('abcdefghijklmnopqrstuvwxyz'.slice(start, end)).body
    // return new Blob(['abcdefghijklmnopqrstuvwxyz'.slice(start, end)]).stream()
    
    return fetch('https://httpbin.dev/range/' + size, {
      headers: {
        range: `bytes=${start}-${end - 1}`
      }
    }).then(r => r.body)
  }
})

blobPart.text().then(text => {
  console.log('a-z', text)
})

blobPart.slice(-3).text().then(text => {
  console.log('x-z', text)
})

const a = blobPart.slice(0, 6)
a.text().then(text => {
  console.log('a-f', text)
})

const b = a.slice(2, 4)
b.text().then(text => {
  console.log('c-d', text)
})
x-z xyz
a-z abcdefghijklmnopqrstuvwxyz
a-f abcdef
c-d cd

An actual working PoC (I would not rely on this unless it became officially supported by nodejs core - this is a hack)

const blob = new Blob()
const symbols = Object.getOwnPropertySymbols(blob)
const blobSymbol = symbols.map(s => [s.description, s])
const symbolMap = Object.fromEntries(blobSymbol)
const {
  kHandle,
  kLength,
} = symbolMap

function BlobFrom ({ size, stream }) {
  const blob = new Blob()
  if (size === 0) return blob

  blob[kLength] = size
  blob[kHandle] = {
    span: [0, size],

    getReader () {
      const [start, end] = this.span
      if (start === end) {
        return { pull: cb => cb(0) }
      }

      let reader

      return {
        async pull (cb) {
          reader ??= (await stream(start, end)).getReader()
          const {done, value} = await reader.read()
          cb(done ^ 1, value)
        }
      }
    },

    slice (start, end) {
      const [baseStart] = this.span

      return {
        span: [baseStart + start, baseStart + end],
        getReader: this.getReader,
        slice: this.slice,
      }
    }
  }

  return blob
}

currently problematic to do: new Blob([a, b]), new File([blobPart], 'alphabet.txt', { type: 'text/plain' })

also need to handle properly clone, serialize & deserialize, if this where to be sent of to another worker - then i would transfer a MessageChannel where the worker thread asks main frame to hand back a transferable ReadableStream when it needs to read something.

but there are probably better ways to handle this internally in core with piping data directly to and from different destinations without having to touch the js runtime? - if only getReader could return the reader directly instead of needing to read from the ReadableStream using js?

const kPath = Symbol('kPath');

// FD range: 10000+ to avoid conflicts with real fds
const VFS_FD_BASE = 10_000;
Copy link
Member

Choose a reason for hiding this comment

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

Alternatively here... it might be interesting to use negative fds for vfs nodes. Not sure if practically possible but it would avoid the potential for collision.

Copy link
Contributor

Choose a reason for hiding this comment

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

Negative fds aren't valid per unix rules and other third-party tools sometimes have issues with that. See yarnpkg/berry#4737.

-->
* `path` {string} The virtual path for the file.
* `content` {string|Buffer|Function} The file content, or a function that
Copy link
Member

Choose a reason for hiding this comment

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

Suggest not limiting this to Buffer... also support Uint8Array explicitly.

For "dynamic content", why not also accept sync/async iterables here so that the content can be provided by a stream or generator.

We might also want to support Blob and File here.

Adds a virtual file. The `content` can be:
* A `string` or `Buffer` for static content
Copy link
Member

Choose a reason for hiding this comment

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

When a string is provided, is there a default encoding?

* `prefix` {string} The path prefix where the VFS will be mounted.
Mounts the VFS at a specific path prefix. All paths in the VFS become accessible
under this prefix. Only one mount point can be active at a time.
Copy link
Member

Choose a reason for hiding this comment

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

Does this last sentence mean this will throw if it is called twice?

Having vfs.mount(...) return a disposable object that will unmount on disposal would be nice.

const myMount = vfs.mount(prefix);
myMount.unmount();

// or

{
  using myMount = vfs.mount(prefix);
  // ...
}

Copy link
Member

Choose a reason for hiding this comment

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

Alternatively, having vfs itself support Symbol.dispose would be nice (if it is already and I haven't read far enough thru then ignore this ;-) ...)


const vfs = fs.createVirtual();
vfs.addFile('/module.js', 'module.exports = "hello"');
vfs.mount('/virtual');
Copy link
Member

Choose a reason for hiding this comment

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

It would be good for the docs here to explain a bit about what happens if there's already an existing real file system path /virtual. Is it shadowed? Does mount error? etc

Copy link
Member

Choose a reason for hiding this comment

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

If it automatically shadows, then I can see a potential security hole in this. For instance, if I can mount a path that shadows any real file system path, then a module could obfuscate mounting over a critical system path and trick users into reading malicious data or writing data to something that intercepts the data and forwards it somewhere. Not a critical concern given our security model but something that would be worth documenting

Enables overlay mode, where the VFS is checked first for all file system
operations. If a path exists in the VFS, it is used; otherwise, the operation
falls through to the real file system (if `fallthrough` is enabled).
Copy link
Member

Choose a reason for hiding this comment

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

I would add warnings to the docs here about how this could be abused to secretly shadow critical file system paths.

* `path` {string} The path to check.
* Returns: {boolean}
Returns `true` if the VFS contains a file or directory at the given path.
Copy link
Member

Choose a reason for hiding this comment

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

This should document the behavior if "overlay" mode is enabled.

added: REPLACEME
-->
* `path` {string} The path to remove.
Copy link
Member

Choose a reason for hiding this comment

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

This should document the behavior when "overlay" mode is enabled.

const mod = require('/virtual/module.js');
```
#### `vfs.overlay()`
Copy link
Member

Choose a reason for hiding this comment

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

I think I'd actually prefer this to be an option passed to createVirtual

When the VFS is unmounted, `process.chdir()` and `process.cwd()` are restored
to their original implementations.
> **Note:** VFS hooks are not automatically shared with worker threads. Each
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
> **Note:** VFS hooks are not automatically shared with worker threads. Each
> VFS hooks are not automatically shared with worker threads. Each

console.log(myVfs.provider.readonly); // true
```

### `vfs.mount(prefix)`
Copy link
Member

Choose a reason for hiding this comment

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

This feels like it's duplicating the docs in fs.md a bit too much


* {boolean}

Returns `true` if the provider supports symbolic links.
Copy link
Member

Choose a reason for hiding this comment

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

Symbolic link support needs more explanation. For instance, in "overlay" mode... would it be possible for a real file system symbolic link to target a vfs symbolic link and vis versa? I assume yes but that gets tricky especially when worker threads are involved... sometimes the link works other times it doesn't, etc.

lib/vfs.js Outdated
const { RealFSProvider } = require('internal/vfs/providers/real');

// SEAProvider is lazy-loaded to avoid loading SEA bindings when not needed
let SEAProvider = null;
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
let SEAProvider = null;
let SEAProvider;

lib/vfs.js Outdated
let SEAProvider = null;

function getSEAProvider() {
if (SEAProvider === null) {
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
if (SEAProvider === null) {
if (SEAProvider === undefined) {

Comment on lines +22 to +26
SEAProvider = class SEAProviderUnavailable {
constructor() {
throw new ERR_INVALID_STATE('SEAProvider can only be used in a Single Executable Application');
}
};
Copy link
Member

@jasnell jasnell Jan 29, 2026

Choose a reason for hiding this comment

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

Not objecting or suggesting that this be change but it something about this pattern just feels weird to me... Don't have a better suggestion tho

this[kMounted] = false;
this[kOverlay] = false;
this[kFallthrough] = options.fallthrough !== false;
this[kModuleHooks] = options.moduleHooks !== false;
Copy link
Member

Choose a reason for hiding this comment

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

Nit... would prefer strict type checking for these (e.g. validateBoolean).

* Returns true if VFS falls through to real fs on miss.
* @returns {boolean}
*/
get fallthrough() {
Copy link
Member

Choose a reason for hiding this comment

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

Feels a bit off that some of the bool properties are is* and others are not.


// If virtual cwd is enabled and set, resolve relative to it
if (this[kVirtualCwdEnabled] && this[kVirtualCwd] !== null) {
const resolved = this[kVirtualCwd] + '/' + inputPath;
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
const resolved = this[kVirtualCwd] + '/' + inputPath;
const resolved = `${this[kVirtualCwd]}/${inputPath}`;

* asynchronously via process.nextTick - matching real fs behavior.
* @private
*/
_openFile() {
Copy link
Member

Choose a reason for hiding this comment

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

Why _ prefix? Why not make this private or a top-level module local function?

}
const { getAssetKeys } = internalBinding('sea');
const keys = getAssetKeys() || [];
return keys.length > 0;
Copy link
Member

Choose a reason for hiding this comment

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

Do we really need a separate method for this? getSeaVfs could likely just return null if SEA VFS is not available.

### Glob support
The VFS integrates with `fs.glob()`, `fs.globSync()`, and `fs/promises.glob()`
when mounted or in overlay mode:
Copy link
Member

Choose a reason for hiding this comment

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

btw, am I just missing it or is overlay mode not covered in the tests? I'm given how the file paths are being normalized and processed in the implementation, It's not clear if overlay works correctly on windows when there are drive letters in the path or win-style namespace prefixes.


// readlinkSync should work (returns target path)
const target = vfs.readlinkSync('/virtual/broken');
assert.strictEqual(target, '/nonexistent');
Copy link
Member

Choose a reason for hiding this comment

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

This should probably also test that the target can be created after the symlink and, once it is created, the symlink starts working correctly.

// VFS uses forward slashes internally regardless of platform.
if (sep === '\\') {
normalized = StringPrototypeReplaceAll(normalized, '\\', '/');
}
Copy link
Member

Choose a reason for hiding this comment

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

Is there a reason not to use the path module?

Comment on lines 66 to 68
this.mtime = DateNow();
this.ctime = DateNow();
this.birthtime = DateNow();
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
this.mtime = DateNow();
this.ctime = DateNow();
this.birthtime = DateNow();
const now = DateNow();
this.mtime = now;
this.ctime = now;
this.birthtime = now;

@jasnell
Copy link
Member

jasnell commented Jan 29, 2026

Left an initial round of feedback comments but to keep from it being too overwhelming I'll stop here and do a second pass after the comments are addressed/responded to. They don't all require changes if there are reasons, of course.

- Add security considerations section warning about path shadowing risks
- Document that VFS shadows real paths when mounted
- Add symlink documentation explaining VFS-internal-only behavior
- Clarify that only mount mode exists (no overlay mode)
- Reorder synchronous methods alphabetically per doc conventions

Addresses review comments from @jasnell regarding security documentation,
overlay mode clarification, alphabetical ordering, and symlink behavior.
- Use undefined instead of null for lazy-loaded SEAProvider
- Add validateBoolean for moduleHooks and virtualCwd options
- Use template literal for path concatenation
- Convert VirtualReadStream to use private class fields
- Cache DateNow() result in MemoryEntry constructor

Addresses review comments nodejs#18, nodejs#19, nodejs#21, nodejs#23, nodejs#24, nodejs#29.
Add overlay mode to VirtualFileSystem that only intercepts paths
that exist in the VFS, allowing real filesystem access to fall
through for non-mocked files. This enables use cases like the
test runner mock fs where specific files are mocked while allowing
access to real files.

Changes:
- Add `overlay` option to VirtualFileSystem constructor
- Modify shouldHandle() to check file existence in overlay mode
- Add `overlay` property getter
- Update test runner mock to use VFS with overlay mode
- Use path.dirname() instead of string manipulation in mock.js
- Rename isMounted to mounted for consistency
- Add security documentation for overlay mode risks
- Add comprehensive tests for overlay mode including worker threads
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

fs Issues and PRs related to the fs subsystem / file system. lib / src Issues and PRs related to general changes in the lib or src directory. module Issues and PRs related to the module subsystem. needs-benchmark-ci PR that need a benchmark CI run. needs-ci PRs that need a full CI run. notable-change PRs with changes that should be highlighted in changelogs. semver-minor PRs that contain new features and should be released in the next minor version. test_runner Issues and PRs related to the test runner subsystem.

Projects

None yet

Development

Successfully merging this pull request may close these issues.