diff --git a/CHANGELOG.md b/CHANGELOG.md index d507a365eedf..af88c7714c23 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,6 +21,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 diff --git a/packages/jest-haste-map/src/crawlers/__tests__/node.test.js b/packages/jest-haste-map/src/crawlers/__tests__/node.test.js index 69d3c3d6ca73..6130a2f8ffe8 100644 --- a/packages/jest-haste-map/src/crawlers/__tests__/node.test.js +++ b/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); + }); + }); + }); }); diff --git a/packages/jest-haste-map/src/crawlers/node.ts b/packages/jest-haste-map/src/crawlers/node.ts index 6e6b6b5d4c9d..833f51f80f14 100644 --- a/packages/jest-haste-map/src/crawlers/node.ts +++ b/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); }