nodejs / node

Node.js JavaScript runtime ✨🐢🚀✨
https://nodejs.org
Other
104.27k stars 28.06k forks source link

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

Closed theoludwig closed 1 week ago

theoludwig commented 2 weeks ago

What is the problem this feature will solve?

Since Node.js v22, with this PR: https://github.com/nodejs/node/pull/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
}
benjamingr commented 2 weeks ago

@MoLow

benjamingr commented 2 weeks ago

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

fs.promises.glob("foo/**", { exclude(entry) { return entry.isFile(); })
theoludwig commented 2 weeks ago

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 commented 2 weeks ago

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