vfs: address review feedback for module_hooks and file_system

- 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: #61478

Author: mcollina

lib/internal/vfs/file_system.js

@@ -59,7 +59,7 @@ const {
   createEBADF,
 } = require('internal/vfs/errors');
 const { VirtualReadStream } = require('internal/vfs/streams');
-const { emitExperimentalWarning } = require('internal/util');
+const { emitExperimentalWarning, kEmptyObject } = require('internal/util');
 
 // Private symbols
 const kProvider = Symbol('kProvider');
@@ -97,7 +97,7 @@ class VirtualFileSystem {
    * @param {boolean} [options.virtualCwd] Whether to enable virtual working directory
    * @param {boolean} [options.overlay] Whether to enable overlay mode (only intercept existing files)
    */
-  constructor(providerOrOptions, options = {}) {
+  constructor(providerOrOptions, options = kEmptyObject) {
     emitExperimentalWarning('VirtualFileSystem');
 
     // Handle case where first arg is options object (no provider)
@@ -690,8 +690,7 @@ class VirtualFileSystem {
     }
 
     this[kProvider].readFile(this.#toProviderPath(filePath), options)
-      .then((data) => callback(null, data))
-      .catch((err) => callback(err));
+      .then((data) => callback(null, data), (err) => callback(err));
   }
 
   /**
@@ -708,8 +707,7 @@ class VirtualFileSystem {
     }
 
     this[kProvider].writeFile(this.#toProviderPath(filePath), data, options)
-      .then(() => callback(null))
-      .catch((err) => callback(err));
+      .then(() => callback(null), (err) => callback(err));
   }
 
   /**
@@ -725,8 +723,7 @@ class VirtualFileSystem {
     }
 
     this[kProvider].stat(this.#toProviderPath(filePath), options)
-      .then((stats) => callback(null, stats))
-      .catch((err) => callback(err));
+      .then((stats) => callback(null, stats), (err) => callback(err));
   }
 
   /**
@@ -742,8 +739,7 @@ class VirtualFileSystem {
     }
 
     this[kProvider].lstat(this.#toProviderPath(filePath), options)
-      .then((stats) => callback(null, stats))
-      .catch((err) => callback(err));
+      .then((stats) => callback(null, stats), (err) => callback(err));
   }
 
   /**
@@ -759,8 +755,7 @@ class VirtualFileSystem {
     }
 
     this[kProvider].readdir(this.#toProviderPath(dirPath), options)
-      .then((entries) => callback(null, entries))
-      .catch((err) => callback(err));
+      .then((entries) => callback(null, entries), (err) => callback(err));
   }
 
   /**
@@ -776,8 +771,7 @@ class VirtualFileSystem {
     }
 
     this[kProvider].realpath(this.#toProviderPath(filePath), options)
-      .then((realPath) => callback(null, this.#toMountedPath(realPath)))
-      .catch((err) => callback(err));
+      .then((realPath) => callback(null, this.#toMountedPath(realPath)), (err) => callback(err));
   }
 
   /**
@@ -793,8 +787,7 @@ class VirtualFileSystem {
     }
 
     this[kProvider].readlink(this.#toProviderPath(linkPath), options)
-      .then((target) => callback(null, target))
-      .catch((err) => callback(err));
+      .then((target) => callback(null, target), (err) => callback(err));
   }
 
   /**
@@ -810,8 +803,7 @@ class VirtualFileSystem {
     }
 
     this[kProvider].access(this.#toProviderPath(filePath), mode)
-      .then(() => callback(null))
-      .catch((err) => callback(err));
+      .then(() => callback(null), (err) => callback(err));
   }
 
   /**
@@ -836,8 +828,7 @@ class VirtualFileSystem {
       .then((handle) => {
         const fd = openVirtualFd(handle);
         callback(null, fd);
-      })
-      .catch((err) => callback(err));
+      }, (err) => callback(err));
   }
 
   /**
@@ -856,8 +847,7 @@ class VirtualFileSystem {
       .then(() => {
         closeVirtualFd(fd);
         callback(null);
-      })
-      .catch((err) => callback(err));
+      }, (err) => callback(err));
   }
 
   /**
@@ -877,8 +867,7 @@ class VirtualFileSystem {
     }
 
     vfd.entry.read(buffer, offset, length, position)
-      .then(({ bytesRead }) => callback(null, bytesRead, buffer))
-      .catch((err) => callback(err));
+      .then(({ bytesRead }) => callback(null, bytesRead, buffer), (err) => callback(err));
   }
 
   /**
@@ -900,8 +889,7 @@ class VirtualFileSystem {
     }
 
     vfd.entry.stat(options)
-      .then((stats) => callback(null, stats))
-      .catch((err) => callback(err));
+      .then((stats) => callback(null, stats), (err) => callback(err));
   }
 
   // ==================== Stream Operations ====================

lib/internal/vfs/module_hooks.js

@@ -33,10 +33,17 @@ function normalizeVFSPath(inputPath) {
   return path.normalize(inputPath);
 }
 const { isURL, pathToFileURL, fileURLToPath, toPathIfFileURL, URL } = require('internal/url');
-const { kEmptyObject } = require('internal/util');
+const { getLazy, kEmptyObject } = require('internal/util');
 const { validateObject } = require('internal/validators');
 const { createENOENT } = require('internal/vfs/errors');
 
+// Lazy-load modules to avoid circular dependencies during bootstrap
+const lazyFs = getLazy(() => require('fs'));
+const lazyFsPromises = getLazy(() => require('fs/promises'));
+const lazyModule = getLazy(
+  () => require('internal/modules/cjs/loader').Module,
+);
+
 // Registry of active VFS instances
 const activeVFSList = [];
 
@@ -757,8 +764,10 @@ function resolveBareSpecifier(specifier, context, nextResolve) {
 }
 
 /**
- * Convert a file path or file URL to a normalized file path.
- * @param {string} urlOrPath URL or path string
+ * Convert a file URL string or path to a normalized file path.
+ * Note: This differs from toPathIfFileURL which only handles URL objects,
+ * not 'file:' strings that come from context.parentURL.
+ * @param {string} urlOrPath URL string or path
  * @returns {string} Normalized file path
  */
 function urlToPath(urlOrPath) {
@@ -861,9 +870,7 @@ function vfsLoadHook(url, context, nextLoad) {
  * are called, eliminating the need for monkeypatching.
  */
 function installModuleHooks() {
-  const Module = require('internal/modules/cjs/loader').Module;
-
-  Module.registerHooks({
+  lazyModule().registerHooks({
     resolve: vfsResolveHook,
     load: vfsLoadHook,
   });
@@ -874,7 +881,7 @@ function installModuleHooks() {
  * These make fs.readFileSync('/vfs/path'), fs.statSync, etc. work for user code.
  */
 function installFsPatches() {
-  const fs = require('fs');
+  const fs = lazyFs();
 
   // Save originals
   originalReadFileSync = fs.readFileSync;
@@ -961,7 +968,7 @@ function installFsPatches() {
   };
 
   // Hook fs/promises for async glob support
-  const fsPromises = require('fs/promises');
+  const fsPromises = lazyFsPromises();
 
   // Override fs/promises.readdir (needed for async glob support)
   originalPromisesReaddir = fsPromises.readdir;
@@ -1065,8 +1072,6 @@ function installFsPatches() {
 
 /**
  * Install all VFS hooks: module loading hooks and fs patches.
- * Note: fs and internal modules are required here (not at top level) to avoid
- * circular dependencies during bootstrap. This module may be loaded early.
  */
 function installHooks() {
   if (hooksInstalled) {