vfs: address code review feedback from @jasnell

- 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 #18, #19, #21, #23, #24, #29.

Author: mcollina

lib/internal/vfs/file_system.js

@@ -10,6 +10,7 @@ const {
     ERR_INVALID_STATE,
   },
 } = require('internal/errors');
+const { validateBoolean } = require('internal/validators');
 const { MemoryProvider } = require('internal/vfs/providers/memory');
 const {
   normalizePath,
@@ -81,6 +82,14 @@ class VirtualFileSystem {
       }
     }
 
+    // Validate boolean options
+    if (options.moduleHooks !== undefined) {
+      validateBoolean(options.moduleHooks, 'options.moduleHooks');
+    }
+    if (options.virtualCwd !== undefined) {
+      validateBoolean(options.virtualCwd, 'options.virtualCwd');
+    }
+
     this[kProvider] = provider ?? new MemoryProvider();
     this[kMountPoint] = null;
     this[kMounted] = false;
@@ -181,7 +190,7 @@ class VirtualFileSystem {
 
     // If virtual cwd is enabled and set, resolve relative to it
     if (this[kVirtualCwdEnabled] && this[kVirtualCwd] !== null) {
-      const resolved = this[kVirtualCwd] + '/' + inputPath;
+      const resolved = `${this[kVirtualCwd]}/${inputPath}`;
       return normalizePath(resolved);
     }
 

lib/internal/vfs/providers/memory.js

@@ -63,9 +63,10 @@ class MemoryEntry {
     this.children = null; // For directories
     this.populate = null; // For directories - lazy population callback
     this.populated = true; // For directories - has populate been called?
-    this.mtime = DateNow();
-    this.ctime = DateNow();
-    this.birthtime = DateNow();
+    const now = DateNow();
+    this.mtime = now;
+    this.ctime = now;
+    this.birthtime = now;
   }
 
   /**

lib/internal/vfs/streams.js

@@ -11,6 +11,15 @@ const { createEBADF } = require('internal/vfs/errors');
  * A readable stream for virtual files.
  */
 class VirtualReadStream extends Readable {
+  #vfs;
+  #path;
+  #fd = null;
+  #end;
+  #pos;
+  #content = null;
+  #destroyed = false;
+  #autoClose;
+
   /**
    * @param {VirtualFileSystem} vfs The VFS instance
    * @param {string} filePath The path to the file
@@ -27,38 +36,33 @@ class VirtualReadStream extends Readable {
 
     super({ ...streamOptions, highWaterMark, encoding });
 
-    this._vfs = vfs;
-    this._path = filePath;
-    this._fd = null;
-    this._start = start;
-    this._end = end;
-    this._pos = start;
-    this._content = null;
-    this._destroyed = false;
-    this._autoClose = options.autoClose !== false;
+    this.#vfs = vfs;
+    this.#path = filePath;
+    this.#end = end;
+    this.#pos = start;
+    this.#autoClose = options.autoClose !== false;
 
     // Open the file on next tick so listeners can be attached
-    process.nextTick(() => this._openFile());
+    process.nextTick(() => this.#openFile());
   }
 
   /**
    * Gets the file path.
    * @returns {string}
    */
   get path() {
-    return this._path;
+    return this.#path;
   }
 
   /**
    * Opens the virtual file.
    * Events are emitted synchronously within this method, which runs
    * asynchronously via process.nextTick - matching real fs behavior.
-   * @private
    */
-  _openFile() {
+  #openFile() {
     try {
-      this._fd = this._vfs.openSync(this._path);
-      this.emit('open', this._fd);
+      this.#fd = this.#vfs.openSync(this.#path);
+      this.emit('open', this.#fd);
       this.emit('ready');
     } catch (err) {
       this.destroy(err);
@@ -68,23 +72,22 @@ class VirtualReadStream extends Readable {
   /**
    * Implements the readable _read method.
    * @param {number} size Number of bytes to read
-   * @private
    */
   _read(size) {
-    if (this._destroyed || this._fd === null) {
+    if (this.#destroyed || this.#fd === null) {
       return;
     }
 
     // Load content on first read (lazy loading)
-    if (this._content === null) {
+    if (this.#content === null) {
       try {
-        const vfd = require('internal/vfs/fd').getVirtualFd(this._fd);
+        const vfd = require('internal/vfs/fd').getVirtualFd(this.#fd);
         if (!vfd) {
           this.destroy(createEBADF('read'));
           return;
         }
         // Use the file handle's readFileSync to get content
-        this._content = vfd.entry.readFileSync();
+        this.#content = vfd.entry.readFileSync();
       } catch (err) {
         this.destroy(err);
         return;
@@ -93,53 +96,51 @@ class VirtualReadStream extends Readable {
 
     // Calculate how much to read
     // Note: end is inclusive, so we use end + 1 for the upper bound
-    const endPos = this._end === Infinity ? this._content.length : this._end + 1;
-    const remaining = MathMin(endPos, this._content.length) - this._pos;
+    const endPos = this.#end === Infinity ? this.#content.length : this.#end + 1;
+    const remaining = MathMin(endPos, this.#content.length) - this.#pos;
     if (remaining <= 0) {
       this.push(null);
-      // Note: _close() will be called by _destroy() when autoClose is true
+      // Note: #close() will be called by _destroy() when autoClose is true
       return;
     }
 
     const bytesToRead = MathMin(size, remaining);
-    const chunk = this._content.subarray(this._pos, this._pos + bytesToRead);
-    this._pos += bytesToRead;
+    const chunk = this.#content.subarray(this.#pos, this.#pos + bytesToRead);
+    this.#pos += bytesToRead;
 
     this.push(chunk);
 
     // Check if we've reached the end
-    if (this._pos >= endPos || this._pos >= this._content.length) {
+    if (this.#pos >= endPos || this.#pos >= this.#content.length) {
       this.push(null);
-      // Note: _close() will be called by _destroy() when autoClose is true
+      // Note: #close() will be called by _destroy() when autoClose is true
     }
   }
 
   /**
    * Closes the file descriptor.
    * Note: Does not emit 'close' - the base Readable class handles that.
-   * @private
    */
-  _close() {
-    if (this._fd !== null) {
+  #close() {
+    if (this.#fd !== null) {
       try {
-        this._vfs.closeSync(this._fd);
+        this.#vfs.closeSync(this.#fd);
       } catch {
         // Ignore close errors
       }
-      this._fd = null;
+      this.#fd = null;
     }
   }
 
   /**
    * Implements the readable _destroy method.
    * @param {Error|null} err The error
    * @param {Function} callback Callback
-   * @private
    */
   _destroy(err, callback) {
-    this._destroyed = true;
-    if (this._autoClose) {
-      this._close();
+    this.#destroyed = true;
+    if (this.#autoClose) {
+      this.#close();
     }
     callback(err);
   }

lib/vfs.js

@@ -11,10 +11,10 @@ const { MemoryProvider } = require('internal/vfs/providers/memory');
 const { RealFSProvider } = require('internal/vfs/providers/real');
 
 // SEAProvider is lazy-loaded to avoid loading SEA bindings when not needed
-let SEAProvider = null;
+let SEAProvider;
 
 function getSEAProvider() {
-  if (SEAProvider === null) {
+  if (SEAProvider === undefined) {
     try {
       SEAProvider = require('internal/vfs/providers/sea').SEAProvider;
     } catch {