Conversation
|
Review requested:
|
nodejs-github-bot
added
lib / src
mcollina
requested review from
Ethan-Arrowood,
RaisinTen,
avivkeller and
joyeecheung
avivkeller
added
fs
|
The
notable-change
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. |
|
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); |
| validateObject(files, 'options.files'); | ||
| } | ||
|
|
||
| const { VirtualFileSystem } = require('internal/vfs/virtual_fs'); |
There was a problem hiding this comment.
Shouldn't we import this at the top level / lazy load it at the top level?
| ArrayPrototypePush(this.#mocks, { | ||
| __proto__: null, | ||
| ctx, | ||
| restore: restoreFS, |
There was a problem hiding this comment.
| restore: restoreFS, | |
| restore: ctx.restore, |
nit
lib/internal/vfs/entries.js
Outdated
| * @param {object} [options] Optional configuration | ||
| */ | ||
| addFile(name, content, options) { | ||
| const path = this._directory.path + '/' + name; |
lib/internal/vfs/virtual_fs.js
Outdated
| let entry = current.getEntry(segment); | ||
| if (!entry) { | ||
| // Auto-create parent directory | ||
| const dirPath = '/' + segments.slice(0, i + 1).join('/'); |
lib/internal/vfs/virtual_fs.js
Outdated
| let entry = current.getEntry(segment); | ||
| if (!entry) { | ||
| // Auto-create parent directory | ||
| const parentPath = '/' + segments.slice(0, i + 1).join('/'); |
lib/internal/vfs/virtual_fs.js
Outdated
| } | ||
| } | ||
| callback(null, content); | ||
| }).catch((err) => { |
There was a problem hiding this comment.
| }).catch((err) => { | |
| }, (err) => { |
lib/internal/vfs/virtual_fs.js
Outdated
| const bytesToRead = Math.min(length, available); | ||
| content.copy(buffer, offset, readPos, readPos + bytesToRead); |
lib/internal/vfs/virtual_fs.js
Outdated
| } | ||
|
|
||
| callback(null, bytesToRead, buffer); | ||
| }).catch((err) => { |
There was a problem hiding this comment.
| }).catch((err) => { | |
| }, (err) => { |
|
Left an initial review, but like @Ethan-Arrowood said, it'll take time for a more in depth look |
|
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 Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #61478 +/- ##
==========================================
- Coverage 89.70% 89.63% -0.07%
==========================================
Files 676 691 +15
Lines 206069 212695 +6626
Branches 39520 40481 +961
==========================================
+ Hits 184860 190659 +5799
- Misses 13358 14140 +782
- Partials 7851 7896 +45
🚀 New features to boost your workflow:
|
|
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
force-pushed
the
vfs
branch
3 times, most recently
from
5e317de to
977cc3d
Compare
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. |
mcollina
force-pushed
the
vfs
branch
2 times, most recently
from
8d711c1 to
73c18cd
Compare
|
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 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. |
just a bit off topic... but this reminds me of why i created this feature request: Would not lie, it would be cool if NodeJS also provided some type of static 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)
})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: 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? |
Document that VFS is always case-sensitive internally and explain how this interacts with case-insensitive file systems in overlay mode.
Use wrapModuleLoad instead of Module._load directly to ensure diagnostics channels and performance tracing work properly when loading modules from SEA VFS.
- Add "Limitations" section to vfs.md documenting unsupported features: native addons, child processes, worker threads, fs.watch polling, and SEA code caching - Add code caching limitations section to SEA VFS documentation - Add VFS support section to fs.md with example and link to vfs.md
Use POSIX path normalization for VFS paths starting with '/' to preserve forward slashes on Windows. The platform's path.normalize() converts forward slashes to backslashes on Windows, breaking VFS path matching. Add normalizeVFSPath() helper that uses path.posix.normalize() for Unix-style paths and path.normalize() for Windows drive letter paths. Fixes test-vfs-chdir, test-vfs-real-provider, test-vfs-mount-events, and test-vfs-watch on Windows.
- Make SEA VFS opt-in via `"useVfs": true` config field with
corresponding `kEnableVfs` flag in C++ and `isVfsEnabled()` binding
- Replace manual VFS path resolution in embedderRequire with
wrapModuleLoad() that flows through registered module hooks,
supporting transitive requires
- Set main module filename to VFS path when VFS is active so
relative require('./foo.js') resolves correctly
- Convert _-prefixed methods to # private in file_system.js
- Fix inaccurate code caching docs per reviewer suggestion
- Replace supported sync method list with unsupported method list
- Add native addon limitation note for VFS in SEA docs
Use require('./modules/math.js') instead of require('/sea/modules/math.js')
in the main SEA script to verify that relative requires work from the
entry point, since __filename is now set to a VFS path.
Fix clang-format issue in FPrintF call for useVfs config parsing. Fix markdown lint warning for unordered reference link.
- Change "Instead of" to "In addition to" for node:sea API since both the sea API and VFS work together - Replace duplicated supported ops list in SEA docs with cross-reference to VFS documentation - Move useVfs precondition to Loading modules section header - Clarify code caching limitation is due to incomplete implementation, not a technical impossibility
- Rename unregisterVFS to deregisterVFS to match public API naming - Fix Buffer copy safety in SEA provider: use Buffer.from(new Uint8Array(content)) to ensure returned buffers are independent copies safe to mutate, not views over read-only memory segments - Consolidate C++ boolean getters (isSea, isVfsEnabled, isExperimentalSeaWarningNeeded) into boolean properties set once during Initialize(), avoiding repeated function call overhead - Use Module.createRequire() when VFS is enabled in SEA so that require has all standard properties (resolve, cache, etc.) and builtin loading flows through hooks
Add test verifying MemoryProvider readFileSync returns independent buffer copies that don't share underlying memory. Add SEA fixture tests for node:sea API coexistence with VFS and node_modules package lookups. Use Uint8Array from primordials in SEA provider.
Use Environment::GetCurrent(context) instead of context->GetIsolate(), add missing using v8::Boolean declaration, and fix clang-format style.
ESM resolve.js captures `realpathSync` via destructuring at import time, so patching `fs.realpathSync` later has no effect on ESM resolution. Replace the direct `realpathSync` call in `finalizeResolution()` with the shared `toRealPath()` from helpers, which dispatches to a VFS-aware override at runtime. Split `installHooks()` into `installModuleHooks()` (Module._stat, toRealPath override, ESM hooks) and `installFsPatches()` (fs.* patches for user code transparency) for clearer separation of concerns.
ESM files capture fs methods at import time before VFS patches are applied. Extend the toggle pattern (used for toRealPath) to readFileSync and internalModuleStat so VFS can intercept these calls in the ESM loader. - Add readFileSync/setCustomReadFileSync wrappers to helpers.js - Add internalModuleStat/setCustomInternalModuleStat wrappers - Update esm/load.js and esm/translators.js to use helpers.readFileSync - Update esm/resolve.js to use helpers.internalModuleStat - Fix pre-existing bug: StringPrototypeEndsWith received wrong first arg - Revert inline extensionFormatMap, import from esm/formats instead - Register new VFS overrides in module_hooks.js installModuleHooks() - Add buffer independence test for SEA VFS - Document C++ package.json reader gap for VFS packages
The C++ package.json reader reads files via libuv, bypassing VFS entirely. This means "exports", "imports", and "type" fields in package.json don't work for VFS packages. Add toggleable wrappers to package_json_reader.js (same pattern as helpers.js) for read, getPackageScopeConfig, getPackageType, and getNearestParentPackageJSON. When VFS is active, module_hooks.js registers overrides that read from VFS and reimplement upward directory walks in JS. When VFS is inactive, the fast C++ path is used unchanged. Also fix getPackageJSONURL to use the toggleable internalModuleStat from helpers instead of the captured internalFsBinding, and update getFormatFromExtension to use the now-VFS-aware getPackageType.
Replace all _-prefixed methods with # private class fields across VFS provider classes. Since #checkClosed was defined on the base class VirtualFileHandle and called by subclasses via inheritance, each subclass now gets its own #checkClosed using the public closed getter. Also removes unused _rawContent getter from MemoryFileHandle.
Add callback-based tests for writeFile, readlink, and fd operations (open, read, fstat, close) to match the sync coverage in test-vfs-basic.
The eager require of 'internal/modules/esm/formats' at module load time causes a TypeError in shared library builds where the module is not available. Make the require lazy since it's only needed when determining module format from file extension.
Add a test that exercises ESM bare specifier resolution (e.g., import 'my-vfs-pkg') from a node_modules directory inside VFS. This ensures the internalModuleStat wrapper and package.json reader hooks correctly handle package resolution for virtual file system mounts.
Replace 8 monkeypatched internal functions with self-contained resolve
and load hooks using module.registerHooks(). The resolve hook now
handles bare specifiers (node_modules walking + package.json exports
resolution), relative paths, absolute paths, and file: URLs entirely
within VFS. The load hook uses a local format map, eliminating the
problematic esm/formats lazy dependency.
Remove ~140 lines of toggle infrastructure from helpers.js and
package_json_reader.js, reverting them to direct implementations.
Revert esm/resolve.js to use internalBinding('fs') directly.
Add 33 test cases targeting uncovered code paths in the new self-contained module.registerHooks implementation: package exports resolution (string, subpath, conditional, nested, default, array), bare specifier resolution (main field, index fallback, extension resolution, scoped packages), format detection (package type walk, node_modules boundary, .cjs/.mjs/.json), directory entry resolution, and error handling (invalid JSON, null exports).
- Use getLazy() for fs, fs/promises, and Module requires to avoid circular dependencies during bootstrap - Use kEmptyObject as default constructor options in VirtualFileSystem - Replace .then().catch() with .then(success, error) in callback methods - Add JSDoc explaining why urlToPath differs from toPathIfFileURL PR-URL: nodejs#61478
Replace numbered mount paths (/virtual, /virtual2, etc.) with descriptive names (/esm-named, /esm-default, /esm-chain, etc.) that clarify what each test exercises. Clarify the comment explaining why unique mount paths are needed (V8 ESM cache persists after unmount). PR-URL: nodejs#61478
Add tests for internal VFS utility functions and provider base class to improve code coverage: - router.js: splitPath, getParentPath, getBaseName - provider.js: readonly EROFS checks for all write operations - provider.js: ERR_METHOD_NOT_IMPLEMENTED for unimplemented methods PR-URL: nodejs#61478
Add tests for internal VFS utility functions and provider base class to improve code coverage: - router.js: splitPath, getParentPath, getBaseName - provider.js: readonly EROFS checks for all write operations - provider.js: ERR_METHOD_NOT_IMPLEMENTED for unimplemented methods PR-URL: nodejs#61478
Add tests for internal VFS utility functions and provider base class to improve code coverage: - router.js: splitPath, getParentPath, getBaseName - provider.js: readonly EROFS checks for all write operations - provider.js: ERR_METHOD_NOT_IMPLEMENTED for unimplemented methods PR-URL: nodejs#61478
Add tests for internal VFS utility functions and provider base class to improve code coverage: - router.js: splitPath, getParentPath, getBaseName - provider.js: readonly EROFS checks for all write operations - provider.js: ERR_METHOD_NOT_IMPLEMENTED for unimplemented methods PR-URL: nodejs#61478
17 participants
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 supportSEAProvider- Read-only access to Single Executable Application assetsVirtualProvider- Base class for creating custom providersStandard fs API - Uses familiar
writeFileSync,readFileSync,mkdirSyncinstead of custom methodsMount Mode - VFS mounts at a specific path prefix (e.g.,
/virtual), clear separation from real filesystemModule Loading -
require()andimportwork seamlessly from virtual filesSEA Integration - Assets automatically mounted at
/seawhen running as a Single Executable ApplicationFull fs Support - readFile, stat, readdir, exists, streams, promises, glob, symlinks
Example
SEA Usage
When running as a Single Executable Application, bundled assets are automatically available:
Public API
Disclaimer: I've used a significant amount of Claude Code tokens to create this PR. I've reviewed all changes myself.
Fixes #60021