Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 34.2k
fs: use w flag for writeFileSync with utf8 encoding when flag not specified#50990
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
fs: use w flag for writeFileSync with utf8 encoding when flag not specified #50990
Uh oh!
There was an error while loading. Please reload this page.
Conversation
MuriloKakazu commented Dec 1, 2023 • edited
Loading Uh oh!
There was an error while loading. Please reload this page.
edited
Uh oh!
There was an error while loading. Please reload this page.
b5305d0 to b5283b6Compareanonrig commented Dec 1, 2023
Can you add a test? |
b5283b6 to aaa4eddCompareMuriloKakazu commented Dec 1, 2023
@anonrig Done :) |
nodejs-github-bot commented Dec 1, 2023
chenrui333 left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Works for me. Thanks for the quick turnaround!
bricss commented Dec 2, 2023
It would be great to get this fixed asap with semver patch release at least 👮 |
nodejs-github-bot commented Dec 2, 2023
nodejs-github-bot commented Dec 3, 2023
Commit Queue failed- Loading data for nodejs/node/pull/50990 ✔ Done loading data for nodejs/node/pull/50990 ----------------------------------- PR info ------------------------------------ Title fs: use w flag for writeFileSync with utf8 encoding when flag not specified (#50990) Author Murilo Kakazu (@MuriloKakazu, first-time contributor) Branch MuriloKakazu:fix/default-flag-fs-write-file-sync -> nodejs:main Labels fs, author ready, needs-ci Commits 2 - fs: use default w flag for writeFileSync with utf8 encoding - fs: add tests for writeFileSync with no flag Committers 1 - Murilo Kakazu PR-URL: https://github.com/nodejs/node/pull/50990 Reviewed-By: Luigi Pinca Reviewed-By: Yagiz Nizipli ------------------------------ Generated metadata ------------------------------ PR-URL: https://github.com/nodejs/node/pull/50990 Reviewed-By: Luigi Pinca Reviewed-By: Yagiz Nizipli -------------------------------------------------------------------------------- ℹ This PR was created on Fri, 01 Dec 2023 07:05:02 GMT ✔ Approvals: 2 ✔ - Luigi Pinca (@lpinca): https://github.com/nodejs/node/pull/50990#pullrequestreview-1760486535 ✔ - Yagiz Nizipli (@anonrig) (TSC): https://github.com/nodejs/node/pull/50990#pullrequestreview-1760934016 ✔ Last GitHub CI successful ℹ Last Full PR CI on 2023-12-02T18:04:37Z: https://ci.nodejs.org/job/node-test-pull-request/56046/ - Querying data for job/node-test-pull-request/56046/ ✔ Last Jenkins CI successful -------------------------------------------------------------------------------- ✔ No git cherry-pick in progress ✔ No git am in progress ✔ No git rebase in progress -------------------------------------------------------------------------------- - Bringing origin/main up to date... From https://github.com/nodejs/node * branch main -> FETCH_HEAD ✔ origin/main is now up-to-date - Downloading patch for 50990 From https://github.com/nodejs/node * branch refs/pull/50990/merge -> FETCH_HEAD ✔ Fetched commits as 23031d9b0a56..aaa4edda7d2e -------------------------------------------------------------------------------- Auto-merging lib/fs.js [main 2fba1f3a18] fs: use default w flag for writeFileSync with utf8 encoding Author: Murilo Kakazu Date: Fri Dec 1 03:45:10 2023 -0300 1 file changed, 3 insertions(+), 3 deletions(-) [main 934e830937] fs: add tests for writeFileSync with no flag Author: Murilo Kakazu Date: Fri Dec 1 13:46:14 2023 -0300 1 file changed, 16 insertions(+) ✔ Patches applied There are 2 commits in the PR. Attempting autorebase. Rebasing (2/4)https://github.com/nodejs/node/actions/runs/7075388656 |
nodejs-github-bot commented Dec 3, 2023
Landed in 7bfb087 |
PR-URL: #50990 Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Yagiz Nizipli <[email protected]>
Notable changes: fs: * (SEMVER-MINOR) introduce `dirent.parentPath` (Antoine du Hamel) nodejs#50976 * use default w flag for writeFileSync with utf8 encoding (Murilo Kakazu) nodejs#50990 PR-URL: nodejs#51043
PR-URL: #50990 Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Yagiz Nizipli <[email protected]>
PR-URL: #50990 Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Yagiz Nizipli <[email protected]>
PR #49884 seems to have accidentally changed the behavior for
fs.writeFileSyncwith utf-8 encoding when the file does not exist, as compared to previous node versions.On a low level, it seems we are not passing the
O_CREATflag touvlibanymore.Examples:
In node 16.16.0: ✅
In node 21.2.0: ✅
In node 21.3.0 (currently latest): ❌
Currently, a workaround for 21.3.0 is to pass the
wflag (which includesO_CREAT) explicitly when callingwriteFileSync. e.g:This PR will just set the
wflag back as the default value when it is not specified, so its the same behavior from previous node versions.Fixes#50989