Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Aug 14, 2025

The fs.promises.glob method in memfs was incorrectly returning Promise<string[]> instead of the Node.js standard AsyncIterableIterator<string>. This made it incompatible with the native Node.js API and prevented users from using the standard for await...of iteration pattern.

What was wrong

// Before: Incorrect return type
fs.promises.glob: (pattern: string, options?: IGlobOptions) => Promise<string[]>

// Usage required awaiting the entire result set
const files = await fs.promises.glob('*.js');
files.forEach(file => console.log(file));

What's fixed

// After: Correct return type with proper overloads
fs.promises.glob: {
  (pattern: string | readonly string[]): AsyncIterableIterator<string>;
  (pattern: string | readonly string[], options: IGlobOptions & { withFileTypes: true }): AsyncIterableIterator<IDirent>;
  (pattern: string | readonly string[], options: IGlobOptions & { withFileTypes?: false }): AsyncIterableIterator<string>;
  (pattern: string | readonly string[], options?: IGlobOptions): AsyncIterableIterator<IDirent | string>;
}

// Usage now matches Node.js exactly
for await (const file of fs.promises.glob('*.js')) {
  console.log(file);
}

// Supports withFileTypes option
for await (const dirent of fs.promises.glob('*.js', { withFileTypes: true })) {
  console.log(dirent.name, dirent.isFile());
}

// Supports multiple patterns
for await (const file of fs.promises.glob(['*.js', '*.ts'])) {
  console.log(file);
}

Implementation

  • Created GlobAsyncIterator: A custom AsyncIterator implementation that yields glob results incrementally
  • Added proper method overloads: Supporting all Node.js glob API variants including withFileTypes and multiple patterns
  • Maintained backward compatibility: Existing functionality works unchanged, just accessed via iteration
  • Enhanced test coverage: Added tests for withFileTypes option and multiple pattern support

The fix ensures memfs now perfectly matches the Node.js fs.promises.glob API, enabling seamless migration between real and in-memory file systems.

Fixes #1161.


💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

Copilot AI changed the title [WIP] Bug: fs.promises.glob incorrect return typing feat: fix fs.promises.glob return type to match Node.js API Aug 14, 2025
Copilot AI requested a review from streamich August 14, 2025 17:52
Copilot finished work on behalf of streamich August 14, 2025 17:52
@streamich streamich marked this pull request as ready for review August 15, 2025 16:22
@tokerson
Copy link

tokerson commented Nov 13, 2025

Dear @streamich, first of all, thank you for the memfs package. It made unit tests that operate on the filesystem in my project a lot easier to write and maintain.

However, I suffer from the issue that this Pull Request is trying to fix.
I managed to patch locally the [email protected] with the changes suggested in this PR, and the tests started passing.

Before this fix, I had to mock the glob in a nasty way like:

vi.mock('node:fs/promises', async () => {
  return {
    ...memfs.promises,
    glob: vi.fn(),
  };
});

async function* mockGlobIterator(
  paths: string[],
): AsyncGenerator<string, undefined, unknown> {
  for (const p of paths) yield p;
}

and later
vi.mocked(glob).mockImplementationOnce(() =>
   mockGlobIterator(Object.keys(expectedKeys)),
);

Do you have plans to review this Pull Request in the near future?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Bug: fs.promises.glob incorrect return typing

3 participants