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

fs.glob add option to only include files (not directories) in the results entries #52752

Closed
theoludwig opened this issue Apr 29, 2024 · 4 comments · Fixed by #52837
Closed

fs.glob add option to only include files (not directories) in the results entries #52752

theoludwig opened this issue Apr 29, 2024 · 4 comments · Fixed by #52837
Labels
feature request Issues that request new features to be added to Node.js. fs Issues and PRs related to the fs subsystem / file system.

Comments

@theoludwig
Copy link
Contributor

What is the problem this feature will solve?

Since Node.js v22, with this PR: #51912, it is possible to use fs.promises.glob for matching file paths based on specified patterns.

However, the results of entries also includes directories, but other famous userland library (e.g: globby) only returns files (not directories).

Example

With a file structure like the following:

$ mkdir -p foo/bar && touch foo/bar.md
$ tree foo
foo
├── bar
└── bar.md

2 directories, 1 file

And the following code:

import fs from "node:fs"
import { globby } from "globby"

console.log("fs.glob", await Array.fromAsync(fs.promises.glob("foo/**")))
console.log("globby", await globby("foo/**"))

It prints:

fs.glob [ 'foo', 'foo/bar.md', 'foo/bar' ]
globby [ 'foo/bar.md' ]

What is the feature you are proposing to solve the problem?

Add 2 options to fs.glob:

Both default to false, to keep same behavior, so no breaking changes is introduced.

Options based on the fast-glob library which globby uses under the hood.

What alternatives have you considered?

export const globby = async (patterns) => {
  const files = []
  for await (const entry of fs.promises.glob(patterns)) {
    const stats = await fs.promises.stat(entry)
    if (stats.isFile()) {
      files.push(entry)
    }
  }
  return files
}
@theoludwig theoludwig added the feature request Issues that request new features to be added to Node.js. label Apr 29, 2024
@benjamingr
Copy link
Member

@MoLow

@benjamingr
Copy link
Member

Maybe just expose the dirent insstead of its name to exclude which would enable:

fs.promises.glob("foo/**", { exclude(entry) { return entry.isFile(); })

@theoludwig
Copy link
Contributor Author

Maybe just expose the dirent insstead of its name to exclude which would enable:

fs.promises.glob("foo/**", { exclude(entry) { return entry.isFile(); })

You mean exclude take as argument (entry) the await fs.promises.stat(entry)?
That would work, fs.promises.glob("foo/**", { exclude(entry) { return entry.isDirectory(); }) to only get files.
However, that would be a BREAKING CHANGE, probably fine, as it's still experimental, but still worth to point out.

But then we don't have information about filename/path.
Maybe the argument in exclude should be an object with multiple information about the file/directory (name, relativePath, absolutePath, stats, etc.).

@MoLow
Copy link
Member

MoLow commented Apr 30, 2024

we should probably just add a withFileTypes option like fs.readDir has

@MoLow MoLow added the fs Issues and PRs related to the fs subsystem / file system. label Apr 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request Issues that request new features to be added to Node.js. fs Issues and PRs related to the fs subsystem / file system.
Projects
Status: Pending Triage
Development

Successfully merging a pull request may close this issue.

3 participants