Skip to content

Commit d96bc30

Browse files
committed
vfs: address aduh95 review feedback
Simplify srcIsDir checks in cp-sync.js and cp.js to one-liner with optional chaining and nullish coalescing. Use isPromise() instead of duck-typing in memory provider. Use FunctionPrototypeSymbolHasInstance instead of instanceof in vfs.js.
1 parent 2b95b97 commit d96bc30

4 files changed

Lines changed: 9 additions & 15 deletions

File tree

lib/internal/fs/cp/cp-sync.js

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -239,13 +239,7 @@ function onLink(destStat, src, dest, verbatimSymlinks) {
239239
}
240240
// internalModuleStat is a C++ call that can't see VFS files.
241241
const { vfsState } = require('internal/fs/utils');
242-
let srcIsDir;
243-
const vfsStat = vfsState.handlers?.statSync(src);
244-
if (vfsStat) {
245-
srcIsDir = vfsStat.isDirectory();
246-
} else {
247-
srcIsDir = fsBinding.internalModuleStat(src) === 1;
248-
}
242+
const srcIsDir = vfsState.handlers?.statSync(src)?.isDirectory() ?? fsBinding.internalModuleStat(src) === 1;
249243

250244
if (srcIsDir && isSrcSubdir(resolvedSrc, resolvedDest)) {
251245
throw new ERR_FS_CP_EINVAL({

lib/internal/fs/cp/cp.js

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -357,12 +357,7 @@ async function onLink(destStat, src, dest, opts) {
357357

358358
// internalModuleStat is a C++ call that can't see VFS files.
359359
const { vfsState } = require('internal/fs/utils');
360-
let srcIsDir;
361-
if (vfsState.handlers !== null && vfsState.handlers.statSync(src)) {
362-
srcIsDir = (await stat(src)).isDirectory();
363-
} else {
364-
srcIsDir = fsBinding.internalModuleStat(src) === 1;
365-
}
360+
const srcIsDir = vfsState.handlers?.statSync(src)?.isDirectory() ?? fsBinding.internalModuleStat(src) === 1;
366361

367362
if (srcIsDir && isSrcSubdir(resolvedSrc, resolvedDest)) {
368363
throw new ERR_FS_CP_EINVAL({

lib/internal/vfs/providers/memory.js

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ const {
1010
} = primordials;
1111

1212
const { Buffer } = require('buffer');
13+
const { isPromise } = require('util/types');
1314
const { posix: pathPosix } = require('path');
1415
const { VirtualProvider } = require('internal/vfs/provider');
1516
const { MemoryFileHandle } = require('internal/vfs/file_handle');
@@ -88,7 +89,7 @@ class MemoryEntry {
8889
getContentSync() {
8990
if (this.contentProvider !== null) {
9091
const result = this.contentProvider();
91-
if (typeof result?.then === 'function') {
92+
if (isPromise(result)) {
9293
// It's a Promise - can't use sync API
9394
throw new ERR_INVALID_STATE('cannot use sync API with async content provider');
9495
}

lib/vfs.js

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

3+
const {
4+
FunctionPrototypeSymbolHasInstance,
5+
} = primordials;
6+
37
const { VirtualFileSystem } = require('internal/vfs/file_system');
48
const { VirtualProvider } = require('internal/vfs/provider');
59
const { MemoryProvider } = require('internal/vfs/providers/memory');
@@ -16,7 +20,7 @@ const { RealFSProvider } = require('internal/vfs/providers/real');
1620
function create(provider, options) {
1721
// Handle case where first arg is options (no provider)
1822
if (provider != null &&
19-
!(provider instanceof VirtualProvider) &&
23+
!FunctionPrototypeSymbolHasInstance(VirtualProvider, provider) &&
2024
typeof provider === 'object') {
2125
options = provider;
2226
provider = undefined;

0 commit comments

Comments
 (0)