-
Notifications
You must be signed in to change notification settings - Fork 30.1k
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: deprecate exists() and existsSync() #166
Conversation
@@ -228,9 +228,9 @@ fs.exists = function(path, callback) { | |||
function cb(err, stats) { | |||
if (callback) callback(err ? false : true); | |||
} | |||
}; | |||
}, 'fs: exists() is deprecated.'); |
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.
Other modules write it as module.function()
and point the user to what to use instead (e.g. 'http.createClient is deprecated. Use http.request instead.'
). I would suggest following that.
It's probably a good idea to update most uses of fs.exists() in the test suit as well. The only exceptions I can think of are simple/test-fs-exists and maybe simple/test-fs-null-bytes. |
These methods don't follow standard conventions, and shouldn't be used anyway.
@bnoordhuis ready for review again. |
Actually, |
@cjihrig LGTM @vtambourine I kind of like it. I hope it discourages the anti-pattern of checking if a file exists before trying to open it. |
Can you please explain why this is considered as anti-pattern? Besides, consider following situation. I want to ensure that set of particular files exists (set of configs, for example, |
Because the way most people use it, it's a TOCTOU race condition: the file may have changed or have been deleted between checking and opening it. If you are going to open it, then you need to be prepared for such eventualities, so you might as well skip the check and open it straight away. |
Ok, thank you. |
Landed in 5678595 |
These methods don't follow standard conventions, and shouldn't be used anyway.
This closes #103