Skip to content

Commit 30ee249

Browse files
committed
vfs: address review feedback on path utilities and providers
- Remove redundant #getBaseName/#getParentPath from MemoryProvider, use pathPosix.basename/dirname directly - Remove redundant getBaseName/getParentPath/splitPath from router.js, keep only functions with non-trivial VFS-specific logic - Convert RealFSProvider constant getters (readonly, supportsSymlinks) to readonly properties via ObjectDefineProperty - Fix joinVFSPath/normalizeVFSPath to detect Windows drive-letter paths by checking for ':' at position 1 instead of checking for leading '/', so bare '/' is always treated as a POSIX VFS path - Update test-vfs-internals.js to match router.js export changes
1 parent 632979a commit 30ee249

File tree

5 files changed

+33
-110
lines changed

5 files changed

+33
-110
lines changed

lib/internal/vfs/file_system.js

Lines changed: 19 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -18,31 +18,39 @@ const pathPosix = path.posix;
1818
const { isAbsolute, resolve: resolvePath } = path;
1919

2020
/**
21-
* Normalizes a VFS path. Uses POSIX normalization for Unix-style paths (starting with /)
22-
* and platform normalization for Windows drive letter paths.
21+
* Normalizes a VFS path. Uses POSIX normalization for all paths except
22+
* Windows drive-letter paths (e.g. C:\), which use platform normalization.
23+
*
24+
* On Windows, a bare '/' prefix is treated as a POSIX VFS path, not as the
25+
* current-drive root.
2326
* @param {string} inputPath The path to normalize
2427
* @returns {string} The normalized path
2528
*/
2629
function normalizeVFSPath(inputPath) {
27-
// If path starts with / (Unix-style), use posix normalization to preserve forward slashes
28-
if (inputPath.startsWith('/')) {
29-
return pathPosix.normalize(inputPath);
30+
if (inputPath.length >= 3 && inputPath[1] === ':') {
31+
// Windows drive-letter path (e.g. C:\app)
32+
return path.normalize(inputPath);
3033
}
31-
// Otherwise use platform normalization (for Windows drive letters like C:\)
32-
return path.normalize(inputPath);
34+
return pathPosix.normalize(inputPath);
3335
}
3436

3537
/**
36-
* Joins VFS paths. Uses POSIX join for Unix-style base paths.
38+
* Joins VFS paths. Uses POSIX join for Unix-style base paths and platform
39+
* join for Windows drive-letter paths (e.g. C:\).
40+
*
41+
* On Windows, a bare '/' prefix is treated as a POSIX VFS path, not as the
42+
* current-drive root. Only explicit drive-letter paths like 'C:\app' use
43+
* platform path.join.
3744
* @param {string} base The base path
3845
* @param {string} part The path part to join
3946
* @returns {string} The joined path
4047
*/
4148
function joinVFSPath(base, part) {
42-
if (base.startsWith('/')) {
43-
return pathPosix.join(base, part);
49+
if (base.length >= 3 && base[1] === ':') {
50+
// Windows drive-letter path (e.g. C:\app)
51+
return path.join(base, part);
4452
}
45-
return path.join(base, part);
53+
return pathPosix.join(base, part);
4654
}
4755
const {
4856
isUnderMountPoint,

lib/internal/vfs/providers/memory.js

Lines changed: 10 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -195,26 +195,6 @@ class MemoryProvider extends VirtualProvider {
195195
return path.slice(1).split('/');
196196
}
197197

198-
/**
199-
* Gets the parent path.
200-
* @param {string} path Normalized path
201-
* @returns {string|null} Parent path or null for root
202-
*/
203-
#getParentPath(path) {
204-
if (path === '/') {
205-
return null;
206-
}
207-
return pathPosix.dirname(path);
208-
}
209-
210-
/**
211-
* Gets the base name.
212-
* @param {string} path Normalized path
213-
* @returns {string} Base name
214-
*/
215-
#getBaseName(path) {
216-
return pathPosix.basename(path);
217-
}
218198

219199
/**
220200
* Resolves a symlink target to an absolute path.
@@ -227,7 +207,7 @@ class MemoryProvider extends VirtualProvider {
227207
return this.#normalizePath(target);
228208
}
229209
// Relative target: resolve against symlink's parent directory
230-
const parentPath = this.#getParentPath(symlinkPath) || '/';
210+
const parentPath = pathPosix.dirname(symlinkPath);
231211
return this.#normalizePath(pathPosix.join(parentPath, target));
232212
}
233213

@@ -323,10 +303,10 @@ class MemoryProvider extends VirtualProvider {
323303
* @returns {MemoryEntry} The parent directory entry
324304
*/
325305
#ensureParent(path, create, syscall) {
326-
const parentPath = this.#getParentPath(path);
327-
if (parentPath === null) {
306+
if (path === '/') {
328307
return this[kRoot];
329308
}
309+
const parentPath = pathPosix.dirname(path);
330310

331311
const segments = this.#splitPath(parentPath);
332312
let current = this[kRoot];
@@ -458,7 +438,7 @@ class MemoryProvider extends VirtualProvider {
458438
if (err.code !== 'ENOENT' || !isCreate) throw err;
459439
// Create the file
460440
const parent = this.#ensureParent(normalized, true, 'open');
461-
const name = this.#getBaseName(normalized);
441+
const name = pathPosix.basename(normalized);
462442
entry = new MemoryEntry(TYPE_FILE, { mode });
463443
entry.content = Buffer.alloc(0);
464444
parent.children.set(name, entry);
@@ -570,7 +550,7 @@ class MemoryProvider extends VirtualProvider {
570550
}
571551
} else {
572552
const parent = this.#ensureParent(normalized, false, 'mkdir');
573-
const name = this.#getBaseName(normalized);
553+
const name = pathPosix.basename(normalized);
574554
const entry = new MemoryEntry(TYPE_DIR, { mode: options?.mode });
575555
entry.children = new SafeMap();
576556
parent.children.set(name, entry);
@@ -600,7 +580,7 @@ class MemoryProvider extends VirtualProvider {
600580
}
601581

602582
const parent = this.#ensureParent(normalized, false, 'rmdir');
603-
const name = this.#getBaseName(normalized);
583+
const name = pathPosix.basename(normalized);
604584
parent.children.delete(name);
605585
}
606586

@@ -621,7 +601,7 @@ class MemoryProvider extends VirtualProvider {
621601
}
622602

623603
const parent = this.#ensureParent(normalized, false, 'unlink');
624-
const name = this.#getBaseName(normalized);
604+
const name = pathPosix.basename(normalized);
625605
parent.children.delete(name);
626606
}
627607

@@ -642,12 +622,12 @@ class MemoryProvider extends VirtualProvider {
642622

643623
// Remove from old location
644624
const oldParent = this.#ensureParent(normalizedOld, false, 'rename');
645-
const oldName = this.#getBaseName(normalizedOld);
625+
const oldName = pathPosix.basename(normalizedOld);
646626
oldParent.children.delete(oldName);
647627

648628
// Add to new location
649629
const newParent = this.#ensureParent(normalizedNew, true, 'rename');
650-
const newName = this.#getBaseName(normalizedNew);
630+
const newName = pathPosix.basename(normalizedNew);
651631
newParent.children.set(newName, entry);
652632
}
653633

@@ -684,7 +664,7 @@ class MemoryProvider extends VirtualProvider {
684664
}
685665

686666
const parent = this.#ensureParent(normalized, true, 'symlink');
687-
const name = this.#getBaseName(normalized);
667+
const name = pathPosix.basename(normalized);
688668
const entry = new MemoryEntry(TYPE_SYMLINK);
689669
entry.target = target;
690670
parent.children.set(name, entry);

lib/internal/vfs/providers/real.js

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
'use strict';
22

33
const {
4+
ObjectDefineProperty,
45
Promise,
56
StringPrototypeStartsWith,
67
} = primordials;
@@ -172,6 +173,8 @@ class RealFSProvider extends VirtualProvider {
172173
}
173174
// Resolve to absolute path and normalize
174175
this.#rootPath = path.resolve(rootPath);
176+
ObjectDefineProperty(this, 'readonly', { __proto__: null, value: false });
177+
ObjectDefineProperty(this, 'supportsSymlinks', { __proto__: null, value: true });
175178
}
176179

177180
/**
@@ -182,14 +185,6 @@ class RealFSProvider extends VirtualProvider {
182185
return this.#rootPath;
183186
}
184187

185-
get readonly() {
186-
return false;
187-
}
188-
189-
get supportsSymlinks() {
190-
return true;
191-
}
192-
193188
/**
194189
* Resolves a VFS path to a real filesystem path.
195190
* Ensures the path doesn't escape the root directory.

lib/internal/vfs/router.js

Lines changed: 1 addition & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -2,47 +2,10 @@
22

33
const {
44
StringPrototypeSlice,
5-
StringPrototypeSplit,
65
StringPrototypeStartsWith,
76
} = primordials;
87

9-
const { isAbsolute, posix: pathPosix } = require('path');
10-
11-
/**
12-
* Splits a path into segments.
13-
* VFS paths are always normalized to use forward slashes.
14-
* @param {string} normalizedPath A normalized absolute path
15-
* @returns {string[]} Path segments
16-
*/
17-
function splitPath(normalizedPath) {
18-
if (normalizedPath === '/') {
19-
return [];
20-
}
21-
// Remove leading slash and split by forward slash (VFS internal format)
22-
return StringPrototypeSplit(StringPrototypeSlice(normalizedPath, 1), '/');
23-
}
24-
25-
/**
26-
* Gets the parent path of a normalized path.
27-
* VFS paths are always normalized to use forward slashes.
28-
* @param {string} normalizedPath A normalized absolute path
29-
* @returns {string|null} The parent path, or null if at root
30-
*/
31-
function getParentPath(normalizedPath) {
32-
if (normalizedPath === '/') {
33-
return null;
34-
}
35-
return pathPosix.dirname(normalizedPath);
36-
}
37-
38-
/**
39-
* Gets the base name from a normalized path.
40-
* @param {string} normalizedPath A normalized absolute path
41-
* @returns {string} The base name
42-
*/
43-
function getBaseName(normalizedPath) {
44-
return pathPosix.basename(normalizedPath);
45-
}
8+
const { isAbsolute } = require('path');
469

4710
/**
4811
* Checks if a path is under a mount point.
@@ -84,9 +47,6 @@ function getRelativePath(normalizedPath, mountPoint) {
8447
}
8548

8649
module.exports = {
87-
splitPath,
88-
getParentPath,
89-
getBaseName,
9050
isUnderMountPoint,
9151
getRelativePath,
9252
isAbsolutePath: isAbsolute,

test/parallel/test-vfs-internals.js

Lines changed: 0 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -8,31 +8,11 @@ const assert = require('assert');
88

99
// === Router utility functions ===
1010
const {
11-
splitPath,
12-
getParentPath,
13-
getBaseName,
1411
isUnderMountPoint,
1512
getRelativePath,
1613
isAbsolutePath,
1714
} = require('internal/vfs/router');
1815

19-
// splitPath
20-
assert.deepStrictEqual(splitPath('/'), []);
21-
assert.deepStrictEqual(splitPath('/foo'), ['foo']);
22-
assert.deepStrictEqual(splitPath('/foo/bar'), ['foo', 'bar']);
23-
assert.deepStrictEqual(splitPath('/a/b/c/d'), ['a', 'b', 'c', 'd']);
24-
25-
// getParentPath
26-
assert.strictEqual(getParentPath('/'), null);
27-
assert.strictEqual(getParentPath('/foo'), '/');
28-
assert.strictEqual(getParentPath('/foo/bar'), '/foo');
29-
assert.strictEqual(getParentPath('/a/b/c'), '/a/b');
30-
31-
// getBaseName
32-
assert.strictEqual(getBaseName('/foo'), 'foo');
33-
assert.strictEqual(getBaseName('/foo/bar'), 'bar');
34-
assert.strictEqual(getBaseName('/a/b/c.txt'), 'c.txt');
35-
3616
// isAbsolutePath
3717
assert.strictEqual(isAbsolutePath('/foo'), true);
3818
assert.strictEqual(isAbsolutePath('foo'), false);

0 commit comments

Comments
 (0)