Skip to content
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

Wrap require("fs").promises if "fs" is required #416

Merged
merged 2 commits into from
Oct 16, 2024
Merged

Conversation

hansott
Copy link
Collaborator

@hansott hansott commented Oct 15, 2024

No description provided.

We only wrapped require("fs/promises") when fs/promises is required

This would leave require("fs").promises unprotected if just fs is
required

We should only wrap promises once

They point to the same functions:

Welcome to Node.js v21.4.0.
Type ".help" for more information.
> require("fs").promises.readFile === require("fs/promises").readFile
true
>
@hansott hansott marked this pull request as ready for review October 15, 2024 12:16
@hansott hansott changed the title Add test for require("fs").promises usage @hansott Wrap require("fs").promises if "fs" is required Oct 15, 2024
@hansott hansott changed the title @hansott Wrap require("fs").promises if "fs" is required Wrap require("fs").promises if "fs" is required Oct 15, 2024
Copy link

codecov bot commented Oct 15, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

📢 Thoughts on this report? Let us know!

@hansott hansott requested a review from timokoessler October 15, 2024 13:42
@hansott hansott merged commit 4d5404f into beta Oct 16, 2024
8 checks passed
@hansott hansott deleted the patch-fs-promises branch October 16, 2024 09:16
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