Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Begin docs-deprecation of fs.readdir{Sync}(), add fs.scandirSync() alias. #36008

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 36 additions & 0 deletions doc/api/deprecations.md
Original file line number Diff line number Diff line change
Expand Up @@ -2699,6 +2699,37 @@ resolutions not in `node_modules`. This means there will not be deprecation
warnings for `"exports"` in dependencies. With `--pending-deprecation`, a
runtime warning results no matter where the `"exports"` usage occurs.

### DEP0147: `fs.readdir()`
<!-- YAML
changes:
- version: REPLACEME
pr-url: REPLACEME
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
pr-url: REPLACEME
pr-url: https://github.com/nodejs/node/pull/36008

description: Documentation-only deprecation.
-->

Type: Documentation-only

[`fs.readdir()`][] is deprecated. Use [`fsPromises.opendir()`][] instead,
where possible, otherwise use [`fs.opendir()`][].
`fs.readdir()` is not fully asynchronous. It is asynchronous while gathering
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How is fs.readdir() not fully asynchronous? That seems inaccurate

the list of directory entries but then sends them to JavaScript as one large
array. In cases where this array contains hndreds of entries, particularly if
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
array. In cases where this array contains hndreds of entries, particularly if
array. In cases where this array contains hundreds of entries, particularly if

`withFileTypes` is enabled, this can be quite slow and cause performance
degredations.

### DEP0148: `fs.readdirSync()`
<!-- YAML
changes:
- version: REPLACEME
pr-url: REPLACEME
description: Documentation-only deprecation.
-->

Type: Documentation-only

[`fs.readdirSync()`][] is deprecated. Use [`fs.scandirSync()`][] instead.
`fs.readdirSync()` was always mis-named.

[Legacy URL API]: url.md#url_legacy_url_api
[NIST SP 800-38D]: https://nvlpubs.nist.gov/nistpubs/Legacy/SP/nistspecialpublication800-38d.pdf
[RFC 6066]: https://tools.ietf.org/html/rfc6066#section-3
Expand Down Expand Up @@ -2750,9 +2781,14 @@ runtime warning results no matter where the `"exports"` usage occurs.
[`fs.lchmodSync(path, mode)`]: fs.md#fs_fs_lchmodsync_path_mode
[`fs.lchown(path, uid, gid, callback)`]: fs.md#fs_fs_lchown_path_uid_gid_callback
[`fs.lchownSync(path, uid, gid)`]: fs.md#fs_fs_lchownsync_path_uid_gid
[`fs.opendir()`]: fs.html#fs_fs_opendir_path_options_callback
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
[`fs.opendir()`]: fs.html#fs_fs_opendir_path_options_callback
[`fs.opendir()`]: fs.md#fs_fs_opendir_path_options_callback

[`fs.read()`]: fs.md#fs_fs_read_fd_buffer_offset_length_position_callback
[`fs.readSync()`]: fs.md#fs_fs_readsync_fd_buffer_offset_length_position
[`fs.readdir()`]: fs.html#fs_fs_readdir_path_options_callback
[`fs.readdirSync()`]: fs.html#fs_fs_readdirsync_path_options
[`fs.scandirSync()`]: fs.html#fs_fs_scandirsync_path_options
[`fs.stat()`]: fs.md#fs_fs_stat_path_options_callback
[`fsPromises.opendir()`]: fs.html#fs_fspromises_opendir_path_options
[`http.get()`]: http.md#http_http_get_options_callback
[`http.request()`]: http.md#http_http_request_options_callback
[`https.get()`]: https.md#https_https_get_options_callback
Expand Down
32 changes: 30 additions & 2 deletions doc/api/fs.md
Original file line number Diff line number Diff line change
Expand Up @@ -2957,6 +2957,7 @@ If no `options` object is specified, it will default with the above values.
## `fs.readdir(path[, options], callback)`
<!-- YAML
added: v0.1.8
deprecated: REPLACEME
changes:
- version: v10.10.0
pr-url: https://github.com/nodejs/node/pull/22020
Expand All @@ -2978,6 +2979,9 @@ changes:
description: The `options` parameter was added.
-->

> Stability: 0 - Deprecated. Use [`fsPromises.opendir()`][] instead,
where possible, otherwise use [`fs.opendir()`][].

* `path` {string|Buffer|URL}
* `options` {string|Object}
* `encoding` {string} **Default:** `'utf8'`
Expand All @@ -2986,7 +2990,7 @@ changes:
* `err` {Error}
* `files` {string[]|Buffer[]|fs.Dirent[]}

Asynchronous readdir(3). Reads the contents of a directory.
Asynchronous scandir(3). Reads the contents of a directory.
The callback gets two arguments `(err, files)` where `files` is an array of
the names of the files in the directory excluding `'.'` and `'..'`.

Expand All @@ -3011,13 +3015,15 @@ changes:
protocol. Support is currently still *experimental*.
-->

> Stability: 0 - Deprecated. Use [`fs.scandirSync()`][] instead.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There shoud be a deprecated: REPLACEME in the YAML above.


* `path` {string|Buffer|URL}
* `options` {string|Object}
* `encoding` {string} **Default:** `'utf8'`
* `withFileTypes` {boolean} **Default:** `false`
* Returns: {string[]|Buffer[]|fs.Dirent[]}

Synchronous readdir(3).
Synchronous scandir(3).

The optional `options` argument can be a string specifying an encoding, or an
object with an `encoding` property specifying the character encoding to use for
Expand Down Expand Up @@ -3654,6 +3660,27 @@ added: v14.14.0
Synchronously removes files and directories (modeled on the standard POSIX `rm`
utility). Returns `undefined`.

## `fs.scandirSync(path[, options])`
<!-- YAML
added: REPLACEME
-->

* `path` {string|Buffer|URL}
* `options` {string|Object}
* `encoding` {string} **Default:** `'utf8'`
* `withFileTypes` {boolean} **Default:** `false`
* Returns: {string[]|Buffer[]|fs.Dirent[]}

Synchronous scandir(3).

The optional `options` argument can be a string specifying an encoding, or an
object with an `encoding` property specifying the character encoding to use for
the filenames returned. If the `encoding` is set to `'buffer'`,
the filenames returned will be passed as `Buffer` objects.

If `options.withFileTypes` is set to `true`, the result will contain
[`fs.Dirent`][] objects.

## `fs.stat(path[, options], callback)`
<!-- YAML
added: v0.0.2
Expand Down Expand Up @@ -6125,6 +6152,7 @@ the file contents.
[`fs.readv()`]: #fs_fs_readv_fd_buffers_position_callback
[`fs.realpath()`]: #fs_fs_realpath_path_options_callback
[`fs.rmdir()`]: #fs_fs_rmdir_path_options_callback
[`fs.scandirSync()`]: fs.html#fs_fs_scandirsync_path_options
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
[`fs.scandirSync()`]: fs.html#fs_fs_scandirsync_path_options
[`fs.scandirSync()`]: #fs_fs_scandirsync_path_options

[`fs.stat()`]: #fs_fs_stat_path_options_callback
[`fs.symlink()`]: #fs_fs_symlink_target_path_type_callback
[`fs.utimes()`]: #fs_fs_utimes_path_atime_mtime_callback
Expand Down
13 changes: 7 additions & 6 deletions lib/fs.js
Original file line number Diff line number Diff line change
Expand Up @@ -994,7 +994,7 @@ function mkdirSync(path, options) {
}
}

function readdir(path, options, callback) {
function scandir(path, options, callback) {
callback = makeCallback(typeof options === 'function' ? options : callback);
options = getOptions(options, {});
path = getValidatedPath(path);
Expand All @@ -1011,15 +1011,15 @@ function readdir(path, options, callback) {
getDirents(path, result, callback);
};
}
binding.readdir(pathModule.toNamespacedPath(path), options.encoding,
binding.scandir(pathModule.toNamespacedPath(path), options.encoding,
!!options.withFileTypes, req);
}

function readdirSync(path, options) {
function scandirSync(path, options) {
options = getOptions(options, {});
path = getValidatedPath(path);
const ctx = { path };
const result = binding.readdir(pathModule.toNamespacedPath(path),
const result = binding.scandir(pathModule.toNamespacedPath(path),
options.encoding, !!options.withFileTypes,
undefined, ctx);
handleErrorFromBinding(ctx);
Expand Down Expand Up @@ -2061,8 +2061,8 @@ module.exports = fs = {
openSync,
opendir,
opendirSync,
readdir,
readdirSync,
readdir: scandir,
readdirSync: scandirSync,
read,
readSync,
readv,
Expand All @@ -2079,6 +2079,7 @@ module.exports = fs = {
rmSync,
rmdir,
rmdirSync,
scandirSync,
stat,
statSync,
symlink,
Expand Down
6 changes: 3 additions & 3 deletions lib/internal/fs/promises.js
Original file line number Diff line number Diff line change
Expand Up @@ -461,10 +461,10 @@ async function mkdir(path, options) {
kUsePromises);
}

async function readdir(path, options) {
async function scandir(path, options) {
options = getOptions(options, {});
path = getValidatedPath(path);
const result = await binding.readdir(pathModule.toNamespacedPath(path),
const result = await binding.scandir(pathModule.toNamespacedPath(path),
options.encoding,
!!options.withFileTypes,
kUsePromises);
Expand Down Expand Up @@ -646,7 +646,7 @@ module.exports = {
rm,
rmdir,
mkdir,
readdir,
readdir: scandir,
readlink,
symlink,
lstat,
Expand Down
14 changes: 7 additions & 7 deletions src/node_file.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1613,7 +1613,7 @@ static void RealPath(const FunctionCallbackInfo<Value>& args) {
}
}

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

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

FSReqBase* req_wrap_async = GetReqWrap(args, 3);
if (req_wrap_async != nullptr) { // readdir(path, encoding, withTypes, req)
if (req_wrap_async != nullptr) { // scandir(path, encoding, withTypes, req)
if (with_types) {
AsyncCall(env, req_wrap_async, args, "scandir", encoding,
AfterScanDirWithTypes, uv_fs_scandir, *path, 0 /*flags*/);
} else {
AsyncCall(env, req_wrap_async, args, "scandir", encoding,
AfterScanDir, uv_fs_scandir, *path, 0 /*flags*/);
}
} else { // readdir(path, encoding, withTypes, undefined, ctx)
} else { // scandir(path, encoding, withTypes, undefined, ctx)
CHECK_EQ(argc, 5);
FSReqWrapSync req_wrap_sync;
FS_SYNC_TRACE_BEGIN(readdir);
FS_SYNC_TRACE_BEGIN(scandir);
int err = SyncCall(env, args[4], &req_wrap_sync, "scandir",
uv_fs_scandir, *path, 0 /*flags*/);
FS_SYNC_TRACE_END(readdir);
FS_SYNC_TRACE_END(scandir);
if (err < 0) {
return; // syscall failed, no need to continue, error info is in ctx
}
Expand All @@ -1663,7 +1663,7 @@ static void ReadDir(const FunctionCallbackInfo<Value>& args) {
ctx->Set(env->context(), env->errno_string(),
Integer::New(isolate, r)).Check();
ctx->Set(env->context(), env->syscall_string(),
OneByteString(isolate, "readdir")).Check();
OneByteString(isolate, "scandir")).Check();
return;
}

Expand Down Expand Up @@ -2416,7 +2416,7 @@ void Initialize(Local<Object> target,
env->SetMethod(target, "ftruncate", FTruncate);
env->SetMethod(target, "rmdir", RMDir);
env->SetMethod(target, "mkdir", MKDir);
env->SetMethod(target, "readdir", ReadDir);
env->SetMethod(target, "scandir", ScanDir);
env->SetMethod(target, "internalModuleReadJSON", InternalModuleReadJSON);
env->SetMethod(target, "internalModuleStat", InternalModuleStat);
env->SetMethod(target, "stat", Stat);
Expand Down
10 changes: 5 additions & 5 deletions test/parallel/test-fs-readdir-types.js
Original file line number Diff line number Diff line change
Expand Up @@ -80,9 +80,9 @@ fs.readdir(readdirDir, {

// Check for correct types when the binding returns unknowns
const UNKNOWN = constants.UV_DIRENT_UNKNOWN;
const oldReaddir = binding.readdir;
process.on('beforeExit', () => { binding.readdir = oldReaddir; });
binding.readdir = common.mustCall((path, encoding, types, req, ctx) => {
const oldScandir = binding.scandir;
process.on('beforeExit', () => { binding.scandir = oldScandir; });
binding.scandir = common.mustCall((path, encoding, types, req, ctx) => {
if (req) {
const oldCb = req.oncomplete;
req.oncomplete = (err, results) => {
Expand All @@ -93,9 +93,9 @@ binding.readdir = common.mustCall((path, encoding, types, req, ctx) => {
results[1] = results[1].map(() => UNKNOWN);
oldCb(null, results);
};
oldReaddir(path, encoding, types, req);
oldScandir(path, encoding, types, req);
} else {
const results = oldReaddir(path, encoding, types, req, ctx);
const results = oldScandir(path, encoding, types, req, ctx);
results[1] = results[1].map(() => UNKNOWN);
return results;
}
Expand Down
4 changes: 2 additions & 2 deletions test/parallel/test-repl-underscore.js
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ function testError() {
r.write(`_error; // initial value undefined
throw new Error('foo'); // throws error
_error; // shows error
fs.readdirSync('/nonexistent?'); // throws error, sync
fs.scandirSync('/nonexistent?'); // throws error, sync
_error.code; // shows error code
_error.syscall; // shows error syscall
setImmediate(() => { throw new Error('baz'); }); undefined;
Expand All @@ -178,7 +178,7 @@ function testError() {

// The sync error, with individual property echoes
/^Uncaught Error: ENOENT: no such file or directory, scandir '.*nonexistent\?'/,
/Object\.readdirSync/,
/Object\.scandirSync/,
/^ errno: -(2|4058),$/,
" syscall: 'scandir',",
" code: 'ENOENT',",
Expand Down
2 changes: 1 addition & 1 deletion test/parallel/test-trace-events-fs-sync.js
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ tests['fs.sync.open'] = 'fs.writeFileSync("fs.txt", "123", "utf8");' +
tests['fs.sync.read'] = 'fs.writeFileSync("fs.txt", "123", "utf8");' +
'fs.readFileSync("fs.txt");' +
'fs.unlinkSync("fs.txt")';
tests['fs.sync.readdir'] = 'fs.readdirSync("./")';
tests['fs.sync.scandir'] = 'fs.scandirSync("./")';
tests['fs.sync.realpath'] = 'fs.writeFileSync("fs.txt", "123", "utf8");' +
'fs.linkSync("fs.txt", "linkx");' +
'fs.realpathSync.native("linkx");' +
Expand Down
7 changes: 5 additions & 2 deletions tools/build-addons.js
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,11 @@ async function runner(directoryQueue) {
}

async function main(directory) {
const directoryQueue = (await fs.readdir(directory))
.map((subdir) => path.join(directory, subdir));
const dir = (await fs.opendir(directory));
const directoryQueue = [];
for await (let subdir of dir) {
directoryQueue.push(path.join(directory, subdir.name));
}

const runners = [];
for (let i = 0; i < parallelization; ++i)
Expand Down