[New] Add isDirectory; use to speed up node_modules lookups

This is a backport of 4cf8928 and fa11d48 (browserify#190 and browserify#191) to the 1.x branch. This adds the `isDirectory` option which is needed to drive the directory lookups. This offers a small but useful performance improvement by avoiding unnecessary stat calls.
keithamus · May 14, 2019 · 7b166f2 · 7b166f2
1 parent d098e92
commit 7b166f2
Show file tree

Hide file tree

Showing 5 changed files with 79 additions and 6 deletions.
diff --git a/lib/async.js b/lib/async.js
@@ -15,6 +15,16 @@ var defaultIsFile = function isFile(file, cb) {
     });
 };
 
+var defaultIsDir = function isDirectory(dir, cb) {
+    fs.stat(dir, function (err, stat) {
+        if (!err) {
+            return cb(null, stat.isDirectory());
+        }
+        if (err.code === 'ENOENT' || err.code === 'ENOTDIR') return cb(null, false);
+        return cb(err);
+    });
+};
+
 module.exports = function resolve(x, options, callback) {
     var cb = callback;
     var opts = options;
@@ -32,6 +42,7 @@ module.exports = function resolve(x, options, callback) {
     opts = normalizeOptions(x, opts);
 
     var isFile = opts.isFile || defaultIsFile;
+    var isDirectory = opts.isDirectory || defaultIsDir;
     var readFile = opts.readFile || fs.readFile;
 
     var extensions = opts.extensions || ['.js'];
@@ -208,8 +219,14 @@ module.exports = function resolve(x, options, callback) {
         if (dirs.length === 0) return cb(null, undefined);
         var dir = dirs[0];
 
-        var file = path.join(dir, x);
-        loadAsFile(file, opts.package, onfile);
+        isDirectory(dir, isdir);
+
+        function isdir(err, isdir) {
+            if (err) return cb(err);
+            if (!isdir) return processDirs(cb, dirs.slice(1));
+            var file = path.join(dir, x);
+            loadAsFile(file, opts.package, onfile);
+        }
 
         function onfile(err, m, pkg) {
             if (err) return cb(err);

diff --git a/lib/sync.js b/lib/sync.js
@@ -15,6 +15,16 @@ var defaultIsFile = function isFile(file) {
     return stat.isFile() || stat.isFIFO();
 };
 
+var defaultIsDir = function isDirectory(dir) {
+    try {
+        var stat = fs.statSync(dir);
+    } catch (e) {
+        if (e && (e.code === 'ENOENT' || e.code === 'ENOTDIR')) return false;
+        throw e;
+    }
+    return stat.isDirectory();
+};
+
 module.exports = function (x, options) {
     if (typeof x !== 'string') {
         throw new TypeError('Path must be a string.');
@@ -23,6 +33,7 @@ module.exports = function (x, options) {
 
     var isFile = opts.isFile || defaultIsFile;
     var readFileSync = opts.readFileSync || fs.readFileSync;
+    var isDirectory = opts.isDirectory || defaultIsDir;
 
     var extensions = opts.extensions || ['.js'];
     var basedir = opts.basedir || path.dirname(caller());
@@ -145,10 +156,12 @@ module.exports = function (x, options) {
         var dirs = nodeModulesPaths(start, opts, x);
         for (var i = 0; i < dirs.length; i++) {
             var dir = dirs[i];
-            var m = loadAsFileSync(path.join(dir, '/', x));
-            if (m) return m;
-            var n = loadAsDirectorySync(path.join(dir, '/', x));
-            if (n) return n;
+            if (isDirectory(dir)) {
+                var m = loadAsFileSync(path.join(dir, '/', x));
+                if (m) return m;
+                var n = loadAsDirectorySync(path.join(dir, '/', x));
+                if (n) return n;
+            }
         }
     }
 };
diff --git a/readme.markdown b/readme.markdown
@@ -59,6 +59,8 @@ options are:
 
 * opts.isFile - function to asynchronously test whether a file exists
 
+* opts.isDirectory - function to asynchronously test whether a directory exists
+
 * `opts.packageFilter(pkg, pkgfile)` - transform the parsed package.json contents before looking at the "main" field
   * pkg - package data
   * pkgfile - path to package.json
@@ -101,6 +103,15 @@ default `opts` values:
             return cb(err);
         });
     },
+    isDirectory: function isDirectory(dir, cb) {
+        fs.stat(dir, function (err, stat) {
+            if (!err) {
+                return cb(null, stat.isDirectory());
+            }
+            if (err.code === 'ENOENT' || err.code === 'ENOTDIR') return cb(null, false);
+            return cb(err);
+        });
+    },
     moduleDirectory: 'node_modules',
     preserveSymlinks: true
 }
@@ -121,6 +132,8 @@ options are:
 
 * opts.isFile - function to synchronously test whether a file exists
 
+* opts.isDirectory - function to synchronously test whether a directory exists
+
 * `opts.packageFilter(pkg, dir)` - transform the parsed package.json contents before looking at the "main" field
   * pkg - package data
   * dir - directory for package.json (Note: the second argument will change to "pkgfile" in v2)
@@ -157,6 +170,15 @@ default `opts` values:
         }
         return stat.isFile() || stat.isFIFO();
     },
+    isDirectory: function isDirectory(dir) {
+        try {
+            var stat = fs.statSync(dir);
+        } catch (e) {
+            if (e && (e.code === 'ENOENT' || e.code === 'ENOTDIR')) return false;
+            throw e;
+        }
+        return stat.isDirectory();
+    },
     moduleDirectory: 'node_modules',
     preserveSymlinks: true
 }

diff --git a/test/mock.js b/test/mock.js
@@ -94,12 +94,19 @@ test('mock package', function (t) {
         main: './baz.js'
     });
 
+    var dirs = {};
+    dirs[path.resolve('/foo')] = true;
+    dirs[path.resolve('/foo/node_modules')] = true;
+
     function opts(basedir) {
         return {
             basedir: path.resolve(basedir),
             isFile: function (file, cb) {
                 cb(null, Object.prototype.hasOwnProperty.call(files, path.resolve(file)));
             },
+            isDirectory: function (dir, cb) {
+                cb(null, !!dirs[path.resolve(dir)]);
+            },
             readFile: function (file, cb) {
                 cb(null, files[path.resolve(file)]);
             }
@@ -122,12 +129,19 @@ test('mock package from package', function (t) {
         main: './baz.js'
     });
 
+    var dirs = {};
+    dirs[path.resolve('/foo')] = true;
+    dirs[path.resolve('/foo/node_modules')] = true;
+
     function opts(basedir) {
         return {
             basedir: path.resolve(basedir),
             isFile: function (file, cb) {
                 cb(null, Object.prototype.hasOwnProperty.call(files, path.resolve(file)));
             },
+            isDirectory: function (dir, cb) {
+                cb(null, !!dirs[path.resolve(dir)]);
+            },
             'package': { main: 'bar' },
             readFile: function (file, cb) {
                 cb(null, files[path.resolve(file)]);

diff --git a/test/mock_sync.js b/test/mock_sync.js
@@ -48,12 +48,19 @@ test('mock package', function (t) {
         main: './baz.js'
     });
 
+    var dirs = {};
+    dirs[path.resolve('/foo')] = true;
+    dirs[path.resolve('/foo/node_modules')] = true;
+
     function opts(basedir) {
         return {
             basedir: path.resolve(basedir),
             isFile: function (file) {
                 return Object.prototype.hasOwnProperty.call(files, file);
             },
+            isDirectory: function (dir) {
+                return !!dirs[path.resolve(dir)];
+            },
             readFileSync: function (file) {
                 return files[file];
             }