Skip to content

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

Description

@chris-sophos

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

  1. added
    docIssues and PRs related to Node.js documentation.
    on Jul 3, 2023
  2. anonrig commented on Jul 4, 2023

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

    @pernelkanic

    can i work on this?

  5. 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

  6. targos commented on Jul 4, 2023

    @targos
    Member

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

  7. added
    fsIssues and PRs related to file-system APIs and the fs module.
    on Jul 4, 2023
  8. removed
    good first issueIssues that are suitable for first-time contributors.
    on Jul 4, 2023
  9. chris-sophos commented on Jul 5, 2023

    @chris-sophos
    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.

  10. chris-sophos commented on Jul 5, 2023

    @chris-sophos
    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.

      DetailsNote: 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.

  11. 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?

  12. chris-sophos commented on Jul 7, 2023

    @chris-sophos
    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.

  13. Ethan-Arrowood commented on Jul 7, 2023

    @Ethan-Arrowood
    Contributor

    Oh good point 👍 that should be fixed too then

  14. 9 remaining items

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 and PRs for confirmed bugs.docIssues and PRs related to Node.js documentation.fsIssues and PRs related to file-system APIs and the fs module.

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions