Skip to content

Conversation

Tango992
Copy link
Contributor

@Tango992 Tango992 commented Jul 24, 2025

Notable changes:

  • Introduces a separate op between Deno.open and node:fs.open.
  • Removes redundant existenceCheckRequired and Deno.lstatSync calls, as the op layer already handles that when options.create_new is true and uses synchronous I/O when O_SYNC is passed.
  • Allows passing custom flags to the op (e.g. O_SYNC).
  • Addresses prefer-primordials lint rule.
  • Allows parallel/test-fs-open.js test to pass. There are also several tests that have passed before that I added to the config.toml.

I decided to make a separate op function between the Deno fs and node, because Deno.OpenOptions currently doesn't allow passing numeric flags.

There's a caveat though, because the node:fs.constants are only correctly defined for macOS, passing these constants to the open function might not work as intended (excluding O_APPEND, O_CREAT, O_EXCL, O_TRUNC, O_RDONLY, O_WRONLY, O_RDWR). PR #30113 fixes this. Once merged it will allow parallel/test-fs-open-flags.js to pass.

I'm very open for any feedback.

Towards: #29972, #24236

- Accepts "rs" flag.
- Allows passing custom flags to the op.
- Addresses `prefer-primordials` lint rule.
- Allows "parallel/test-fs-open.js" test to pass.
Copy link
Member

@dsherret dsherret left a comment

Choose a reason for hiding this comment

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

Great!

@dsherret dsherret enabled auto-merge (squash) July 31, 2025 14:22
@dsherret dsherret merged commit 8680d97 into denoland:main Jul 31, 2025
18 checks passed
@Tango992 Tango992 deleted the fix-node-fs-open branch July 31, 2025 15:18
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.

2 participants