Skip to content
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

Documentation fsPromises.readdir: possibly superfluous await #3317

Closed
michaelrommel opened this issue Apr 13, 2021 · 5 comments
Closed

Documentation fsPromises.readdir: possibly superfluous await #3317

michaelrommel opened this issue Apr 13, 2021 · 5 comments
Labels

Comments

@michaelrommel
Copy link

The documentation at: https://nodejs.org/api/fs.html#fs_fspromises_readdir_path_options provides an example to iterate over the returned file array. It uses the construct:

for await (const file of files)
    console.log(file);

I thought, that this await would only be necessary, if the returned array would contain objects of a class, which provides an asyncIterator, like fs.Dir (https://nodejs.org/api/fs.html#fs_dir_symbol_asynciterator).

Why is this await statement necessary? I would understand, if the example had the options object set to withFileTypes set to true, because in this case the returned array is of type fs.Dirent and not an array of Strings or Buffers. (But even then it is questionable, since also fs.Dirent does not document an asyncIterator.

If this is an error in the documentation, I am happy to open an issue on the node repository.

@addaleax
Copy link
Member

If this is an error in the documentation, I am happy to open an issue on the node repository.

It is. :) You can also submit a PR for this directly if you like, the documentation files are in doc/api/ in the node repo.

@marsonya marsonya added the bug label Apr 13, 2021
@marsonya
Copy link
Member

Hi @michaelrommel - Is it okay if I go ahead and create a PR for fixing this issue?
It's okay if you wanna create a PR instead.

@michaelrommel
Copy link
Author

I'll create the PR tomorrow morning. I was snowed under last week...

@michaelrommel
Copy link
Author

So, it took me some learning and reformatting the commit message to conform to the guidelines. I should have done that right away using the commandline, instead of trying that in the github UI... It's my first PR to node...

Thank you all for your help and patience. Node is awesome. I would now close this issue, as it is now in the hands of the node project repo itself or will that close by itself, once the referenced PR lands or is closed?

@Trott
Copy link
Member

Trott commented Apr 19, 2021

Thanks for the contribution! 🎉

@Trott Trott closed this as completed Apr 19, 2021
targos pushed a commit to nodejs/node that referenced this issue Sep 4, 2021
The `await` operator in the example, iterating over the returned array
of filenames is not necessary, since the returned array is either
consisting of `string`s or of `fs.Dirent` objects, neither providing
an asyncIterator.

Refs: nodejs/help#3317

PR-URL: #38293
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants