fs: introduce `opendir()` and `fs.Dir` #29349

Fishrock123 · 2019-08-28T00:41:22Z

This adds long-requested methods for asynchronously interacting and iterating through directory entries by using uv_fs_opendir, uv_fs_readdir, and uv_fs_closedir.

const fs = require('fs');

async function print(path) {
  const dir = await fs.promises.opendir(path);
  for await (const dirent of dir) {
    console.log(dirent.name);
  }
}
print('./').catch(console.error);

fs.opendir() and friends return an fs.Dir, which contains methods for doing reads and cleanup. fs.Dir also has the async iterator symbol exposed.

The read() method and friends only return fs.Dirents for this API.
Having a entry type or doing a stat call is deemed to be necessary in the majority of cases, so just returning dirents seems like the logical choice for a new api.

Reading when there are no more entries returns null instead of a dirent. However the async iterator hides that (and does automatic cleanup).

The code lives in separate files from the rest of fs, this is done partially to prevent over-pollution of those (already very large) files, but also in the case of js allows loading into fsPromises.

Due to async_hooks, this introduces a new handle type of DIRHANDLE.

This PR does not attempt to make complete optimization of this feature. Notable future improvements include:

Moving promise work into C++ land like FileHandle.
~~Making uv_dir_t keep a record of its path (or a way to retrieve it).~~
Possibly adding readv() to do multi-entry directory reads.
Aliasing fs.readdir to fs.scandir and doing a deprecation.

Refs: nodejs/node-v0.x-archive#388
Refs: #583
Refs: libuv/libuv#2057

Checklist

make -j4 test (UNIX), or vcbuild test (Windows) passes
tests and/or benchmarks are included
documentation is changed or added
commit message follows commit guidelines

I am open to discussing the api shapes, I found that this fs.Dir api made it easiest to reconcile the callback and promise apis into "one" reasonably nice (hopefully future proof) api, but I feel folks might have strong feelings about that. It does make a bit of mess in the docs though.

Also please tell me any C++ nonsense I got terribly wrong, haha.

lib/internal/fs/dir.js

doc/api/fs.md

lib/internal/fs/dir.js

src/node_dir.h

src/node_file.h

test/parallel/test-fs-opendir.js

lib/internal/fs/dir.js

src/node_dir.h

addaleax · 2019-08-28T01:20:52Z

Also: Very nice work! I’m excited to see this happen :)

src/node_dir.cc

doc/api/fs.md

src/node_file.h

src/env.h

src/node_dir.cc

requested changes handled

doc/api/fs.md

lib/internal/fs/dir.js

devnexen · 2019-08-30T09:50:32Z

Few missing commas in documentation spottable with make lint

addaleax

LGTM with a few comments

src/node_dir.cc

lib/internal/fs/dir.js


        (fixup) address more C++ review

Addresses most of addaleax's 3rd(?) review: nodejs#29349 (review)

addaleax

Looks good!

nodejs-github-bot · 2019-09-05T17:34:05Z

CI: https://ci.nodejs.org/job/node-test-pull-request/25366/

lib/internal/fs/dir.js

nodejs-github-bot · 2019-10-07T18:22:27Z

CI: https://ci.nodejs.org/job/node-test-pull-request/25894/

nodejs-github-bot · 2019-10-07T18:48:10Z

CI: https://ci.nodejs.org/job/node-test-pull-request/25896/


        fs: introduce `opendir()` and `fs.Dir`

This adds long-requested methods for asynchronously interacting and iterating through directory entries by using `uv_fs_opendir`, `uv_fs_readdir`, and `uv_fs_closedir`. `fs.opendir()` and friends return an `fs.Dir`, which contains methods for doing reads and cleanup. `fs.Dir` also has the async iterator symbol exposed. The `read()` method and friends only return `fs.Dirent`s for this API. Having a entry type or doing a `stat` call is deemed to be necessary in the majority of cases, so just returning dirents seems like the logical choice for a new api. Reading when there are no more entries returns `null` instead of a dirent. However the async iterator hides that (and does automatic cleanup). The code lives in separate files from the rest of fs, this is done partially to prevent over-pollution of those (already very large) files, but also in the case of js allows loading into `fsPromises`. Due to async_hooks, this introduces a new handle type of `DIRHANDLE`. This PR does not attempt to make complete optimization of this feature. Notable future improvements include: - Moving promise work into C++ land like FileHandle. - Possibly adding `readv()` to do multi-entry directory reads. - Aliasing `fs.readdir` to `fs.scandir` and doing a deprecation. Refs: nodejs/node-v0.x-archive#388 Refs: #583 Refs: libuv/libuv#2057

Fishrock123 · 2019-10-07T23:34:40Z

Anyone else want to ✅ before I land this tomorrow? Last CI was all green.


        fs: introduce `opendir()` and `fs.Dir`

This adds long-requested methods for asynchronously interacting and iterating through directory entries by using `uv_fs_opendir`, `uv_fs_readdir`, and `uv_fs_closedir`. `fs.opendir()` and friends return an `fs.Dir`, which contains methods for doing reads and cleanup. `fs.Dir` also has the async iterator symbol exposed. The `read()` method and friends only return `fs.Dirent`s for this API. Having a entry type or doing a `stat` call is deemed to be necessary in the majority of cases, so just returning dirents seems like the logical choice for a new api. Reading when there are no more entries returns `null` instead of a dirent. However the async iterator hides that (and does automatic cleanup). The code lives in separate files from the rest of fs, this is done partially to prevent over-pollution of those (already very large) files, but also in the case of js allows loading into `fsPromises`. Due to async_hooks, this introduces a new handle type of `DIRHANDLE`. This PR does not attempt to make complete optimization of this feature. Notable future improvements include: - Moving promise work into C++ land like FileHandle. - Possibly adding `readv()` to do multi-entry directory reads. - Aliasing `fs.readdir` to `fs.scandir` and doing a deprecation. Refs: nodejs/node-v0.x-archive#388 Refs: #583 Refs: libuv/libuv#2057 PR-URL: #29349 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: David Carlier <devnexen@gmail.com>

Fishrock123 · 2019-10-08T17:36:27Z

Landed in cbd8d71! 😄

saghul · 2019-10-09T08:00:38Z

Outstanding work @Fishrock123 👏


        fs: introduce `opendir()` and `fs.Dir`

This adds long-requested methods for asynchronously interacting and iterating through directory entries by using `uv_fs_opendir`, `uv_fs_readdir`, and `uv_fs_closedir`. `fs.opendir()` and friends return an `fs.Dir`, which contains methods for doing reads and cleanup. `fs.Dir` also has the async iterator symbol exposed. The `read()` method and friends only return `fs.Dirent`s for this API. Having a entry type or doing a `stat` call is deemed to be necessary in the majority of cases, so just returning dirents seems like the logical choice for a new api. Reading when there are no more entries returns `null` instead of a dirent. However the async iterator hides that (and does automatic cleanup). The code lives in separate files from the rest of fs, this is done partially to prevent over-pollution of those (already very large) files, but also in the case of js allows loading into `fsPromises`. Due to async_hooks, this introduces a new handle type of `DIRHANDLE`. This PR does not attempt to make complete optimization of this feature. Notable future improvements include: - Moving promise work into C++ land like FileHandle. - Possibly adding `readv()` to do multi-entry directory reads. - Aliasing `fs.readdir` to `fs.scandir` and doing a deprecation. Refs: nodejs/node-v0.x-archive#388 Refs: #583 Refs: libuv/libuv#2057 PR-URL: #29349 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: David Carlier <devnexen@gmail.com>


        fs: introduce `opendir()` and `fs.Dir`

This adds long-requested methods for asynchronously interacting and iterating through directory entries by using `uv_fs_opendir`, `uv_fs_readdir`, and `uv_fs_closedir`. `fs.opendir()` and friends return an `fs.Dir`, which contains methods for doing reads and cleanup. `fs.Dir` also has the async iterator symbol exposed. The `read()` method and friends only return `fs.Dirent`s for this API. Having a entry type or doing a `stat` call is deemed to be necessary in the majority of cases, so just returning dirents seems like the logical choice for a new api. Reading when there are no more entries returns `null` instead of a dirent. However the async iterator hides that (and does automatic cleanup). The code lives in separate files from the rest of fs, this is done partially to prevent over-pollution of those (already very large) files, but also in the case of js allows loading into `fsPromises`. Due to async_hooks, this introduces a new handle type of `DIRHANDLE`. This PR does not attempt to make complete optimization of this feature. Notable future improvements include: - Moving promise work into C++ land like FileHandle. - Possibly adding `readv()` to do multi-entry directory reads. - Aliasing `fs.readdir` to `fs.scandir` and doing a deprecation. Refs: nodejs/node-v0.x-archive#388 Refs: nodejs/node#583 Refs: libuv/libuv#2057 PR-URL: nodejs/node#29349 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: David Carlier <devnexen@gmail.com>


        fs: introduce `opendir()` and `fs.Dir`

This adds long-requested methods for asynchronously interacting and iterating through directory entries by using `uv_fs_opendir`, `uv_fs_readdir`, and `uv_fs_closedir`. `fs.opendir()` and friends return an `fs.Dir`, which contains methods for doing reads and cleanup. `fs.Dir` also has the async iterator symbol exposed. The `read()` method and friends only return `fs.Dirent`s for this API. Having a entry type or doing a `stat` call is deemed to be necessary in the majority of cases, so just returning dirents seems like the logical choice for a new api. Reading when there are no more entries returns `null` instead of a dirent. However the async iterator hides that (and does automatic cleanup). The code lives in separate files from the rest of fs, this is done partially to prevent over-pollution of those (already very large) files, but also in the case of js allows loading into `fsPromises`. Due to async_hooks, this introduces a new handle type of `DIRHANDLE`. This PR does not attempt to make complete optimization of this feature. Notable future improvements include: - Moving promise work into C++ land like FileHandle. - Possibly adding `readv()` to do multi-entry directory reads. - Aliasing `fs.readdir` to `fs.scandir` and doing a deprecation. Refs: nodejs/node-v0.x-archive#388 Refs: nodejs/node#583 Refs: libuv/libuv#2057 PR-URL: nodejs/node#29349 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: David Carlier <devnexen@gmail.com>


        2019-10-10, Version 12.12.0 (Current)

Notable changes: * build: * Add `--force-context-aware` flag to prevent usage of native node addons that aren't context aware #29631 * deprecations: * Add documentation-only deprecation for `process._tickCallback()` #29781 * esm: * Using JSON modules is experimental again #29754 * fs: * Introduce `opendir()` and `fs.Dir` to iterate through directories #29349 * process: * Add source-map support to stack traces by using `--source-map-support` #29564 * tls: * Honor `pauseOnConnect` option #29635 * Add option for private keys for OpenSSL engines #28973 PR-URL: TBD


        2019-10-10, Version 12.12.0 (Current)

Notable changes: * build: * Add `--force-context-aware` flag to prevent usage of native node addons that aren't context aware #29631 * deprecations: * Add documentation-only deprecation for `process._tickCallback()` #29781 * esm: * Using JSON modules is experimental again #29754 * fs: * Introduce `opendir()` and `fs.Dir` to iterate through directories #29349 * process: * Add source-map support to stack traces by using `--source-map-support` #29564 * tls: * Honor `pauseOnConnect` option #29635 * Add option for private keys for OpenSSL engines #28973 PR-URL: #29919


        2019-10-10, Version 12.12.0 (Current)

Notable changes: * build: * Add `--force-context-aware` flag to prevent usage of native node addons that aren't context aware #29631 * deprecations: * Add documentation-only deprecation for `process._tickCallback()` #29781 * esm: * Using JSON modules is experimental again #29754 * fs: * Introduce `opendir()` and `fs.Dir` to iterate through directories #29349 * process: * Add source-map support to stack traces by using `--source-map-support` #29564 * tls: * Honor `pauseOnConnect` option #29635 * Add option for private keys for OpenSSL engines #28973 PR-URL: #29919


        2019-10-11, Version 12.12.0 (Current)

Notable changes: * build: * Add `--force-context-aware` flag to prevent usage of native node addons that aren't context aware #29631 * deprecations: * Add documentation-only deprecation for `process._tickCallback()` #29781 * esm: * Using JSON modules is experimental again #29754 * fs: * Introduce `opendir()` and `fs.Dir` to iterate through directories #29349 * process: * Add source-map support to stack traces by using `--source-map-support` #29564 * tls: * Honor `pauseOnConnect` option #29635 * Add option for private keys for OpenSSL engines #28973 PR-URL: #29919


        2019-10-11, Version 12.12.0 (Current)

Notable changes: * build: * Add `--force-context-aware` flag to prevent usage of native node addons that aren't context aware #29631 * deprecations: * Add documentation-only deprecation for `process._tickCallback()` #29781 * esm: * Using JSON modules is experimental again #29754 * fs: * Introduce `opendir()` and `fs.Dir` to iterate through directories #29349 * process: * Add source-map support to stack traces by using `--source-map-support` #29564 * tls: * Honor `pauseOnConnect` option #29635 * Add option for private keys for OpenSSL engines #28973 PR-URL: #29919


        fs: introduce `opendir()` and `fs.Dir`

This adds long-requested methods for asynchronously interacting and iterating through directory entries by using `uv_fs_opendir`, `uv_fs_readdir`, and `uv_fs_closedir`. `fs.opendir()` and friends return an `fs.Dir`, which contains methods for doing reads and cleanup. `fs.Dir` also has the async iterator symbol exposed. The `read()` method and friends only return `fs.Dirent`s for this API. Having a entry type or doing a `stat` call is deemed to be necessary in the majority of cases, so just returning dirents seems like the logical choice for a new api. Reading when there are no more entries returns `null` instead of a dirent. However the async iterator hides that (and does automatic cleanup). The code lives in separate files from the rest of fs, this is done partially to prevent over-pollution of those (already very large) files, but also in the case of js allows loading into `fsPromises`. Due to async_hooks, this introduces a new handle type of `DIRHANDLE`. This PR does not attempt to make complete optimization of this feature. Notable future improvements include: - Moving promise work into C++ land like FileHandle. - Possibly adding `readv()` to do multi-entry directory reads. - Aliasing `fs.readdir` to `fs.scandir` and doing a deprecation. Refs: nodejs/node-v0.x-archive#388 Refs: nodejs/node#583 Refs: libuv/libuv#2057 PR-URL: nodejs/node#29349 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: David Carlier <devnexen@gmail.com>


        update BUILD.gn for new files

introduced in nodejs/node#29349


        update BUILD.gn for new files

introduced in nodejs/node#29349


        update BUILD.gn for new files

introduced in nodejs/node#29349


        update BUILD.gn for new files

introduced in nodejs/node#29349


        update BUILD.gn for new files

introduced in nodejs/node#29349


        update BUILD.gn for new files

introduced in nodejs/node#29349

Fishrock123 added C++ fs labels Aug 28, 2019

Fishrock123 requested a review from cjihrig Aug 28, 2019

nodejs-github-bot added the lib / src label Aug 28, 2019

Fishrock123 requested review from addaleax and jasnell Aug 28, 2019

Fishrock123 referenced this pull request Aug 28, 2019

streaming / iterative fs.readdir #583

Closed

devsnek reviewed Aug 28, 2019

View changes

lib/internal/fs/dir.js Outdated Show resolved Hide resolved

lib/internal/fs/dir.js Outdated Show resolved Hide resolved

addaleax reviewed Aug 28, 2019

View changes

addaleax added semver-minor and removed lib / src labels Aug 28, 2019

saghul reviewed Aug 28, 2019

View changes

src/node_dir.cc Outdated Show resolved Hide resolved

cjihrig reviewed Aug 28, 2019

View changes

doc/api/fs.md Outdated Show resolved Hide resolved

doc/api/fs.md Outdated Show resolved Hide resolved

doc/api/fs.md Outdated Show resolved Hide resolved

src/node_file.h Outdated Show resolved Hide resolved

addaleax reviewed Aug 28, 2019

View changes

src/env.h Outdated Show resolved Hide resolved

src/node_dir.cc Outdated Show resolved Hide resolved