Skip to content

Commit 580bc2a

Browse files
committed
vfs: address aduh95 code review comments
- Use kEmptyObject, optional chaining, primordials in memory.js - Simplify openSync try/catch and readdir iteration - Use parentPath instead of recomputing in ensureParent - Explain readFileSync wrapper purpose in helpers.js - Revert opts→options JSDoc rename in package_json_reader.js - Restore internalFsBinding to module level in package_json_reader.js - Add POSIX separator comment in embedding.js
1 parent 8ad97f1 commit 580bc2a

File tree

4 files changed

+22
-24
lines changed

4 files changed

+22
-24
lines changed

lib/internal/main/embedding.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,7 @@ function embedderRunCjs(content, filename) {
9393
// and use createRequire for a fully-featured require function.
9494
let requireFn;
9595
if (seaVfsActive && seaVfsMountPoint) {
96+
// VFS paths always use POSIX separators regardless of platform.
9697
customModule.filename = seaVfsMountPoint + '/' + path.basename(filename);
9798
customModule.paths = Module._nodeModulePaths(
9899
path.dirname(customModule.filename));

lib/internal/modules/helpers.js

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,8 @@ function toRealPath(requestPath) {
6868
}
6969

7070
/**
71-
* Reads a file from disk.
71+
* Reads a file from disk via the `fs` module reference, which may be
72+
* monkey-patched by VFS to intercept reads for virtual paths.
7273
* @param {string} path The file path
7374
* @param {string|object} options Read options
7475
* @returns {string|Buffer}

lib/internal/modules/package_json_reader.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ const {
2525
} = require('internal/errors');
2626
const { kEmptyObject } = require('internal/util');
2727
const modulesBinding = internalBinding('modules');
28+
const internalFsBinding = internalBinding('fs');
2829
const path = require('path');
2930
const { validateString } = require('internal/validators');
3031

@@ -115,7 +116,7 @@ const requiresJSONParse = (value) => (value !== undefined && (value[0] === '[' |
115116
* base?: URL | string,
116117
* specifier?: URL | string,
117118
* isESM?: boolean,
118-
* }} opts
119+
* }} options
119120
* @returns {PackageConfig}
120121
*/
121122
function read(jsonPath, { base, specifier, isESM } = kEmptyObject) {
@@ -275,7 +276,6 @@ function getPackageJSONURL(specifier, base) {
275276
}
276277
}
277278

278-
const internalFsBinding = internalBinding('fs');
279279
let packageJSONUrl = new URL(`./node_modules/${packageName}/package.json`, base);
280280
let packageJSONPath = fileURLToPath(packageJSONUrl);
281281
let lastPath;

lib/internal/vfs/providers/memory.js

Lines changed: 17 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,11 @@
11
'use strict';
22

33
const {
4+
ArrayFrom,
45
ArrayPrototypePush,
56
DateNow,
67
SafeMap,
8+
StringPrototypeReplaceAll,
79
Symbol,
810
} = primordials;
911

@@ -37,6 +39,7 @@ const {
3739
createSymlinkStats,
3840
} = require('internal/vfs/stats');
3941
const { Dirent } = require('internal/fs/utils');
42+
const { kEmptyObject } = require('internal/util');
4043
const {
4144
fs: {
4245
UV_DIRENT_FILE,
@@ -62,7 +65,7 @@ const kMaxSymlinkDepth = 40;
6265
* Internal entry representation for MemoryProvider.
6366
*/
6467
class MemoryEntry {
65-
constructor(type, options = {}) {
68+
constructor(type, options = kEmptyObject) {
6669
this.type = type;
6770
this.mode = options.mode ?? (type === TYPE_DIR ? 0o755 : 0o644);
6871
this.content = null; // For files - static Buffer content
@@ -85,7 +88,7 @@ class MemoryEntry {
8588
getContentSync() {
8689
if (this.contentProvider !== null) {
8790
const result = this.contentProvider();
88-
if (result && typeof result.then === 'function') {
91+
if (typeof result?.then === 'function') {
8992
// It's a Promise - can't use sync API
9093
throw new ERR_INVALID_STATE('cannot use sync API with async content provider');
9194
}
@@ -171,9 +174,9 @@ class MemoryProvider extends VirtualProvider {
171174
*/
172175
#normalizePath(path) {
173176
// Convert backslashes to forward slashes
174-
let normalized = path.replace(/\\/g, '/');
177+
let normalized = StringPrototypeReplaceAll(path, '\\', '/');
175178
// Ensure absolute path
176-
if (!normalized.startsWith('/')) {
179+
if (normalized[0] !== '/') {
177180
normalized = '/' + normalized;
178181
}
179182
// Use path.posix.normalize to resolve . and ..
@@ -368,8 +371,7 @@ class MemoryProvider extends VirtualProvider {
368371
}
369372

370373
// Ensure final directory is populated
371-
const finalPath = pathPosix.join('/', ...segments);
372-
this.#ensurePopulated(current, finalPath);
374+
this.#ensurePopulated(current, parentPath);
373375

374376
return current;
375377
}
@@ -453,16 +455,13 @@ class MemoryProvider extends VirtualProvider {
453455
try {
454456
entry = this.#getEntry(normalized, 'open');
455457
} catch (err) {
456-
if (err.code === 'ENOENT' && isCreate) {
457-
// Create the file
458-
const parent = this.#ensureParent(normalized, true, 'open');
459-
const name = this.#getBaseName(normalized);
460-
entry = new MemoryEntry(TYPE_FILE, { mode });
461-
entry.content = Buffer.alloc(0);
462-
parent.children.set(name, entry);
463-
} else {
464-
throw err;
465-
}
458+
if (err.code !== 'ENOENT' || !isCreate) throw err;
459+
// Create the file
460+
const parent = this.#ensureParent(normalized, true, 'open');
461+
const name = this.#getBaseName(normalized);
462+
entry = new MemoryEntry(TYPE_FILE, { mode });
463+
entry.content = Buffer.alloc(0);
464+
parent.children.set(name, entry);
466465
}
467466

468467
if (entry.isDirectory()) {
@@ -509,13 +508,10 @@ class MemoryProvider extends VirtualProvider {
509508
// Ensure directory is populated (for lazy population)
510509
this.#ensurePopulated(entry, path);
511510

512-
const names = [...entry.children.keys()];
513-
514511
if (options?.withFileTypes) {
515512
const normalized = this.#normalizePath(path);
516513
const dirents = [];
517-
for (const name of names) {
518-
const childEntry = entry.children.get(name);
514+
for (const { 0: name, 1: childEntry } of entry.children) {
519515
let type;
520516
if (childEntry.isSymbolicLink()) {
521517
type = UV_DIRENT_LINK;
@@ -529,7 +525,7 @@ class MemoryProvider extends VirtualProvider {
529525
return dirents;
530526
}
531527

532-
return names;
528+
return ArrayFrom(entry.children.keys());
533529
}
534530

535531
async readdir(path, options) {

0 commit comments

Comments
 (0)