Node: Change Dirent with stat in fs.readdir with withFileTypes

Created on 24 Sep 2018  路  6Comments  路  Source: nodejs/node

In node v10.10.0 with merge request #22020 was added a new flag withFileTypes to fs.readdir:

If options.withFileTypes is set to true, the files array will contain fs.Dirent objects.

It is really great addition and I have a couple ideas to share.

Would be great if fs.Dirent contain all information fs.Stat has. I can see no reason to create new entity for such simple data structure which is half duplicated.

Lets compare them:

fs.Dirent

  • dirent.isBlockDevice()
  • dirent.isCharacterDevice()
  • dirent.isDirectory()
  • dirent.isFIFO()
  • dirent.isFile()
  • dirent.isSocket()
  • dirent.isSymbolicLink()
  • dirent.name

fs.Stats

  • stats.isBlockDevice()
  • stats.isCharacterDevice()
  • stats.isDirectory()
  • stats.isFIFO()
  • stats.isFile()
  • stats.isSocket()
  • stats.isSymbolicLink()
  • stats.dev
  • stats.ino
  • stats.mode
  • stats.nlink
  • stats.uid
  • stats.gid
  • stats.rdev
  • stats.size
  • stats.blksize
  • stats.blocks
  • stats.atimeMs
  • stats.mtimeMs
  • stats.ctimeMs
  • stats.birthtimeMs
  • stats.atime
  • stats.mtime
  • stats.ctime
  • stats.birthtime

The only thing that missing in fs.Stats it's name.

Is your feature request related to a problem? Please describe.
I'm working on file manager for the web Cloud Commander, so I do such things all the time: read directory content and then get stat of every file. Now this all done in readify. And I would like to use the new approach of reading files with stats, it can simplify my code a lot, and make node API cleaner.

Right now if I use a flag withFileTypes I will need to call fs.stat anyways because I need not only divide files and directories but also get their size, mode, uid and mtime.

Describe the solution you'd like
I suggest to change withFileTypes behavior to get real stat information: like size, mtime, uid in method call:

fs.readdirSync('/', {withFileTypes: true});

And remove this fs.Dirent.

Describe alternatives you've considered
Also we can use something like withFileStats to get full stats (maybe with names).

fs.readdirSync('/', {withFileStats: true})

Most helpful comment

Just my 2c, but it doesn't seem that this was indeed answered (resolved) as indicated by @bengl. I personally would be in favor of an option to add additional stat properties, as the OP requested. It would be great to see this reopened (forgive me if this was tabled or discussed elsewhere).

All 6 comments

There are cases where stat data is not required and withFileTypes allows for a significant performance increase for that case (I observe up to 300% performance gain using async withFileTypes over calling readdir and stat separately), so if stat data is added to dirent, it must be optional. I'm actually slightly in favor of having such an option because it's pretty common that one needs timestamps and the like.

withFileTypes allows for a significant performance increase for that case (I observe up to 300% performance gain using async withFileTypes over calling readdir and stat separately

That's it, I'm also would like to have benefit of 300% performance gain with no need to call stat separately :).

The thing is what the option withFileTypes is , it's just readdir + stat, so I can't understand why to make simple things so hard :).

@coderaiser

The thing is what the option withFileTypes is , it's just readdir + stat, so I can't understand why to make simple things so hard :).

It isn't just readdir + stat. It's actually quite a bit less than that in most cases, on most platforms.

The libuv call that powers readdir only returns the types of files and their names, not any other metadata. (See the libuv documentation for that returned type.) There are usually no additional stat calls happening. You may see code doing this in #22020, but it's only there to handle the cases where the file type information could not be retrieved in the same libuv call as readdir. This only seems to happen on some specific systems/platforms (in testing, it seemed to happen on AIX, if I remember correctly).

As @silverwind has mentioned, there are cases where most stat data is not required, and file types are enough information to proceed. Indeed, the feature was added to support these use cases. In these cases, on systems where stat calls are unnecessary, an extra stat call per file would negatively impact performance, especially when that data is already available from the original readdir/scandir call.

That being said, as @silverwind also mentioned, there's certainly some utility in having full Stat objects returned via readdir, so perhaps it might seem that this warrants an additional option, withFileStats. A problem I see with that is that there's no name property on Stats objects, and it's rather necessary when listing directory entries. This could be added, though. On the other hand, this can easily be implemented as a library on npm, so it might not be worth doing in Node.js core. I don't really have a strong opinion here yet.

Just published fs-readdir-sync-with-file-types and fs-readdir-with-file-types ponyfill to have ability to benefit of file types on node < v10.10 :).

It seems like everything has been answered here, so I'm going to go ahead and close this. Please feel free to reopen/comment if you have additional related questions or if you think something has been left unanswered.

Just my 2c, but it doesn't seem that this was indeed answered (resolved) as indicated by @bengl. I personally would be in favor of an option to add additional stat properties, as the OP requested. It would be great to see this reopened (forgive me if this was tabled or discussed elsewhere).

Was this page helpful?
0 / 5 - 0 ratings

Related issues

stevenvachon picture stevenvachon  路  3Comments

danielstaleiny picture danielstaleiny  路  3Comments

ksushilmaurya picture ksushilmaurya  路  3Comments

danialkhansari picture danialkhansari  路  3Comments

willnwhite picture willnwhite  路  3Comments