fs: add recursive opendir/readdir

[Ethan-Arrowood]

Implements a recursive option for the readdir and readdirSync methods.

Closes: #34992
Ref: nodejs/tooling#130

---

Starting with tests to just get a sense for how things should behave. Will dive into the implementation next. Any feedback/concerns is welcome at any time.

[Ethan-Arrowood]

@nodejs/tooling @bnb @bcoe

[VoltrexKeyva]

The proposed behavior looks good to me, although the output design can be changed for the sake of easier control and management over the output for developers, but the current output design can also stay if desired.

Imagine a file structure as the following:

foo
|_ bar
  |_ baz
  |_ a.txt
|_ fizz
  |_ b.txt
|_ fuzz
  |_ c.txt
|_ d.txt

Design 1 (The proposed design aka the current design, only resolve paths to files):

import { readdirSync } from 'node:fs';

const output = readdirSync('./foo', { recursive: true });

console.log(output);
// ['bar/a.txt', 'fizz/b.txt', 'fuzz/c.txt', 'd.txt']

Design 2 (Resolve paths to both directories and files):

import { readdirSync } from 'node:fs';

const output = readdirSync('./foo', { recursive: true });

console.log(output);
// ['bar', 'bar/baz', 'bar/a.txt', 'fizz', 'fizz/b.txt', 'fuzz', 'fuzz/c.txt', 'd.txt']

Design 3 (Instead of returning an array of paths, return array of objects with information to each recursively read directories):

import { readdirSync } from 'node:fs';

const output = readdirSync('./foo', { recursive: true });

console.log(output);
/*
[
  {
    type: 'directory',
    name: 'bar',
    children: [
      {
        type: 'directory',
        name: 'baz',
        children: []
      },
      {
        type: 'file',
        name: 'a.txt'
      }
    ]
  },
  {
    type: 'directory',
    name: 'fizz',
    children: [
      {
        type: 'file',
        name: 'b.txt'
      }
    ]
  },
  {
    type: 'directory',
    name: 'fuzz',
    children: [
      {
        type: 'file',
        name: 'c.txt'
      }
    ]
  },
  {
    type: 'file',
    name: 'd.txt'
  }
]
*/

Also remember to fix the formatting issues reported by our linter(s) :).

[tniessen]

Since the result is potentially huge and the process itself might take virtually forever, especially on network drives etc, we should probably encourage streaming APIs instead of returning arrays. If we just return arrays, the process might crash after hours or so when the array allocations run into an out-of-memory situation, never having produced any results.

[fhinkel]

Can you fix linting please? It's hard to review with all the lint comments. Thanks!

[martinheidegger]

I am wondering if it wouldn't be nice for users to change the recursive option to be either depth-first or breadth-first. This would make the output and algorithm very clear:

await fs.readdir('.', { recursive: 'breadth-first' }) // ['a', 'b', 'a/x']
await fs.readdir('.', { recursive: 'depth-first' }) // ['a', 'a/x', b']
await fs.readdir('.', { recursive: true }) // throw new Error('Specify recursive as either "breadth-first" or "depth-first")

If we just return arrays, the process might crash after hours or so when the array allocations run into an out-of-memory situation, never having produced any results.

A readdir operation in itself doesn't expose how many entries may be returned and a huge folder takes a lot of time too. Having the API would make sense for a project that kinda knows its content and can wait for the time.

Considering readdirp as prior art, would fs.readdirp('.') be a good name for a streaming API variant?

[Ethan-Arrowood]

Thanks for all the great feedback folks.

I'm going to update the tests to exemplify an API design for this feature that includes things like:

• stream result
• list of Dirent result
• option for DFS vs BFS

[Ethan-Arrowood]

We also discussed on the call to keep this initial implementation simple. Omit things like a filter option (as shown in readdirp) since it does not already exist in the standard readdir.

I personally would like there to be some sort of default behavior when using just { recursive: true }, but then maybe the option can also support more configuration like { recurisve: { algorithm: 'depth-first', returnType: 'stream' } }.

[bnb]

I personally would like there to be some sort of default behavior when using just { recursive: true }, but then maybe the option can also support more configuration like { recurisve: { algorithm: 'depth-first', returnType: 'stream' } }

I would prefer to also see { recursive: true} Just Work™. Perhaps additional options could be allowed, if people really really want to have that more granular control?

[Ethan-Arrowood]

Summary of recent updates:

Default case uses just { recursive: true }. This will return an array of strings in a random order.

I included examples with the withFileTypes option enabled i.e. { recursive: true, withFileTypes: true } which will output an array of fs.Dirent objects in a random order.

I included another way to configure the recursive property using an object with two optional properties:

{
  result?: 'stream',
  algorithm?: 'depth-first' | 'breadth-first'
}

Unless folks can come up with another result option, I'm going to change that property to just stream?: boolean.

I did not update the promise tests - want to focus on the API shape for now.

I did not include async tests for all of the sync operations. After API agreement I'll add those in.

I did not include error case tests yet. Will think about that more later

[martinheidegger]

When implementing a readdir stream a while back I had to do .stat calls to figure out if an entry is a directory or not. The other data from the stat operation was very useful in the display of the content, and was returned by the API as well. Same is possible in readdirp by supplying the alwaysStat option. Maybe it would be worth considering it here as well?

Different thing: Would it be a good idea to support abort signals?

[bcoe]

Tests are looking quite clean, good start. Added a recommendation that we add a test for symlink loops.

It might be worth looking at the test suite for fs-extra copy/recursive, they have a bunch of test cases around symlink behavior that we might be able to use for inspiration.

[Ethan-Arrowood]

Same is possible in readdirp by supplying the alwaysStat option. Maybe it would be worth considering it here as well?

If i'm not mistaken the withFileTypes option semi-covers this use case. I don't want to deviate too much from the rest of the fs api at least for the first implementation.

Would it be a good idea to support abort signals?

Yes, for the async operations we could do this

[Ethan-Arrowood]

After some consideration, I think supporting alternative algorithms is going to make this PR more complicated than I can handle at this time.

What would be the best way to defer the algorithm feature of this PR? One option: I can put the test files I've written for them into a new branch and open an issue to track. @bcoe @bnb

[bnb]

imo getting a basic implementation in and expanding on that with additional algorithms in later PRs would be fine.

[Ethan-Arrowood]

See linked issue #41709 for algorithm feature

[Ethan-Arrowood]

Also for the sake of initial implementation, I'm dropping the promise API. I'd like to see us land the sync/async versions and then progress from there

[targos]

@Ethan-Arrowood What about the concerns raised in #41439 (review) and #41439 (review) ?

[Ethan-Arrowood]

Ahh I missed those somehow.

My initial thought on the discussion is it seems like a fine idea. Focus on a stream based recursive readdir, and then also add a recursive option to opendir that would return an array.

[Ethan-Arrowood]

Okay new API design for initial implementation:

• Add new boolean option recursive to readdir and readdirSync
• The result is a Readable stream of the paths in the directory
• The fs.promises.readdir version will return an async iterator

Future work:

• Add recursive option to opendir
• Add additional options to recursive option such as:
  • algorithm which specifies how to traverse the directory graph
  • return which specifies what to return (array, stream)
    • this can be used in conjunction with withFileTypes in order to return an array of Dirents

What do we think?

[targos]

The result is a Readable stream of the paths in the directory

Does it mean that readdir and readdirSync would return exactly the same thing?

[Ethan-Arrowood]

Good catch. I meant that the callback will be passed the same readable that is returned by the sync version

[targos]

Good catch. I meant that the callback will be passed the same readable that is returned by the sync version

Would anything really async happen before the stream is constructed and returned in the callback version?

[Ethan-Arrowood]

Not sure. I guess it'd be the difference between opendir and opendirSync under the hood.

The promise version of this is simple, I plan on using an async iterable.

The sync and callback versions I haven't given tooo much thought beyond implementing a Readable and using _read to exhaust recursive calls to something.

[targos]

Then why don't we make opendir optionally recursive? It seems to me that it would make the API more consistent

[Ethan-Arrowood]

Ah i may be lost now. opendir just returns a Dir (via callback, promise, or sync) that's an async iterable of Dirents. Are you suggesting that when recursive is enabled, the async iterable will return everything in it as Dirents?

[karlhorky]

Wow, this looks like a big feature 👀 😲

Does this represent a built-in replacement for some use cases of the following libraries? (npm trends)

• recursive-readdir (5M weekly downloads)
• fs-readdir-recursive (3M weekly downloads)
• readdirp (39M weekly downloads)
• klaw (5M weekly downloads)

[Ethan-Arrowood]

Yes it does! It may not be 1-to-1 replacements but it achieves similar functionality with all of those