Skip to content

Change Dirent with stat in fs.readdir with withFileTypes #23055

Closed
Closed
Change Dirent with stat in fs.readdir with withFileTypes#23055

Description

@coderaiser
coderaiser
opened on Sep 24, 2018

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})

Activity

  1. silverwind commented on Sep 24, 2018

    @silverwind
    silverwind
    on Sep 24, 2018
    Contributor

    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.

  2. coderaiser commented on Sep 24, 2018

    @coderaiser
    coderaiser
    on Sep 24, 2018
    ContributorAuthor

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

  3. bengl commented on Sep 25, 2018

    @bengl
    bengl
    on Sep 25, 2018
    Member

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

  4. coderaiser commented on Oct 9, 2018

    @coderaiser
    coderaiser
    on Oct 9, 2018
    ContributorAuthor

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

  5. bengl commented on Nov 29, 2018

    @bengl
    bengl
    on Nov 29, 2018
    Member

    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.

  6. bengl
    closed this as completedon Nov 29, 2018

  7. jonschlinkert commented on Dec 29, 2018

    @jonschlinkert
    jonschlinkert
    on Dec 29, 2018

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

  8. niryeffet commented on Nov 22, 2024

    @niryeffet
    niryeffet
    on Nov 22, 2024

    Old but interesting. FWIW, withFileTypes: true, at least on modern linux, is calling statx() syscall to get the file type data. (without withFileTypes: true there are no statx() calls for each). In addition, the statx() result is returned with each directory entry, but is inaccessible -- hidden behind a Symbol.

    If one insists not to call fs.stat() and attempts to use readdir data, it is possible to extract the Symbol(stats) from the first entry and reuse it:

    let statsSym;
let dirData = require('fs').readdirSync('.', { withFileTypes: true });

if (!statsSym) Object.getOwnPropertySymbols(dirData[0]).forEach(s => {
  if (s.toString() == 'Symbol(stats)') statsSym = s;
});

dirData.forEach(dentry => console.log(dentry.name, dentry[statsSym].size));
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

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions