Skip to content

fs.readdir's new recursive option is not fully documented #48640

Closed
@That-Guy977

Description

@That-Guy977

Affected URL(s)

https://nodejs.org/dist/latest-v20.x/docs/api/fs.html#fspromisesreaddirpath-options and callback/synchronous versions

Description of the problem

From testing, the recursive option does not work with withFileTypes: true.
The behavior of the recursive option is not documented, and only recursive's type and default value are present in the documentation. The callback-async fs.readdir and the synchronous fs.readdirSync yield erraneous results (entries missing or appear with missing fields) when both recursive and withFileTypes are supplied.

This issue tracks missing documentation. Incorrect behavior is tracked at #48858.

Activity

added
docIssues and PRs related to the documentations.
on Jul 3, 2023
anonrig

anonrig commented on Jul 4, 2023

@anonrig
Member
added
good first issueIssues that are suitable for first-time contributors.
on Jul 4, 2023
pernelkanic

pernelkanic commented on Jul 4, 2023

@pernelkanic

can i work on this?

Ethan-Arrowood

Ethan-Arrowood commented on Jul 4, 2023

@Ethan-Arrowood
Contributor

Can you share the case where it doesn't work with withFileTypes? I added a multitude of tests when I landed the option so I wanna know what I missed

targos

targos commented on Jul 4, 2023

@targos
Member

Btw it looks more like a bug than a documentation issue.

added
fsIssues and PRs related to the fs subsystem / file system.
on Jul 4, 2023
removed
good first issueIssues that are suitable for first-time contributors.
on Jul 4, 2023
That-Guy977

That-Guy977 commented on Jul 5, 2023

@That-Guy977
Author

Btw it looks more like a bug than a documentation issue.

No documentation is provided for recursive regardless, so I think it's still an issue. Maybe I should have filed the lack of documentation and the bug separately.

ghost

ghost commented on Jul 5, 2023

@ghost

Can I work on this, it will be my first issue.

That-Guy977

That-Guy977 commented on Jul 5, 2023

@That-Guy977
Author

@Ethan-Arrowood From further testing, I've found some behavior that varies between the promise, callback, and synchronous versions.

  • The promise version appears to not have any issues.

  • The callback and synchronous versions can generate a entry Dirent { name: undefined, path: '...', [Symbol(type)]: undefined }, when withFileTypes is supplied, replacing a valid entry.

  • The callback and synchronous versions fail to list certain files and directories when both withFileTypes and recursive are provided. I noticed this as several entries under .git missing, I'm unsure what the cause is.

    Note: Since nested directories themselves are not listed, these issues are likely not related to the recursion step as I had previously assumed, however this behavior is only present when recursive is also supplied and is not present in previous versions (Tested on Node v18.16.1.). I will update the original message to reflect this.

A minor inconsistency I found is that when providing the results the promise version appears to act as depth-first, while the callback and synchronous versions act as breath-first in their resulting arrays.

These tests have been on a limited filetree which I've uploaded to fs-readdir-recursive.

Ethan-Arrowood

Ethan-Arrowood commented on Jul 7, 2023

@Ethan-Arrowood
Contributor

Okay I can confirm there are two issues here.

First, missing documentation for the recursive option for the callback and sync versions.

Second, I am seeing the undefined behavior in the callback and sync versions; I think your latest message @That-Guy977 explains it well. What I'm most intrigued by is how the tests aren't catching this. We definitely need to fix this. I'm going to start investigating this problem now.

Finally, I do not believe the difference in search behavior matters. We make no guarantees in how Node recursively searches a directory. In a long historical thread I commented the possibility of specifying which algorithm could be used, but that is a feature not a bug in this case.

Some users here expressed interest in contributing, @pernelkanic you commented first. Would you like to work on the documentation fix?

That-Guy977

That-Guy977 commented on Jul 7, 2023

@That-Guy977
Author

First, missing documentation for the recursive option for the callback and sync versions.

I believe the promise version has the same lack of documentation, as it and the other versions are lacking a similar description as opendir or rm, or even an explanation in the method description as is present in mkdir.

Ethan-Arrowood

Ethan-Arrowood commented on Jul 7, 2023

@Ethan-Arrowood
Contributor

Oh good point 👍 that should be fixed too then

added a commit that references this issue on Jul 7, 2023

29 remaining items

Loading
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    confirmed-bugIssues with confirmed bugs.docIssues and PRs related to the documentations.fsIssues and PRs related to the fs subsystem / file system.

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Participants

    @anonrig@targos@MoLow@Ethan-Arrowood@That-Guy977

    Issue actions

      `fs.readdir`'s new `recursive` option is not fully documented · Issue #48640 · nodejs/node