Skip to content

Commit 35d1c50

Browse files
committed
fs: add scandirSync(), rename internals to scandir
This is a follow-up to nodejs/node#29349 and a precursor to deprecating both `fs.readdir()` and `fs.readdirSync()`. This also updates the documentation which has been incorrect for some 10 years saying that these did `readdir(3)` when in reality they do `scandir(3)`. Only `scandirSync()` is introduced as "async scandir(3)" is kind of a trap, given that it returns the whole list of entries at once, regardless of how many there are. Since in many cases we'd also want to get dirents for them (i.e. `stat`-ing each and every one), this becomes a serious problem, and Node.js should encourage users to use `fs.opendir()`, which is slightly more complex but far better.
1 parent e6e6070 commit 35d1c50

File tree

7 files changed

+30
-26
lines changed

7 files changed

+30
-26
lines changed

lib/fs.js

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -994,7 +994,7 @@ function mkdirSync(path, options) {
994994
}
995995
}
996996

997-
function readdir(path, options, callback) {
997+
function scandir(path, options, callback) {
998998
callback = makeCallback(typeof options === 'function' ? options : callback);
999999
options = getOptions(options, {});
10001000
path = getValidatedPath(path);
@@ -1011,15 +1011,15 @@ function readdir(path, options, callback) {
10111011
getDirents(path, result, callback);
10121012
};
10131013
}
1014-
binding.readdir(pathModule.toNamespacedPath(path), options.encoding,
1014+
binding.scandir(pathModule.toNamespacedPath(path), options.encoding,
10151015
!!options.withFileTypes, req);
10161016
}
10171017

1018-
function readdirSync(path, options) {
1018+
function scandirSync(path, options) {
10191019
options = getOptions(options, {});
10201020
path = getValidatedPath(path);
10211021
const ctx = { path };
1022-
const result = binding.readdir(pathModule.toNamespacedPath(path),
1022+
const result = binding.scandir(pathModule.toNamespacedPath(path),
10231023
options.encoding, !!options.withFileTypes,
10241024
undefined, ctx);
10251025
handleErrorFromBinding(ctx);
@@ -2061,8 +2061,8 @@ module.exports = fs = {
20612061
openSync,
20622062
opendir,
20632063
opendirSync,
2064-
readdir,
2065-
readdirSync,
2064+
readdir: scandir,
2065+
readdirSync: scandirSync,
20662066
read,
20672067
readSync,
20682068
readv,
@@ -2079,6 +2079,7 @@ module.exports = fs = {
20792079
rmSync,
20802080
rmdir,
20812081
rmdirSync,
2082+
scandirSync,
20822083
stat,
20832084
statSync,
20842085
symlink,

lib/internal/fs/promises.js

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -461,10 +461,10 @@ async function mkdir(path, options) {
461461
kUsePromises);
462462
}
463463

464-
async function readdir(path, options) {
464+
async function scandir(path, options) {
465465
options = getOptions(options, {});
466466
path = getValidatedPath(path);
467-
const result = await binding.readdir(pathModule.toNamespacedPath(path),
467+
const result = await binding.scandir(pathModule.toNamespacedPath(path),
468468
options.encoding,
469469
!!options.withFileTypes,
470470
kUsePromises);
@@ -646,7 +646,7 @@ module.exports = {
646646
rm,
647647
rmdir,
648648
mkdir,
649-
readdir,
649+
readdir: scandir,
650650
readlink,
651651
symlink,
652652
lstat,

src/node_file.cc

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1613,7 +1613,7 @@ static void RealPath(const FunctionCallbackInfo<Value>& args) {
16131613
}
16141614
}
16151615

1616-
static void ReadDir(const FunctionCallbackInfo<Value>& args) {
1616+
static void ScanDir(const FunctionCallbackInfo<Value>& args) {
16171617
Environment* env = Environment::GetCurrent(args);
16181618
Isolate* isolate = env->isolate();
16191619

@@ -1628,21 +1628,21 @@ static void ReadDir(const FunctionCallbackInfo<Value>& args) {
16281628
bool with_types = args[2]->IsTrue();
16291629

16301630
FSReqBase* req_wrap_async = GetReqWrap(args, 3);
1631-
if (req_wrap_async != nullptr) { // readdir(path, encoding, withTypes, req)
1631+
if (req_wrap_async != nullptr) { // scandir(path, encoding, withTypes, req)
16321632
if (with_types) {
16331633
AsyncCall(env, req_wrap_async, args, "scandir", encoding,
16341634
AfterScanDirWithTypes, uv_fs_scandir, *path, 0 /*flags*/);
16351635
} else {
16361636
AsyncCall(env, req_wrap_async, args, "scandir", encoding,
16371637
AfterScanDir, uv_fs_scandir, *path, 0 /*flags*/);
16381638
}
1639-
} else { // readdir(path, encoding, withTypes, undefined, ctx)
1639+
} else { // scandir(path, encoding, withTypes, undefined, ctx)
16401640
CHECK_EQ(argc, 5);
16411641
FSReqWrapSync req_wrap_sync;
1642-
FS_SYNC_TRACE_BEGIN(readdir);
1642+
FS_SYNC_TRACE_BEGIN(scandir);
16431643
int err = SyncCall(env, args[4], &req_wrap_sync, "scandir",
16441644
uv_fs_scandir, *path, 0 /*flags*/);
1645-
FS_SYNC_TRACE_END(readdir);
1645+
FS_SYNC_TRACE_END(scandir);
16461646
if (err < 0) {
16471647
return; // syscall failed, no need to continue, error info is in ctx
16481648
}
@@ -1663,7 +1663,7 @@ static void ReadDir(const FunctionCallbackInfo<Value>& args) {
16631663
ctx->Set(env->context(), env->errno_string(),
16641664
Integer::New(isolate, r)).Check();
16651665
ctx->Set(env->context(), env->syscall_string(),
1666-
OneByteString(isolate, "readdir")).Check();
1666+
OneByteString(isolate, "scandir")).Check();
16671667
return;
16681668
}
16691669

@@ -2416,7 +2416,7 @@ void Initialize(Local<Object> target,
24162416
env->SetMethod(target, "ftruncate", FTruncate);
24172417
env->SetMethod(target, "rmdir", RMDir);
24182418
env->SetMethod(target, "mkdir", MKDir);
2419-
env->SetMethod(target, "readdir", ReadDir);
2419+
env->SetMethod(target, "scandir", ScanDir);
24202420
env->SetMethod(target, "internalModuleReadJSON", InternalModuleReadJSON);
24212421
env->SetMethod(target, "internalModuleStat", InternalModuleStat);
24222422
env->SetMethod(target, "stat", Stat);

test/parallel/test-fs-readdir-types.js

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -80,9 +80,9 @@ fs.readdir(readdirDir, {
8080

8181
// Check for correct types when the binding returns unknowns
8282
const UNKNOWN = constants.UV_DIRENT_UNKNOWN;
83-
const oldReaddir = binding.readdir;
84-
process.on('beforeExit', () => { binding.readdir = oldReaddir; });
85-
binding.readdir = common.mustCall((path, encoding, types, req, ctx) => {
83+
const oldScandir = binding.scandir;
84+
process.on('beforeExit', () => { binding.scandir = oldScandir; });
85+
binding.scandir = common.mustCall((path, encoding, types, req, ctx) => {
8686
if (req) {
8787
const oldCb = req.oncomplete;
8888
req.oncomplete = (err, results) => {
@@ -93,9 +93,9 @@ binding.readdir = common.mustCall((path, encoding, types, req, ctx) => {
9393
results[1] = results[1].map(() => UNKNOWN);
9494
oldCb(null, results);
9595
};
96-
oldReaddir(path, encoding, types, req);
96+
oldScandir(path, encoding, types, req);
9797
} else {
98-
const results = oldReaddir(path, encoding, types, req, ctx);
98+
const results = oldScandir(path, encoding, types, req, ctx);
9999
results[1] = results[1].map(() => UNKNOWN);
100100
return results;
101101
}

test/parallel/test-repl-underscore.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -160,7 +160,7 @@ function testError() {
160160
r.write(`_error; // initial value undefined
161161
throw new Error('foo'); // throws error
162162
_error; // shows error
163-
fs.readdirSync('/nonexistent?'); // throws error, sync
163+
fs.scandirSync('/nonexistent?'); // throws error, sync
164164
_error.code; // shows error code
165165
_error.syscall; // shows error syscall
166166
setImmediate(() => { throw new Error('baz'); }); undefined;
@@ -178,7 +178,7 @@ function testError() {
178178

179179
// The sync error, with individual property echoes
180180
/^Uncaught Error: ENOENT: no such file or directory, scandir '.*nonexistent\?'/,
181-
/Object\.readdirSync/,
181+
/Object\.scandirSync/,
182182
/^ errno: -(2|4058),$/,
183183
" syscall: 'scandir',",
184184
" code: 'ENOENT',",

test/parallel/test-trace-events-fs-sync.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,7 @@ tests['fs.sync.open'] = 'fs.writeFileSync("fs.txt", "123", "utf8");' +
7676
tests['fs.sync.read'] = 'fs.writeFileSync("fs.txt", "123", "utf8");' +
7777
'fs.readFileSync("fs.txt");' +
7878
'fs.unlinkSync("fs.txt")';
79-
tests['fs.sync.readdir'] = 'fs.readdirSync("./")';
79+
tests['fs.sync.scandir'] = 'fs.scandirSync("./")';
8080
tests['fs.sync.realpath'] = 'fs.writeFileSync("fs.txt", "123", "utf8");' +
8181
'fs.linkSync("fs.txt", "linkx");' +
8282
'fs.realpathSync.native("linkx");' +

tools/build-addons.js

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -46,8 +46,11 @@ async function runner(directoryQueue) {
4646
}
4747

4848
async function main(directory) {
49-
const directoryQueue = (await fs.readdir(directory))
50-
.map((subdir) => path.join(directory, subdir));
49+
const dir = (await fs.opendir(directory));
50+
const directoryQueue = [];
51+
for await (let subdir of dir) {
52+
directoryQueue.push(path.join(directory, subdir.name));
53+
}
5154

5255
const runners = [];
5356
for (let i = 0; i < parallelization; ++i)

0 commit comments

Comments
 (0)