fs: expose glob and globSync #51912

MoLow · 2024-02-28T13:05:02Z

Fixes #40731
This PR introduces fs.globand fsPromises.glob.

MoLow · 2024-02-28T13:06:10Z

this capability was originally added to core for the use of the test runner, and we did not expose it since some dependencies don't use primordials.
there have been recent discussions regarding the primordials and whether or not they are a must for 100% of core code.
My personal opinion is primordials should be used to prevent catastrophic failures and such an addition to core has more benefits than drawbacks.
this will also not be precedential - we have some dependencies that don't use primordials, such as undici
we might want to add tsc-agenda Issues and PRs to discuss during the meetings of the TSC. to discuss this

github-actions · 2024-02-28T13:06:38Z

The notable-change PRs with changes that should be highlighted in changelogs. label has been added by @MoLow.

Please suggest a text for the release notes if you'd like to include a more detailed summary, then proceed to update the PR description with the text or a link to the notable change suggested text comment. Otherwise, the commit will be placed in the Other Notable Changes section.

MoLow · 2024-02-28T13:10:25Z

lib/internal/fs/glob.js

@@ -376,6 +412,194 @@ class Glob {
 }
 }
 }
+
+
+ async* glob() {


this is pretty much the same implementation as globSync except using yield instead if this.#results.push

The implementations can be unified probably if you yield where you await and then do sync/async I/O depending on the path not sure if it's worth it

lib/internal/fs/glob.js

doc/api/fs.md

MoLow · 2024-02-29T12:47:24Z

CC @nodejs/fs

doc/api/fs.md

lib/internal/fs/glob.js

benjamingr

lgtm for experimental

lib/internal/fs/glob.js

lib/internal/fs/promises.js

aduh95 · 2024-03-02T09:17:20Z

/cc @nodejs/fs

atlowChemi · 2024-03-03T09:54:41Z

cc @nodejs/primordials

nodejs-github-bot · 2024-03-03T11:17:19Z

CI: https://ci.nodejs.org/job/node-test-pull-request/57563/

PR-URL: nodejs#51912 Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Chemi Atlow <chemi@atlow.co.il> Reviewed-By: Nitzan Uziely <linkgoron@gmail.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>

marco-ippolito · 2024-04-15T15:33:54Z

@MoLow can you please write the description of the notable change?

MoLow · 2024-04-16T06:35:26Z

@marco-ippolito Sure:

It is now possible to expand glob expressions using glob and globSync, allowing developers to scan a directory for files matching patterns

felixfbecker · 2024-04-24T20:14:04Z

doc/api/fs.md

+const { globSync } = require('node:fs');
+
+console.log(globSync('**/*.js'));
+```


It would be great to have more details here about what glob patterns are supported, like there is in https://github.com/isaacs/node-glob?tab=readme-ov-file#glob-primer

Since glob implementations can vary and it may not be obvious if a certain pattern is supported (that is not just ** and *)

jonschlinkert · 2024-05-01T15:08:11Z

Sorry to repeat myself, but I'm interested in finding the unit tests for this, can someone point me in the right direction? If there are no unit tests, that brings another set of questions.

styfle · 2024-05-02T00:02:17Z

@jonschlinkert Looks like the tests are in test/parallel/test-fs-glob.mjs

node/test/parallel/test-fs-glob.mjs

Line 6 in b876e00

import { test, describe } from 'node:test';

jonschlinkert · 2024-05-02T00:39:45Z

thanks @styfle, don't know how I missed those.

jonschlinkert · 2024-05-02T01:01:05Z

I see that his is actually depending on minimatch, I would have preferred to see picomatch or micromatch given that minimatch is subject to catastrophic backtracking, and both picomatch and micromatch are more performant, support more patterns, and are better tested. Picomatch also has a tokenizer that can be used. Is there a reason minimatch is being used or can we do a PR to swap in micromatch?

Here are the benchmarks.

I know this isn't the issue for this, I'd be happy to open a new one.

styfle · 2024-05-04T02:25:01Z

@jonschlinkert It might be worth creating a new issue since this one is closed

Refs: nodejs#51912 (comment)

danielbayley · 2024-06-22T14:23:12Z

I see that his is actually depending on minimatch, I would have preferred to see picomatch or micromatch given that minimatch is subject to catastrophic backtracking, and both picomatch and micromatch are more performant, support more patterns, and are better tested. Picomatch also has a tokenizer that can be used. Is there a reason minimatch is being used or can we do a PR to swap in micromatch?

Here are the benchmarks.

I know this isn't the issue for this, I'd be happy to open a new one.

@jonschlinkert It might be worth creating a new issue since this one is closed

@jonschlinkert @styfle See: #53547.

Refs: nodejs#51912 (comment)

Refs: nodejs#51912

- Refs: nodejs#51912

nodejs-github-bot added fs Issues and PRs related to the fs subsystem / file system. needs-ci PRs that need a full CI run. labels Feb 28, 2024

MoLow added semver-minor PRs that contain new features and should be released in the next minor version. notable-change PRs with changes that should be highlighted in changelogs. labels Feb 28, 2024

MoLow linked an issue Feb 28, 2024 that may be closed by this pull request

[node:test] Support for file path patterns in run() #48609

Closed

MoLow commented Feb 28, 2024

View reviewed changes

lib/internal/fs/glob.js Outdated Show resolved Hide resolved

MoLow force-pushed the expose-glob branch from 559006d to 04a89f9 Compare February 28, 2024 15:58

pluris reviewed Feb 28, 2024

View reviewed changes

doc/api/fs.md Outdated Show resolved Hide resolved

pluris reviewed Feb 28, 2024

View reviewed changes

doc/api/fs.md Outdated Show resolved Hide resolved

MoLow force-pushed the expose-glob branch from 04a89f9 to 1334826 Compare February 29, 2024 10:52

benjamingr reviewed Mar 1, 2024

View reviewed changes

doc/api/fs.md Show resolved Hide resolved

benjamingr reviewed Mar 1, 2024

View reviewed changes

doc/api/fs.md Outdated Show resolved Hide resolved

benjamingr reviewed Mar 1, 2024

View reviewed changes

lib/internal/fs/glob.js Show resolved Hide resolved

benjamingr approved these changes Mar 1, 2024

View reviewed changes

aduh95 reviewed Mar 2, 2024

View reviewed changes

fs: expose glob and globSync

f11f70a

MoLow force-pushed the expose-glob branch from 1334826 to f11f70a Compare March 3, 2024 09:24

MoLow requested review from benjamingr and aduh95 March 3, 2024 09:24

atlowChemi approved these changes Mar 3, 2024

View reviewed changes

Linkgoron approved these changes Mar 3, 2024

View reviewed changes

MoLow added the request-ci Add this label to start a Jenkins CI on a PR. label Mar 3, 2024

github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Mar 3, 2024

zachleat mentioned this pull request Apr 8, 2024

Test viability of fdir replacement for fast-glob 11ty/eleventy#3167

Closed

RafaelGSS mentioned this pull request Apr 15, 2024

2024-04-24, Version 22.0.0 (Current) #52505

Merged

styfle mentioned this pull request Apr 16, 2024

Replace fast-glob with alternative in NextJS es-tooling/ecosystem-cleanup#37

Open

felixfbecker reviewed Apr 24, 2024

View reviewed changes

hail2u added a commit to hail2u/hail2u.net that referenced this pull request Apr 25, 2024

Bookmark nodejs/node#51912

a745366

theoludwig mentioned this pull request Apr 29, 2024

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

Closed

marco-ippolito added the backport-requested-v20.x PRs awaiting manual backport to the v20.x-staging branch. label May 1, 2024

marco-ippolito added baking-for-lts PRs that need to wait before landing in a LTS release. and removed backport-requested-v20.x PRs awaiting manual backport to the v20.x-staging branch. labels May 2, 2024

marco-ippolito added backport-requested-v20.x PRs awaiting manual backport to the v20.x-staging branch. and removed baking-for-lts PRs that need to wait before landing in a LTS release. labels Jun 17, 2024

danielbayley added a commit to danielbayley/node that referenced this pull request Jun 22, 2024

deps: swap minimatch with picomatch

add50c9

Refs: nodejs#51912 (comment)

danielbayley added a commit to danielbayley/node that referenced this pull request Jun 22, 2024

deps: swap minimatch with picomatch

035846c

Refs: nodejs#51912 (comment)

danielbayley mentioned this pull request Jun 22, 2024

deps: swap minimatch with micromatch #53547

Closed

danielbayley added a commit to danielbayley/node that referenced this pull request Jun 23, 2024

deps: swap minimatch with picomatch

8d31f8d

Refs: nodejs#51912 (comment)

fregante mentioned this pull request Jul 1, 2024

Enhancement: replace globby with fast-glob typescript-eslint/typescript-eslint#9453

Closed

4 tasks

bizob2828 mentioned this pull request Jul 9, 2024

[spike]: Research additional instrumentation points in Node 22 newrelic/node-newrelic#2304

Closed

voxpelli mentioned this pull request Aug 22, 2024

Future of minimatch considering path.matchesGlob() isaacs/minimatch#240

Closed

kamilmysliwiec mentioned this pull request Aug 26, 2024

Replace fast-glob with fdir or tinyglobby nestjs/graphql#3307

Closed

1 task

sonsurim added a commit to sonsurim/node that referenced this pull request Aug 29, 2024

fs: refactoring declaratively with the fromAsync function

4dd3a9e

Refs: nodejs#51912

sonsurim added a commit to sonsurim/node that referenced this pull request Aug 29, 2024

fs: refactoring declaratively with the fromAsync function

07d075a

- Refs: nodejs#51912

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fs: expose glob and globSync #51912

fs: expose glob and globSync #51912

MoLow commented Feb 28, 2024

MoLow commented Feb 28, 2024 •

edited

Loading

github-actions bot commented Feb 28, 2024

MoLow Feb 28, 2024

benjamingr Mar 1, 2024

MoLow commented Feb 29, 2024

benjamingr left a comment

aduh95 commented Mar 2, 2024

atlowChemi commented Mar 3, 2024

nodejs-github-bot commented Mar 3, 2024

marco-ippolito commented Apr 15, 2024

MoLow commented Apr 16, 2024

felixfbecker Apr 24, 2024

jonschlinkert commented May 1, 2024

styfle commented May 2, 2024

jonschlinkert commented May 2, 2024

jonschlinkert commented May 2, 2024

styfle commented May 4, 2024

danielbayley commented Jun 22, 2024

fs: expose glob and globSync #51912

fs: expose glob and globSync #51912

Conversation

MoLow commented Feb 28, 2024

MoLow commented Feb 28, 2024 • edited Loading

github-actions bot commented Feb 28, 2024

MoLow Feb 28, 2024

Choose a reason for hiding this comment

benjamingr Mar 1, 2024

Choose a reason for hiding this comment

MoLow commented Feb 29, 2024

benjamingr left a comment

Choose a reason for hiding this comment

aduh95 commented Mar 2, 2024

atlowChemi commented Mar 3, 2024

nodejs-github-bot commented Mar 3, 2024

marco-ippolito commented Apr 15, 2024

MoLow commented Apr 16, 2024

felixfbecker Apr 24, 2024

Choose a reason for hiding this comment

jonschlinkert commented May 1, 2024

styfle commented May 2, 2024

jonschlinkert commented May 2, 2024

jonschlinkert commented May 2, 2024

styfle commented May 4, 2024

danielbayley commented Jun 22, 2024

MoLow commented Feb 28, 2024 •

edited

Loading