Closed
Description
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
anonrig commentedon Jul 4, 2023
cc @Ethan-Arrowood
pernelkanic commentedon Jul 4, 2023
can i work on this?
Ethan-Arrowood commentedon Jul 4, 2023
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 missedtargos commentedon Jul 4, 2023
Btw it looks more like a bug than a documentation issue.
That-Guy977 commentedon Jul 5, 2023
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 commentedon Jul 5, 2023
Can I work on this, it will be my first issue.
That-Guy977 commentedon Jul 5, 2023
@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 }
, whenwithFileTypes
is supplied, replacing a valid entry.The callback and synchronous versions fail to list certain files and directories when both
withFileTypes
andrecursive
are provided. I noticed this as several entries under.git
missing, I'm unsure what the cause is.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 commentedon Jul 7, 2023
Okay I can confirm there are two issues here.
First, missing documentation for the
recursive
option for thecallback
andsync
versions.Second, I am seeing the
undefined
behavior in thecallback
andsync
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 commentedon Jul 7, 2023
I believe the promise version has the same lack of documentation, as it and the other versions are lacking a similar description as
opendir
orrm
, or even an explanation in the method description as is present inmkdir
.Ethan-Arrowood commentedon Jul 7, 2023
Oh good point 👍 that should be fixed too then
fs: fix readdir recursive sync & callback
29 remaining items