diff --git a/lib/internal/fs/dir.js b/lib/internal/fs/dir.js index 0293e23c44bc33..0d7f675643dfcd 100644 --- a/lib/internal/fs/dir.js +++ b/lib/internal/fs/dir.js @@ -164,12 +164,16 @@ class Dir { return; } - const result = handle.read( + // Fully read the directory and buffer the entries. + // This is a naive solution and for very large sub-directories + // it can even block the event loop. Essentially, `bufferSize` is + // not respected for recursive mode. This is a known limitation. + // Issue to fix: https://github.com/nodejs/node/issues/55764 + let result; + while ((result = handle.read( this.#options.encoding, this.#options.bufferSize, - ); - - if (result) { + ))) { this.processReadResult(path, result); } diff --git a/test/sequential/test-fs-opendir-recursive.js b/test/sequential/test-fs-opendir-recursive.js index a7e9b9a318e5fb..494e5591491b2f 100644 --- a/test/sequential/test-fs-opendir-recursive.js +++ b/test/sequential/test-fs-opendir-recursive.js @@ -132,6 +132,7 @@ function getDirentPath(dirent) { } function assertDirents(dirents) { + assert.strictEqual(dirents.length, expected.length); dirents.sort((a, b) => (getDirentPath(a) < getDirentPath(b) ? -1 : 1)); assert.deepStrictEqual( dirents.map((dirent) => { @@ -221,3 +222,21 @@ function processDirCb(dir, cb) { test().then(common.mustCall()); } + +// Issue https://github.com/nodejs/node/issues/48820 highlights that +// opendir recursive does not properly handle the buffer size option. +// This test asserts that the buffer size option is respected. +{ + const dir = fs.opendirSync(testDir, { bufferSize: 1, recursive: true }); + processDirSync(dir); + dir.closeSync(); +} + +{ + fs.opendir(testDir, { recursive: true, bufferSize: 1 }, common.mustSucceed((dir) => { + processDirCb(dir, common.mustSucceed((dirents) => { + assertDirents(dirents); + dir.close(common.mustSucceed()); + })); + })); +}