Skip to content

Implement Node.js-compatible Dir API with proper error handling and state management #1115

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

Merged
merged 4 commits into from
Jul 27, 2025

Conversation

Copilot
Copy link
Contributor

@Copilot Copilot AI commented Jul 26, 2025

This PR implements a complete Node.js-compatible Dir API for memfs, addressing all the requirements outlined in the issue. The implementation now matches Node.js fs.Dir behavior exactly, including proper error handling, state management, and concurrent operation detection.

Key Changes

1. Removed Helper Functions

  • Eliminated wrapAsync, isFunction, and promisify helper methods
  • Replaced with standard Node.js patterns for consistency

2. Enhanced State Management

  • Added closed property to track directory handle state
  • Implemented operationQueue for managing concurrent async operations
  • Added proper state validation in all methods

3. Comprehensive Error Handling

  • Added ERR_DIR_CLOSED error code for operations on closed directories
  • Added ERR_DIR_CONCURRENT_OPERATION error code for sync operations during async operations
  • Implemented proper error timing using process.nextTick() for callbacks

4. Fixed Path Property

  • Changed from getParentPath() to getPath() to return the correct directory path
  • Now properly returns the path of the directory being opened, not its parent

5. Improved Method Implementations

Close Operations:

// All three patterns now work correctly with proper error handling
await dir.close();           // Promise-based
dir.close(callback);         // Callback-based  
dir.closeSync();            // Synchronous

Error Handling Examples:

const dir = fs.opendirSync('/test');
dir.closeSync();

// All of these now throw ERR_DIR_CLOSED appropriately
dir.readSync();              // Throws synchronously
await dir.read();            // Rejects promise
dir.read(callback);          // Calls callback with error

Concurrent Operation Detection:

const dir = fs.opendirSync('/test');
dir.read(callback);          // Start async operation
dir.readSync();              // Throws ERR_DIR_CONCURRENT_OPERATION

6. Refactored Async Iterator

  • Simplified implementation to use the standard read() method
  • Maintains compatibility with for await loops
  • Fixed linting issues with proper arrow function usage

Testing

Added comprehensive test suite covering:

  • All close operation variants (sync, callback, promise)
  • Error conditions for closed directories
  • Concurrent operation detection
  • Path property validation
  • Async iterator functionality

All existing tests continue to pass, ensuring no regressions were introduced.

Compatibility

The implementation now matches Node.js fs.Dir behavior exactly:

  • Same error codes and messages
  • Same timing for callback errors
  • Same state management patterns
  • Same concurrent operation handling

Fixes #1087.


💬 Share your feedback on Copilot coding agent for the chance to win a $200 gift card! Click here to start the survey.

…tate management

Co-authored-by: streamich <9773803+streamich@users.noreply.github.com>
@streamich streamich marked this pull request as ready for review July 26, 2025 23:36
Co-authored-by: streamich <9773803+streamich@users.noreply.github.com>
@Copilot Copilot AI changed the title [WIP] Implememt Dir API Implement Node.js-compatible Dir API with proper error handling and state management Jul 26, 2025
@Copilot Copilot AI requested a review from streamich July 26, 2025 23:37
Copilot finished work on behalf of streamich July 26, 2025 23:37
Copy link
Owner

@streamich streamich left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Run yarn prettier and save changes.

Co-authored-by: streamich <9773803+streamich@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented Jul 27, 2025

Run yarn prettier and save changes.

Done! Ran yarn prettier and saved the formatting changes. Also fixed a typo in the error message where "operatio ns" was corrected to "operations". Commit: 9fb854d

@Copilot Copilot AI requested a review from streamich July 27, 2025 09:20
Copilot finished work on behalf of streamich July 27, 2025 09:20
Copy link
Owner

@streamich streamich left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@streamich streamich merged commit cdc257a into master Jul 27, 2025
19 of 20 checks passed
@streamich streamich deleted the copilot/fix-1087-2 branch July 27, 2025 09:51
Copy link

🎉 This PR is included in version 4.21.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Implememt Dir API
2 participants