Skip to content

Commit 27d4157

Browse files
committed
vfs: address aduh95 review comments
- Use kEmptyObject instead of {} for default options - Add validateObject for options parameter validation - Use isURL() instead of instanceof URL - Use toPathIfFileURL() instead of manual URL handling - Use FunctionPrototypeCall instead of .call()
1 parent 9a808b1 commit 27d4157

1 file changed

Lines changed: 27 additions & 18 deletions

File tree

lib/internal/vfs/module_hooks.js

Lines changed: 27 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -4,13 +4,16 @@ const {
44
ArrayPrototypeIndexOf,
55
ArrayPrototypePush,
66
ArrayPrototypeSplice,
7+
FunctionPrototypeCall,
78
StringPrototypeEndsWith,
89
StringPrototypeStartsWith,
910
} = primordials;
1011

1112
const { dirname, isAbsolute, resolve } = require('path');
1213
const { normalizePath } = require('internal/vfs/router');
13-
const { pathToFileURL, fileURLToPath, URL } = require('internal/url');
14+
const { isURL, pathToFileURL, fileURLToPath, toPathIfFileURL, URL } = require('internal/url');
15+
const { kEmptyObject } = require('internal/util');
16+
const { validateObject } = require('internal/validators');
1417
const { createENOENT } = require('internal/vfs/errors');
1518

1619
// Registry of active VFS instances
@@ -590,14 +593,14 @@ function installHooks() {
590593
// Override fs/promises.lstat (needed for async glob support)
591594
originalPromisesLstat = fsPromises.lstat;
592595
fsPromises.lstat = async function lstat(path, options) {
593-
if (typeof path === 'string' || path instanceof URL) {
594-
const pathStr = typeof path === 'string' ? path : path.pathname;
596+
if (typeof path === 'string' || isURL(path)) {
597+
const pathStr = toPathIfFileURL(path);
595598
const vfsResult = await findVFSForLstatAsync(pathStr);
596599
if (vfsResult !== null) {
597600
return vfsResult.stats;
598601
}
599602
}
600-
return originalPromisesLstat.call(fsPromises, path, options);
603+
return FunctionPrototypeCall(originalPromisesLstat, fsPromises, path, options);
601604
};
602605

603606
// Override fs.watch
@@ -607,11 +610,14 @@ function installHooks() {
607610
if (typeof options === 'function') {
608611
listener = options;
609612
options = kEmptyObject;
613+
} else if (options != null) {
614+
validateObject(options, 'options');
615+
} else {
616+
options = kEmptyObject;
610617
}
611-
options ??= {};
612618

613-
if (typeof filename === 'string' || filename instanceof URL) {
614-
const pathStr = typeof filename === 'string' ? filename : filename.pathname;
619+
if (typeof filename === 'string' || isURL(filename)) {
620+
const pathStr = toPathIfFileURL(filename);
615621
const vfsResult = findVFSForWatch(pathStr);
616622
if (vfsResult !== null) {
617623
return vfsResult.vfs.watch(pathStr, options, listener);
@@ -626,45 +632,48 @@ function installHooks() {
626632
// Handle optional options argument
627633
if (typeof options === 'function') {
628634
listener = options;
629-
options = {};
635+
options = kEmptyObject;
636+
} else if (options != null) {
637+
validateObject(options, 'options');
638+
} else {
639+
options = kEmptyObject;
630640
}
631-
options ??= {};
632641

633-
if (typeof filename === 'string' || filename instanceof URL) {
634-
const pathStr = typeof filename === 'string' ? filename : filename.pathname;
642+
if (typeof filename === 'string' || isURL(filename)) {
643+
const pathStr = toPathIfFileURL(filename);
635644
const vfsResult = findVFSForWatch(pathStr);
636645
if (vfsResult !== null) {
637646
return vfsResult.vfs.watchFile(pathStr, options, listener);
638647
}
639648
}
640-
return originalWatchFile.call(fs, filename, options, listener);
649+
return FunctionPrototypeCall(originalWatchFile, fs, filename, options, listener);
641650
};
642651

643652
// Override fs.unwatchFile
644653
originalUnwatchFile = fs.unwatchFile;
645654
fs.unwatchFile = function unwatchFile(filename, listener) {
646-
if (typeof filename === 'string' || filename instanceof URL) {
647-
const pathStr = typeof filename === 'string' ? filename : filename.pathname;
655+
if (typeof filename === 'string' || isURL(filename)) {
656+
const pathStr = toPathIfFileURL(filename);
648657
const vfsResult = findVFSForWatch(pathStr);
649658
if (vfsResult !== null) {
650659
vfsResult.vfs.unwatchFile(pathStr, listener);
651660
return;
652661
}
653662
}
654-
return originalUnwatchFile.call(fs, filename, listener);
663+
return FunctionPrototypeCall(originalUnwatchFile, fs, filename, listener);
655664
};
656665

657666
// Override fs/promises.watch
658667
originalPromisesWatch = fsPromises.watch;
659668
fsPromises.watch = function watch(filename, options) {
660-
if (typeof filename === 'string' || filename instanceof URL) {
661-
const pathStr = typeof filename === 'string' ? filename : filename.pathname;
669+
if (typeof filename === 'string' || isURL(filename)) {
670+
const pathStr = toPathIfFileURL(filename);
662671
const vfsResult = findVFSForWatch(pathStr);
663672
if (vfsResult !== null) {
664673
return vfsResult.vfs.promises.watch(pathStr, options);
665674
}
666675
}
667-
return originalPromisesWatch.call(fsPromises, filename, options);
676+
return FunctionPrototypeCall(originalPromisesWatch, fsPromises, filename, options);
668677
};
669678

670679
// Register ESM hooks using Module.registerHooks

0 commit comments

Comments
 (0)