vfs: integrate with CJS and ESM module loaders#63653
Open
mcollina wants to merge 4 commits into
Open
Conversation
Route loader fs and package.json operations through toggleable wrappers so the VFS can resolve and load modules from mounted paths.
Collaborator
|
Review requested:
|
nodejs-github-bot
added
lib / src
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #63653 +/- ##
==========================================
+ Coverage 90.32% 90.34% +0.01%
==========================================
Files 732 732
Lines 236398 237026 +628
Branches 44504 44672 +168
==========================================
+ Hits 213530 214138 +608
- Misses 14575 14593 +18
- Partials 8293 8295 +2
🚀 New features to boost your workflow:
|
- Normalize setLoaderFsOverrides / setLoaderPackageOverrides to coerce
missing fields to null so calling with {} actually clears them.
- Add clearRealpathCache helper, called from deregisterVFS so paths that
overlap a removed mount are re-resolved against the real filesystem.
- Uninstall loader overrides when the last VFS unmounts so the fast
path is restored; reset hooksInstalled so a future mount reinstalls.
- findVFSPackageJSON now reports the topmost candidate path as a
sentinel; getPackageScopeConfig returns it instead of dirname so the
caller's "not found" cache mirrors the C++ binding's contract.
- legacyMainResolve now throws ERR_MODULE_NOT_FOUND with the resolved
initial candidate path (pkgPath + main) to match the C++ message.
- getFormatOfExtensionlessFile only swallows ENOENT; EACCES/ELOOP and
any other error propagate instead of being silently labeled as JS.
- Validate package.json field types in serializePackageJSON and let the
schema error escape the readPackageJSON override unswallowed.
- Add test-vfs-module-hooks-cleanup covering register/deregister cycles,
malformed schema rejection, and missing-main error propagation.
Address the round-2 adversarial review:
- ERR_MODULE_NOT_FOUND: pass undefined for the third (`exactUrl`) arg
so the message uses "package" and err.url isn't overwritten. The
constructor enforces strict arity; the arg must be present.
- serializePackageJSON now matches the C++ binding's laxity:
non-string `main` is silently omitted (matches USE(value.get_string)),
unknown `type` strings fall back to 'none', non-string `name` / `type`
throw ERR_INVALID_PACKAGE_CONFIG, top-level null / non-object roots
throw too. Old `validateOptionalString` helper removed.
- ESM LoadCache is now cleared from clearLoaderCaches() so dynamic
import() after re-mount sees fresh module jobs.
- deregisterVFS still flushes loader caches on every unmount (we can't
tell which entries belonged to the VFS going away), but only
uninstalls the loader override hooks when the last VFS is removed.
- test-vfs-module-hooks-cleanup: dropped the assertion that the strict
schema rejects `{"main": 42}` (would diverge from real-fs behavior);
added a top-level-null case, a non-string-main lenient case, a multi-
mount partial-deregister case, and rewrote the legacyMainResolve case
to use bare-specifier resolution from an ESM entry inside the VFS so
the override is actually exercised.
Member
Author
|
@joyeecheung take a look, should be easier to review. |
ShogunPanda
approved these changes
May 30, 2026
Address the round-3 adversarial review: - legacyMainResolve override: when `main` is missing/non-string, use pkgPath/index.js as the candidate path in the thrown ERR_MODULE_NOT_FOUND, matching src/node_file.cc:3927-4005. Previously the bare pkgPath was used, diverging from the native binding. - getPackageType override: route through serializePackageJSON so a malformed `type` (non-string) throws ERR_INVALID_PACKAGE_CONFIG just like readPackageJSON and getPackageScopeConfig already do. Closes the divergence where the same package.json answered differently depending on which override was consulted. - clearLoaderCaches no longer clears the ESM cascaded loader's loadCache. Clearing it mid-flight can let a second ModuleJob be created for the same URL, breaking module identity. The trade-off is documented in a comment: ESM modules loaded from a VFS path remain cached after unmount, consistent with how Node.js caches ESM modules everywhere else. - test-vfs-module-hooks-cleanup: tighten the legacyMainResolve case to assert err.url is undefined and the message starts with "Cannot find package " (previously only asserted err.url !== 'package', which is a weak negative).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Route loader fs and package.json operations through toggleable wrappers so the VFS can resolve and load modules from mounted paths.