-
Notifications
You must be signed in to change notification settings - Fork 30.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fs.readdir(path, { recursive: true }) is sync #56006
Comments
We could mark it as legacy and decide to not fix the issue, as long as the promise version is correctly async we can probably get away with the callback version being wrongly sync. |
If a sync version has a feature and its callback equivalent call doesn't... looks like a bad UX to me. Have we considered a new function instead? It seems more appropriate: |
Nevermind, I have a PR almost ready to fix this. |
Fixes: #56006 PR-URL: #56041 Reviewed-By: Ethan Arrowood <[email protected]> Reviewed-By: Juan José Arboleda <[email protected]>
Fixes: #56006 PR-URL: #56041 Reviewed-By: Ethan Arrowood <[email protected]> Reviewed-By: Juan José Arboleda <[email protected]>
Fixes: #56006 PR-URL: #56041 Reviewed-By: Ethan Arrowood <[email protected]> Reviewed-By: Juan José Arboleda <[email protected]>
Fixes: #56006 PR-URL: #56041 Reviewed-By: Ethan Arrowood <[email protected]> Reviewed-By: Juan José Arboleda <[email protected]>
Fixes: #56006 PR-URL: #56041 Reviewed-By: Ethan Arrowood <[email protected]> Reviewed-By: Juan José Arboleda <[email protected]>
Fixes: #56006 PR-URL: #56041 Reviewed-By: Ethan Arrowood <[email protected]> Reviewed-By: Juan José Arboleda <[email protected]>
Reviving #51749.
The implementation in #41439 is calling a synchronous approach even for an asynchronous call. See https://github.com/nodejs/node/blob/main/lib/fs.js#L1458.
I will take a quick look at that and see if I can fix it; otherwise, we might want to consider documenting it explicitly until we have a real solution for it.
cc: @Ethan-Arrowood
The text was updated successfully, but these errors were encountered: