Skip to content

Commit 5e80b9e

Browse files
committed
vfs: address Aviv's review comments
- MockFSContext: use public properties instead of private + getters - MockFSContext: use ctx.restore instead of restoreFS - streams.js: use kEmptyObject for default options parameter - streams.js: use getLazy for fd module import - streams.js: remove unnecessary createVirtualReadStream wrapper - file_system.js: use VirtualReadStream class directly PR-URL: #61478
1 parent 612abc3 commit 5e80b9e

File tree

3 files changed

+27
-41
lines changed

3 files changed

+27
-41
lines changed

lib/internal/test_runner/mock/mock.js

Lines changed: 16 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -413,28 +413,21 @@ const { restore: restoreProperty } = MockPropertyContext.prototype;
413413
* Context for mocking the file system using VFS.
414414
*/
415415
class MockFSContext {
416-
#vfs;
417-
#prefix;
418-
419-
constructor(vfs, prefix) {
420-
this.#vfs = vfs;
421-
this.#prefix = prefix;
422-
}
423-
424416
/**
425-
* Gets the underlying VirtualFileSystem instance.
426-
* @returns {VirtualFileSystem}
417+
* The underlying VirtualFileSystem instance.
418+
* @type {VirtualFileSystem}
427419
*/
428-
get vfs() {
429-
return this.#vfs;
430-
}
420+
vfs;
431421

432422
/**
433-
* Gets the mount prefix for the mock file system.
434-
* @returns {string}
423+
* The mount prefix for the mock file system.
424+
* @type {string}
435425
*/
436-
get prefix() {
437-
return this.#prefix;
426+
prefix;
427+
428+
constructor(vfs, prefix) {
429+
this.vfs = vfs;
430+
this.prefix = prefix;
438431
}
439432

440433
/**
@@ -443,15 +436,15 @@ class MockFSContext {
443436
* @param {string|Buffer} content - The file content.
444437
*/
445438
addFile(filePath, content) {
446-
this.#vfs.addFile(filePath, content);
439+
this.vfs.addFile(filePath, content);
447440
}
448441

449442
/**
450443
* Adds a directory to the mock file system.
451444
* @param {string} dirPath - The path of the directory.
452445
*/
453446
addDirectory(dirPath) {
454-
this.#vfs.addDirectory(dirPath);
447+
this.vfs.addDirectory(dirPath);
455448
}
456449

457450
/**
@@ -460,20 +453,18 @@ class MockFSContext {
460453
* @returns {boolean}
461454
*/
462455
existsSync(path) {
463-
const fullPath = join(this.#prefix, path);
464-
return this.#vfs.existsSync(fullPath);
456+
const fullPath = join(this.prefix, path);
457+
return this.vfs.existsSync(fullPath);
465458
}
466459

467460
/**
468461
* Restores the file system to its original state.
469462
*/
470463
restore() {
471-
this.#vfs.unmount();
464+
this.vfs.unmount();
472465
}
473466
}
474467

475-
const { restore: restoreFS } = MockFSContext.prototype;
476-
477468
class MockTracker {
478469
#mocks = [];
479470
#timers;
@@ -832,7 +823,7 @@ class MockTracker {
832823
ArrayPrototypePush(this.#mocks, {
833824
__proto__: null,
834825
ctx,
835-
restore: restoreFS,
826+
restore: ctx.restore,
836827
});
837828

838829
return ctx;

lib/internal/vfs/file_system.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ const {
2929
createENOTDIR,
3030
createEBADF,
3131
} = require('internal/vfs/errors');
32-
const { createVirtualReadStream } = require('internal/vfs/streams');
32+
const { VirtualReadStream } = require('internal/vfs/streams');
3333
const { emitExperimentalWarning } = require('internal/util');
3434

3535
// Private symbols
@@ -882,7 +882,7 @@ class VirtualFileSystem {
882882
* @returns {ReadStream}
883883
*/
884884
createReadStream(filePath, options) {
885-
return createVirtualReadStream(this, filePath, options);
885+
return new VirtualReadStream(this, filePath, options);
886886
}
887887

888888
// ==================== Watch Operations ====================

lib/internal/vfs/streams.js

Lines changed: 9 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,12 @@ const {
66

77
const { Readable } = require('stream');
88
const { createEBADF } = require('internal/vfs/errors');
9+
const { getLazy, kEmptyObject } = require('internal/util');
10+
11+
// Lazy-load fd module to avoid circular dependency
12+
const lazyGetVirtualFd = getLazy(
13+
() => require('internal/vfs/fd').getVirtualFd,
14+
);
915

1016
/**
1117
* A readable stream for virtual files.
@@ -25,7 +31,7 @@ class VirtualReadStream extends Readable {
2531
* @param {string} filePath The path to the file
2632
* @param {object} [options] Stream options
2733
*/
28-
constructor(vfs, filePath, options = {}) {
34+
constructor(vfs, filePath, options = kEmptyObject) {
2935
const {
3036
start = 0,
3137
end = Infinity,
@@ -81,7 +87,8 @@ class VirtualReadStream extends Readable {
8187
// Load content on first read (lazy loading)
8288
if (this.#content === null) {
8389
try {
84-
const vfd = require('internal/vfs/fd').getVirtualFd(this.#fd);
90+
const getVirtualFd = lazyGetVirtualFd();
91+
const vfd = getVirtualFd(this.#fd);
8592
if (!vfd) {
8693
this.destroy(createEBADF('read'));
8794
return;
@@ -146,18 +153,6 @@ class VirtualReadStream extends Readable {
146153
}
147154
}
148155

149-
/**
150-
* Creates a readable stream for a virtual file.
151-
* @param {VirtualFileSystem} vfs The VFS instance
152-
* @param {string} filePath The path to the file
153-
* @param {object} [options] Stream options
154-
* @returns {VirtualReadStream}
155-
*/
156-
function createVirtualReadStream(vfs, filePath, options = {}) {
157-
return new VirtualReadStream(vfs, filePath, options);
158-
}
159-
160156
module.exports = {
161-
createVirtualReadStream,
162157
VirtualReadStream,
163158
};

0 commit comments

Comments
 (0)