Skip to content

Commit

Permalink
fs: deprecate exists() and existsSync()
Browse files Browse the repository at this point in the history
These methods don't follow standard conventions, and shouldn't
be used anyway.

Fixes: #103
PR-URL: #166
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
  • Loading branch information
cjihrig committed Dec 19, 2014
1 parent a553267 commit 5678595
Show file tree
Hide file tree
Showing 9 changed files with 37 additions and 22 deletions.
4 changes: 3 additions & 1 deletion benchmark/misc/module-loader.js
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ function measure(n) {
}

function rmrf(location) {
if (fs.existsSync(location)) {
try {
var things = fs.readdirSync(location);
things.forEach(function(thing) {
var cur = path.join(location, thing),
Expand All @@ -68,5 +68,7 @@ function rmrf(location) {
fs.unlinkSync(cur);
});
fs.rmdirSync(location);
} catch (err) {
// Ignore error
}
}
4 changes: 2 additions & 2 deletions doc/api/fs.markdown
Original file line number Diff line number Diff line change
Expand Up @@ -656,13 +656,13 @@ that leaves you vulnerable to race conditions: another process may remove the
file between the calls to `fs.exists()` and `fs.open()`. Just open the file
and handle the error when it's not there.

`fs.exists()` will be deprecated.
`fs.exists()` is **deprecated**.

## fs.existsSync(path)

Synchronous version of `fs.exists`.

`fs.existsSync()` will be deprecated.
`fs.existsSync()` is **deprecated**.

## fs.access(path[, mode], callback)

Expand Down
8 changes: 4 additions & 4 deletions lib/fs.js
Original file line number Diff line number Diff line change
Expand Up @@ -220,25 +220,25 @@ fs.accessSync = function(path, mode) {
binding.access(pathModule._makeLong(path), mode);
};

fs.exists = function(path, callback) {
fs.exists = util.deprecate(function(path, callback) {
if (!nullCheck(path, cb)) return;
var req = new FSReqWrap();
req.oncomplete = cb;
binding.stat(pathModule._makeLong(path), req);
function cb(err, stats) {
if (callback) callback(err ? false : true);
}
};
}, 'fs.exists() is deprecated. Use fs.access() instead.');

fs.existsSync = function(path) {
fs.existsSync = util.deprecate(function(path) {
try {
nullCheck(path);
binding.stat(pathModule._makeLong(path));
return true;
} catch (e) {
return false;
}
};
}, 'fs.existsSync() is deprecated. Use fs.accessSync() instead.');

fs.readFile = function(path, options, callback_) {
var callback = maybeCallback(arguments[arguments.length - 1]);
Expand Down
14 changes: 13 additions & 1 deletion test/common.js
Original file line number Diff line number Diff line change
Expand Up @@ -58,8 +58,11 @@ if (process.env.NODE_COMMON_PIPE) {
}
}

if (!fs.existsSync(exports.opensslCli))
try {
fs.accessSync(exports.opensslCli);
} catch (err) {
exports.opensslCli = false;
}

if (process.platform === 'win32') {
exports.faketimeCli = false;
Expand Down Expand Up @@ -319,3 +322,12 @@ exports.isValidHostname = function(str) {

return !!str.match(re) && str.length <= 255;
}

exports.fileExists = function(pathname) {
try {
fs.accessSync(pathname);
return true;
} catch (err) {
return false;
}
};
3 changes: 2 additions & 1 deletion test/parallel/test-child-process-spawn-error.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,14 +19,15 @@
// OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE
// USE OR OTHER DEALINGS IN THE SOFTWARE.

var common = require('../common');
var fs = require('fs');
var spawn = require('child_process').spawn;
var assert = require('assert');

var errors = 0;

var enoentPath = 'foo123';
assert.equal(fs.existsSync(enoentPath), false);
assert.equal(common.fileExists(enoentPath), false);

var enoentChild = spawn(enoentPath);
enoentChild.on('error', function (err) {
Expand Down
6 changes: 3 additions & 3 deletions test/parallel/test-fs-mkdir.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ function unlink(pathname) {

fs.mkdir(pathname, function(err) {
assert.equal(err, null);
assert.equal(fs.existsSync(pathname), true);
assert.equal(common.fileExists(pathname), true);
ncalls++;
});

Expand All @@ -56,7 +56,7 @@ function unlink(pathname) {

fs.mkdir(pathname, 511 /*=0777*/, function(err) {
assert.equal(err, null);
assert.equal(fs.existsSync(pathname), true);
assert.equal(common.fileExists(pathname), true);
ncalls++;
});

Expand All @@ -72,7 +72,7 @@ function unlink(pathname) {
unlink(pathname);
fs.mkdirSync(pathname);

var exists = fs.existsSync(pathname);
var exists = common.fileExists(pathname);
unlink(pathname);

assert.equal(exists, true);
Expand Down
4 changes: 2 additions & 2 deletions test/parallel/test-fs-symlink-dir-junction.js
Original file line number Diff line number Diff line change
Expand Up @@ -54,8 +54,8 @@ fs.symlink(linkData, linkPath, 'junction', function(err) {

fs.unlink(linkPath, function(err) {
if (err) throw err;
assert(!fs.existsSync(linkPath));
assert(fs.existsSync(linkData));
assert(!common.fileExists(linkPath));
assert(common.fileExists(linkData));
completed++;
});
});
Expand Down
6 changes: 3 additions & 3 deletions test/pummel/test-fs-watch-file-slow.js
Original file line number Diff line number Diff line change
Expand Up @@ -40,14 +40,14 @@ fs.watchFile(FILENAME, {interval:TIMEOUT - 250}, function(curr, prev) {
console.log([curr, prev]);
switch (++nevents) {
case 1:
assert.equal(fs.existsSync(FILENAME), false);
assert.equal(common.fileExists(FILENAME), false);
break;
case 2:
case 3:
assert.equal(fs.existsSync(FILENAME), true);
assert.equal(common.fileExists(FILENAME), true);
break;
case 4:
assert.equal(fs.existsSync(FILENAME), false);
assert.equal(common.fileExists(FILENAME), false);
fs.unwatchFile(FILENAME);
break;
default:
Expand Down
10 changes: 5 additions & 5 deletions test/sequential/test-regress-GH-3739.js
Original file line number Diff line number Diff line change
Expand Up @@ -44,17 +44,17 @@ for (var i = 0; i < 50; i++) {
}

// Test existsSync
var r = fs.existsSync(dir);
var r = common.fileExists(dir);
if (r !== true) {
cleanup();
throw new Error('fs.existsSync returned false');
throw new Error('fs.accessSync returned false');
}

// Text exists
fs.exists(dir, function(r) {
fs.access(dir, function(err) {
cleanup();
if (r !== true) {
throw new Error('fs.exists reported false');
if (err) {
throw new Error('fs.access reported false');
}
});

Expand Down

1 comment on commit 5678595

@19h
Copy link
Contributor

@19h 19h commented on 5678595 Dec 21, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That is a breaking one. Please make sure to have this listed in release notes!

Please sign in to comment.