Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 34.3k
fs: deprecate exists() and existsSync()#166
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
Uh oh!
There was an error while loading. Please reload this page.
Conversation
lib/fs.js Outdated
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.
bnoordhuis commented Dec 15, 2014
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.
cjihrig commented Dec 18, 2014
@bnoordhuis ready for review again. |
vtambourine commented Dec 18, 2014
Actually, |
bnoordhuis commented Dec 18, 2014
@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. |
vtambourine commented Dec 18, 2014
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, |
bnoordhuis commented Dec 18, 2014
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. |
vtambourine commented Dec 18, 2014
Ok, thank you. |
These methods don't follow standard conventions, and shouldn't be used anyway. Fixes: #103 PR-URL: #166 Reviewed-By: Ben Noordhuis <[email protected]>
cjihrig commented Dec 19, 2014
Landed in 5678595 |
These methods don't follow standard conventions, and shouldn't be used anyway.
This closes#103