-
Notifications
You must be signed in to change notification settings - Fork 29.8k
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
fs: use w flag for writeFileSync with utf8 encoding when flag not specified #50990
Conversation
b5305d0
to
b5283b6
Compare
Can you add a test? |
b5283b6
to
aaa4edd
Compare
@anonrig Done :) |
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!
It would be great to get this fixed asap with semver patch release at least 👮 |
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 |
Landed in 7bfb087 |
PR-URL: #50990 Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Yagiz Nizipli <yagiz.nizipli@sentry.io>
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 <luigipinca@gmail.com> Reviewed-By: Yagiz Nizipli <yagiz.nizipli@sentry.io>
PR-URL: #50990 Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Yagiz Nizipli <yagiz.nizipli@sentry.io>
PR #49884 seems to have accidentally changed the behavior for
fs.writeFileSync
with 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_CREAT
flag touvlib
anymore.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
w
flag (which includesO_CREAT
) explicitly when callingwriteFileSync
. e.g:This PR will just set the
w
flag back as the default value when it is not specified, so its the same behavior from previous node versions.Fixes #50989