Skip to content

Commit

Permalink
fs: introduce dirent.parentPath
Browse files Browse the repository at this point in the history
The goal is to replace `dirent.path` using a name that's less likely to
create confusion.
`dirent.path` value has not been stable, moving it to a different
property name should avoid breaking some upgrading user expectations.

PR-URL: #50976
Backport-PR-URL: #51021
Reviewed-By: Ethan Arrowood <ethan@arrowood.dev>
Reviewed-By: LiviaMedeiros <livia@cirno.name>
  • Loading branch information
aduh95 authored and richardlau committed Mar 18, 2024
1 parent 96514a8 commit 26e8f77
Show file tree
Hide file tree
Showing 4 changed files with 34 additions and 13 deletions.
13 changes: 13 additions & 0 deletions doc/api/fs.md
Original file line number Diff line number Diff line change
Expand Up @@ -6437,6 +6437,19 @@ The file name that this {fs.Dirent} object refers to. The type of this
value is determined by the `options.encoding` passed to [`fs.readdir()`][] or
[`fs.readdirSync()`][].
#### `dirent.parentPath`
<!-- YAML
added:
- REPLACEME
-->
> Stability: 1 – Experimental
* {string}
The path to the parent directory of the file this {fs.Dirent} object refers to.
#### `dirent.path`
<!-- YAML
Expand Down
3 changes: 2 additions & 1 deletion lib/internal/fs/dir.js
Original file line number Diff line number Diff line change
Expand Up @@ -152,9 +152,10 @@ class Dir {
ArrayPrototypePush(
this[kDirBufferedEntries],
getDirent(
pathModule.join(path, result[i]),
path,
result[i],
result[i + 1],
true, // Quirk to not introduce a breaking change.
),
);
}
Expand Down
23 changes: 14 additions & 9 deletions lib/internal/fs/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -161,9 +161,10 @@ function assertEncoding(encoding) {
}

class Dirent {
constructor(name, type, path) {
constructor(name, type, path, filepath = path && join(path, name)) {
this.name = name;
this.path = path;
this.parentPath = path;
this.path = filepath;
this[kType] = type;
}

Expand Down Expand Up @@ -197,8 +198,8 @@ class Dirent {
}

class DirentFromStats extends Dirent {
constructor(name, stats, path) {
super(name, null, path);
constructor(name, stats, path, filepath) {
super(name, null, path, filepath);
this[kStats] = stats;
}
}
Expand Down Expand Up @@ -233,7 +234,7 @@ function join(path, name) {
}

if (typeof path === 'string' && typeof name === 'string') {
return pathModule.basename(path) === name ? path : pathModule.join(path, name);
return pathModule.join(path, name);
}

if (isUint8Array(path) && isUint8Array(name)) {
Expand Down Expand Up @@ -268,7 +269,7 @@ function getDirents(path, { 0: names, 1: types }, callback) {
callback(err);
return;
}
names[idx] = new DirentFromStats(name, stats, path);
names[idx] = new DirentFromStats(name, stats, path, filepath);
if (--toFinish === 0) {
callback(null, names);
}
Expand Down Expand Up @@ -304,17 +305,21 @@ function getDirent(path, name, type, callback) {
callback(err);
return;
}
callback(null, new DirentFromStats(name, stats, filepath));
callback(null, new DirentFromStats(name, stats, path, filepath));
});
} else {
callback(null, new Dirent(name, type, path));
}
} else if (type === UV_DIRENT_UNKNOWN) {
const filepath = join(path, name);
const stats = lazyLoadFs().lstatSync(filepath);
return new DirentFromStats(name, stats, path);
} else {
// callback === true: Quirk to not introduce a breaking change.
return new DirentFromStats(name, stats, path, callback === true ? filepath : path);
} else if (callback === true) {
// callback === true: Quirk to not introduce a breaking change.
return new Dirent(name, type, path);
} else {
return new Dirent(name, type, path, path);
}
}

Expand Down
8 changes: 5 additions & 3 deletions test/parallel/test-fs-opendir.js
Original file line number Diff line number Diff line change
Expand Up @@ -48,9 +48,11 @@ const invalidCallbackObj = {
const entries = files.map(() => {
const dirent = dir.readSync();
assertDirent(dirent);
return dirent.name;
});
assert.deepStrictEqual(files, entries.sort());
return { name: dirent.name, path: dirent.path, parentPath: dirent.parentPath, toString() { return dirent.name; } };
}).sort();
assert.deepStrictEqual(entries.map((d) => d.name), files);
assert.deepStrictEqual(entries.map((d) => d.path), files.map((name) => path.join(testDir, name)));
assert.deepStrictEqual(entries.map((d) => d.parentPath), Array(entries.length).fill(testDir));

// dir.read should return null when no more entries exist
assert.strictEqual(dir.readSync(), null);
Expand Down

0 comments on commit 26e8f77

Please sign in to comment.