Implement fs.glob, fs.globSync, and fs/promises.glob by fraidev · Pull Request #6378 · cloudflare/workerd

fraidev · 2026-03-22T02:44:09Z

Implements the Node.js fs.glob APIs that were previously stubbed with ERR_UNSUPPORTED_OPERATION. Uses a custom glob-to-regex pattern matcher.

Closes #5416

github-actions · 2026-03-22T02:44:21Z

All contributors have signed the CLA ✍️ ✅
_{Posted by the CLA Assistant Lite bot.}

fraidev · 2026-03-22T02:47:12Z

I have read the CLA Document and I hereby sign the CLA

jasnell · 2026-03-23T15:39:34Z

Hey @fraidev! thank you for this! I've done a first pass look over and have asked the AI agent to review also.. it should post it's findings in a moment. How closely does this follow the node.js implementation? (I haven't looked at it in a while) Is it a straight up port of the original or did you reimplement? The main reason I ask is that if it is a straight port over, we might need to carry over the Node.js copyright/license detail in the header of the new file(s).

jasnell

Review Summary

Note: This review was generated by an AI assistant (Claude) and may contain inaccuracies. Please evaluate each suggestion on its merits.

Findings (6):

[HIGH] exclude type handling — unsafe cast in compileExclude when exclude is a user function
[MEDIUM] ** in middle of exclude pattern doesn't match zero segments (a/**/b won't match a/b)
[MEDIUM] walkGlob has unbounded recursion depth — no depth guard
[MEDIUM] Non-null assertions (!) used extensively — violates ts-style convention
[MEDIUM] Missing callback guard in fs.glob — throws when callback omitted
[LOW] Copyright year says 2017-2022 in new file

src/node/internal/internal_fs_glob.ts

src/node/internal/internal_fs_callback.ts

src/node/internal/internal_fs_glob.ts

fraidev · 2026-03-24T00:06:03Z

Hey @fraidev! thank you for this! I've done a first pass look over and have asked the AI agent to review also.. it should post it's findings in a moment. How closely does this follow the node.js implementation? (I haven't looked at it in a while) Is it a straight up port of the original or did you reimplement? The main reason I ask is that if it is a straight port over, we might need to carry over the Node.js copyright/license detail in the header of the new file(s).

Hey @jasnell! This is a reimplementation, not a port. Node.js uses minimatch internally, which isn't available in the Workers runtime, so I built glob-to-regex to convert and pattern-driven directory walker from scratch.
The test patterns and fixture structure are derived from Node.js's test-fs-glob.mjs to verify compatibility, but the implementation shares no code with Node.js.

fraidev · 2026-03-24T00:07:49Z

@jasnell is there a way to run CI for a non-Cloudflare member?

jasnell · 2026-03-24T00:20:53Z

Just did. Internal_build will fail but I'll run that one manually

fraidev · 2026-03-24T02:53:05Z

Just did. Internal_build will fail but I'll run that one manually

Great! I fixed ESLint errors. I think that we need one more run.

codecov-commenter · 2026-03-24T14:12:56Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 70.79%. Comparing base (a9fc5bb) to head (f726828).
⚠️ Report is 9 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff            @@
##             main    #6378    +/-   ##
========================================
  Coverage   70.79%   70.79%            
========================================
  Files         427      427            
  Lines      117724   117869   +145     
  Branches    18909    18935    +26     
========================================
+ Hits        83338    83446   +108     
- Misses      23132    23154    +22     
- Partials    11254    11269    +15

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

fraidev · 2026-03-26T00:41:58Z

Hi @jasnell, this PR is ready for another review. It was passing all tests (except Internal_build).
I just added a new commit to fix merge conflicts.

jasnell · 2026-03-26T15:36:30Z

@fraidev ... yeah, I'm just a bit backlogged on a number of PR review items. I've got this on my agenda to fully review either later today or tomorrow

src/node/internal/internal_fs_promises.ts

src/node/internal/internal_fs_callback.ts

jasnell · 2026-03-26T20:26:15Z

@fraidev ... can I ask you to remove the merge commit and instead rebase on main? Additional review comments coming shortly.

jasnell

Correctness & Performance Analysis

Note: This review was generated by an AI assistant (Claude) and may contain inaccuracies. Please evaluate each suggestion on its merits.

Findings (5):

[MEDIUM] splitTopLevel lost backslash escape handling when converting to for...of
[MEDIUM] segmentToRegexStr doesn't escape { and } — patterns like a{3} silently produce wrong matches
[HIGH] .. traversal guard uses length comparison instead of prefix check
[MEDIUM] Consecutive ** segments not collapsed — potential exponential blowup
[MEDIUM] segmentToRegex recompiled on every recursive walkGlob call

src/node/internal/internal_fs_glob.ts

jasnell · 2026-03-26T20:27:19Z

src/node/internal/internal_fs_glob.ts

+
+  // Handle '..' — go up one directory (but don't escape cwd)
+  if (seg === '..') {
+    if (currentAbsPath === cwd || currentAbsPath.length <= cwd.length) {


[HIGH] The length comparison currentAbsPath.length <= cwd.length is not a safe containment check. While it works today because currentAbsPath is always built by appending to cwd, it's brittle — it would silently do the wrong thing if the code were refactored to allow other path construction.

Consider using a prefix check instead:

Suggested change

if (currentAbsPath === cwd || currentAbsPath.length <= cwd.length) {

if (currentAbsPath === cwd || !currentAbsPath.startsWith(cwd + '/')) {

Replaced currentAbsPath.length <= cwd.length with !currentAbsPath.startsWith(cwd + '/')

src/node/internal/internal_fs_glob.ts

Implements the Node.js fs.glob APIs that were previously stubbed with ERR_UNSUPPORTED_OPERATION. Uses a custom glob-to-regex pattern matcher instead of the minimatch library which is unavailable in the Workers runtime. Closes cloudflare#5416

fraidev · 2026-03-26T21:30:39Z

@fraidev ... can I ask you to remove the merge commit and instead rebase on main? Additional review comments coming shortly.

Sure, done

jasnell · 2026-03-27T00:25:20Z

Not quite sure what's happening with the CI right now. I'll try to run it again tomorrow and hopefully the issues are transient.

fraidev · 2026-03-27T03:24:47Z

Not quite sure what's happening with the CI right now. I'll try to run it again tomorrow and hopefully the issues are transient.

oh, I think it's just lint. I think I fixed it

jasnell · 2026-03-27T21:19:38Z

Manual testing on the internal-build is green. Merging! Thank you @fraidev !

github-actions bot added a commit that referenced this pull request Mar 22, 2026

@fraidev has signed the CLA in #6378

2dd0c53

jasnell reviewed Mar 23, 2026

View reviewed changes

fraidev requested a review from jasnell March 24, 2026 00:08

fraidev changed the title ~~Feat/implement fs glob~~ Implement fs.glob, fs.globSync, and fs/promises.glob Mar 24, 2026

fraidev marked this pull request as ready for review March 24, 2026 18:49

fraidev requested review from a team as code owners March 24, 2026 18:49

jasnell reviewed Mar 26, 2026

View reviewed changes

src/node/internal/internal_fs_promises.ts Outdated Show resolved Hide resolved

jasnell reviewed Mar 26, 2026

View reviewed changes

src/node/internal/internal_fs_callback.ts Outdated Show resolved Hide resolved

jasnell reviewed Mar 26, 2026

View reviewed changes

fraidev added 6 commits March 26, 2026 17:49

wip

7138341

wip

b059f82

wip

80d2884

lint

1e46fb2

wip

97e8aa8

fraidev force-pushed the feat/implement-fs-glob branch from 756e1b9 to 97e8aa8 Compare March 26, 2026 21:27

jasnell approved these changes Mar 27, 2026

View reviewed changes

lint

f726828

This comment was marked as outdated.

Sign in to view

jasnell merged commit 47f5816 into cloudflare:main Mar 27, 2026
17 of 18 checks passed

	if (currentAbsPath === cwd \|\| currentAbsPath.length <= cwd.length) {
	if (currentAbsPath === cwd \|\| !currentAbsPath.startsWith(cwd + '/')) {

Conversation

fraidev commented Mar 22, 2026

Uh oh!

github-actions bot commented Mar 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

fraidev commented Mar 22, 2026

Uh oh!

jasnell commented Mar 23, 2026

Uh oh!

jasnell left a comment

Choose a reason for hiding this comment

Review Summary

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

fraidev commented Mar 24, 2026

Uh oh!

fraidev commented Mar 24, 2026

Uh oh!

jasnell commented Mar 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

fraidev commented Mar 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov-commenter commented Mar 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

fraidev commented Mar 26, 2026

Uh oh!

jasnell commented Mar 26, 2026

Uh oh!

Uh oh!

Uh oh!

jasnell commented Mar 26, 2026

Uh oh!

jasnell left a comment

Choose a reason for hiding this comment

Correctness & Performance Analysis

Uh oh!

Uh oh!

Uh oh!

jasnell Mar 26, 2026

Choose a reason for hiding this comment

Uh oh!

fraidev Mar 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

fraidev commented Mar 26, 2026

Uh oh!

jasnell commented Mar 27, 2026

Uh oh!

fraidev commented Mar 27, 2026

Uh oh!

This comment was marked as outdated.

jasnell commented Mar 27, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

github-actions bot commented Mar 22, 2026 •

edited

Loading

jasnell commented Mar 24, 2026 •

edited

Loading

fraidev commented Mar 24, 2026 •

edited

Loading

codecov-commenter commented Mar 24, 2026 •

edited

Loading

fraidev Mar 26, 2026 •

edited

Loading