[jest-haste-map] reduce the number of lstat calls in node crawl… (#9514)

* reduce the number of lstat calls

* update changelog

* handle node < v10.10

* lint

* link to PR from changelog

* fix tests

* add lstat tests

* skip checks in lstat callback

* Revert "skip checks in lstat callback"

This reverts commit 758c76a.

Co-authored-by: Simen Bekkhus <[email protected]>

CHANGELOG.md

@@ -22,6 +22,8 @@
 
 ### Performance
 
+- `[jest-haste-map]` Reduce number of `lstat` calls in node crawler ([#9514](https://github.com/facebook/jest/pull/9514))
+
 ## 25.1.0
 
 ### Features

packages/jest-haste-map/src/crawlers/__tests__/node.test.js

@@ -31,6 +31,8 @@ jest.mock('child_process', () => ({
   }),
 }));
 
+let mockHasReaddirWithFileTypesSupport = false;
+
 jest.mock('fs', () => {
   let mtime = 32;
   const size = 42;
@@ -42,7 +44,7 @@ jest.mock('fs', () => {
             return path.endsWith('/directory');
           },
           isSymbolicLink() {
-            return false;
+            return path.endsWith('symlink');
           },
           mtime: {
             getTime() {
@@ -56,13 +58,66 @@ jest.mock('fs', () => {
   };
   return {
     lstat: jest.fn(stat),
-    readdir: jest.fn((dir, callback) => {
-      if (dir === '/project/fruits') {
-        setTimeout(() => callback(null, ['directory', 'tomato.js']), 0);
-      } else if (dir === '/project/fruits/directory') {
-        setTimeout(() => callback(null, ['strawberry.js']), 0);
-      } else if (dir == '/error') {
-        setTimeout(() => callback({code: 'ENOTDIR'}, undefined), 0);
+    readdir: jest.fn((dir, options, callback) => {
+      // readdir has an optional `options` arg that's in the middle of the args list.
+      // we always provide it in practice, but let's try to handle the case where it's not
+      // provided too
+      if (typeof callback === 'undefined') {
+        if (typeof options === 'function') {
+          callback = options;
+        }
+        throw new Error('readdir: callback is not a function!');
+      }
+
+      if (mockHasReaddirWithFileTypesSupport) {
+        if (dir === '/project/fruits') {
+          setTimeout(
+            () =>
+              callback(null, [
+                {
+                  isDirectory: () => true,
+                  isSymbolicLink: () => false,
+                  name: 'directory',
+                },
+                {
+                  isDirectory: () => false,
+                  isSymbolicLink: () => false,
+                  name: 'tomato.js',
+                },
+                {
+                  isDirectory: () => false,
+                  isSymbolicLink: () => true,
+                  name: 'symlink',
+                },
+              ]),
+            0,
+          );
+        } else if (dir === '/project/fruits/directory') {
+          setTimeout(
+            () =>
+              callback(null, [
+                {
+                  isDirectory: () => false,
+                  isSymbolicLink: () => false,
+                  name: 'strawberry.js',
+                },
+              ]),
+            0,
+          );
+        } else if (dir == '/error') {
+          setTimeout(() => callback({code: 'ENOTDIR'}, undefined), 0);
+        }
+      } else {
+        if (dir === '/project/fruits') {
+          setTimeout(
+            () => callback(null, ['directory', 'tomato.js', 'symlink']),
+            0,
+          );
+        } else if (dir === '/project/fruits/directory') {
+          setTimeout(() => callback(null, ['strawberry.js']), 0);
+        } else if (dir == '/error') {
+          setTimeout(() => callback({code: 'ENOTDIR'}, undefined), 0);
+        }
       }
     }),
     stat: jest.fn(stat),
@@ -296,4 +351,65 @@ describe('node crawler', () => {
       expect(removedFiles).toEqual(new Map());
     });
   });
+
+  describe('readdir withFileTypes support', () => {
+    it('calls lstat for directories and symlinks if readdir withFileTypes is not supported', () => {
+      nodeCrawl = require('../node');
+      const fs = require('fs');
+
+      const files = new Map();
+      return nodeCrawl({
+        data: {files},
+        extensions: ['js'],
+        forceNodeFilesystemAPI: true,
+        ignore: pearMatcher,
+        rootDir,
+        roots: ['/project/fruits'],
+      }).then(({hasteMap, removedFiles}) => {
+        expect(hasteMap.files).toEqual(
+          createMap({
+            'fruits/directory/strawberry.js': ['', 33, 42, 0, '', null],
+            'fruits/tomato.js': ['', 32, 42, 0, '', null],
+          }),
+        );
+        expect(removedFiles).toEqual(new Map());
+        // once for /project/fruits, once for /project/fruits/directory
+        expect(fs.readdir).toHaveBeenCalledTimes(2);
+        // once for each of:
+        // 1. /project/fruits/directory
+        // 2. /project/fruits/directory/strawberry.js
+        // 3. /project/fruits/tomato.js
+        // 4. /project/fruits/symlink
+        // (we never call lstat on the root /project/fruits, since we know it's a directory)
+        expect(fs.lstat).toHaveBeenCalledTimes(4);
+      });
+    });
+    it('avoids calling lstat for directories and symlinks if readdir withFileTypes is supported', () => {
+      mockHasReaddirWithFileTypesSupport = true;
+      nodeCrawl = require('../node');
+      const fs = require('fs');
+
+      const files = new Map();
+      return nodeCrawl({
+        data: {files},
+        extensions: ['js'],
+        forceNodeFilesystemAPI: true,
+        ignore: pearMatcher,
+        rootDir,
+        roots: ['/project/fruits'],
+      }).then(({hasteMap, removedFiles}) => {
+        expect(hasteMap.files).toEqual(
+          createMap({
+            'fruits/directory/strawberry.js': ['', 33, 42, 0, '', null],
+            'fruits/tomato.js': ['', 32, 42, 0, '', null],
+          }),
+        );
+        expect(removedFiles).toEqual(new Map());
+        // once for /project/fruits, once for /project/fruits/directory
+        expect(fs.readdir).toHaveBeenCalledTimes(2);
+        // once for strawberry.js, once for tomato.js
+        expect(fs.lstat).toHaveBeenCalledTimes(2);
+      });
+    });
+  });
 });

packages/jest-haste-map/src/crawlers/node.ts

@@ -32,22 +32,42 @@ function find(
 
   function search(directory: string): void {
     activeCalls++;
-    fs.readdir(directory, (err, names) => {
+    fs.readdir(directory, {withFileTypes: true}, (err, entries) => {
       activeCalls--;
       if (err) {
         callback(result);
         return;
       }
-      names.forEach(file => {
-        file = path.join(directory, file);
+      // node < v10.10 does not support the withFileTypes option, and
+      // entry will be a string.
+      entries.forEach((entry: string | fs.Dirent) => {
+        const file = path.join(
+          directory,
+          typeof entry === 'string' ? entry : entry.name,
+        );
+
         if (ignore(file)) {
           return;
         }
+
+        if (typeof entry !== 'string') {
+          if (entry.isSymbolicLink()) {
+            return;
+          }
+
+          if (entry.isDirectory()) {
+            search(file);
+            return;
+          }
+        }
+
         activeCalls++;
 
         fs.lstat(file, (err, stat) => {
           activeCalls--;
 
+          // This logic is unnecessary for node > v10.10, but leaving it in
+          // since we need it for backwards-compatibility still.
           if (!err && stat && !stat.isSymbolicLink()) {
             if (stat.isDirectory()) {
               search(file);
@@ -58,6 +78,7 @@ function find(
               }
             }
           }
+
           if (activeCalls === 0) {
             callback(result);
           }