From fd91a00000c7e99d9cb01677d66ff1462b66c079 Mon Sep 17 00:00:00 2001 From: SukkaW Date: Sat, 29 Jun 2024 21:53:31 +0800 Subject: [PATCH 1/3] refactor(espree): replace `globby` w/ `fast-glob` --- packages/typescript-estree/package.json | 2 +- .../src/parseSettings/resolveProjectList.ts | 2 +- .../typescript-estree/tests/lib/parse.test.ts | 36 +++++++++---------- yarn.lock | 2 +- 4 files changed, 21 insertions(+), 21 deletions(-) diff --git a/packages/typescript-estree/package.json b/packages/typescript-estree/package.json index 05c9c708d672..cba434939378 100644 --- a/packages/typescript-estree/package.json +++ b/packages/typescript-estree/package.json @@ -57,7 +57,7 @@ "@typescript-eslint/types": "8.2.0", "@typescript-eslint/visitor-keys": "8.2.0", "debug": "^4.3.4", - "globby": "^11.1.0", + "fast-glob": "^3.3.2", "is-glob": "^4.0.3", "minimatch": "^9.0.4", "semver": "^7.6.0", diff --git a/packages/typescript-estree/src/parseSettings/resolveProjectList.ts b/packages/typescript-estree/src/parseSettings/resolveProjectList.ts index 1e2155cd0103..99c077d24112 100644 --- a/packages/typescript-estree/src/parseSettings/resolveProjectList.ts +++ b/packages/typescript-estree/src/parseSettings/resolveProjectList.ts @@ -1,5 +1,5 @@ import debug from 'debug'; -import { sync as globSync } from 'globby'; +import { sync as globSync } from 'fast-glob'; import isGlob from 'is-glob'; import type { CanonicalPath } from '../create-program/shared'; diff --git a/packages/typescript-estree/tests/lib/parse.test.ts b/packages/typescript-estree/tests/lib/parse.test.ts index 609fe139549d..98e4d8969c44 100644 --- a/packages/typescript-estree/tests/lib/parse.test.ts +++ b/packages/typescript-estree/tests/lib/parse.test.ts @@ -2,7 +2,7 @@ import { join, resolve } from 'node:path'; import type { CacheDurationSeconds } from '@typescript-eslint/types'; import debug from 'debug'; -import * as globbyModule from 'globby'; +import * as fastGlobModule from 'fast-glob'; import type * as typescriptModule from 'typescript'; import * as parser from '../../src'; @@ -39,11 +39,11 @@ jest.mock('typescript', () => { }; }); -jest.mock('globby', () => { - const globby = jest.requireActual('globby'); +jest.mock('fast-glob', () => { + const fastGlob = jest.requireActual('fast-glob'); return { - ...globby, - sync: jest.fn(globby.sync), + ...fastGlob, + sync: jest.fn(fastGlob.sync), }; }); @@ -52,7 +52,7 @@ const hrtimeSpy = jest.spyOn(process, 'hrtime'); const createDefaultCompilerOptionsFromExtra = jest.mocked( sharedParserUtilsModule.createDefaultCompilerOptionsFromExtra, ); -const globbySyncMock = jest.mocked(globbyModule.sync); +const fastGlobSyncMock = jest.mocked(fastGlobModule.sync); /** * Aligns paths between environments, node for windows uses `\`, for linux and mac uses `/` @@ -699,46 +699,46 @@ describe('parseAndGenerateServices', () => { it('should cache globs if the lifetime is non-zero', () => { doParse(30); - expect(globbySyncMock).toHaveBeenCalledTimes(1); + expect(fastGlobSyncMock).toHaveBeenCalledTimes(1); doParse(30); - // shouldn't call globby again due to the caching - expect(globbySyncMock).toHaveBeenCalledTimes(1); + // shouldn't call fast-glob again due to the caching + expect(fastGlobSyncMock).toHaveBeenCalledTimes(1); }); it('should not cache globs if the lifetime is zero', () => { doParse(0); - expect(globbySyncMock).toHaveBeenCalledTimes(1); + expect(fastGlobSyncMock).toHaveBeenCalledTimes(1); doParse(0); - // should call globby again because we specified immediate cache expiry - expect(globbySyncMock).toHaveBeenCalledTimes(2); + // should call fast-glob again because we specified immediate cache expiry + expect(fastGlobSyncMock).toHaveBeenCalledTimes(2); }); it('should evict the cache if the entry expires', () => { hrtimeSpy.mockReturnValueOnce([1, 0]); doParse(30); - expect(globbySyncMock).toHaveBeenCalledTimes(1); + expect(fastGlobSyncMock).toHaveBeenCalledTimes(1); // wow so much time has passed hrtimeSpy.mockReturnValueOnce([Number.MAX_VALUE, 0]); doParse(30); - // shouldn't call globby again due to the caching - expect(globbySyncMock).toHaveBeenCalledTimes(2); + // shouldn't call fast-glob again due to the caching + expect(fastGlobSyncMock).toHaveBeenCalledTimes(2); }); it('should infinitely cache if passed Infinity', () => { hrtimeSpy.mockReturnValueOnce([1, 0]); doParse('Infinity'); - expect(globbySyncMock).toHaveBeenCalledTimes(1); + expect(fastGlobSyncMock).toHaveBeenCalledTimes(1); // wow so much time has passed hrtimeSpy.mockReturnValueOnce([Number.MAX_VALUE, 0]); doParse('Infinity'); - // shouldn't call globby again due to the caching - expect(globbySyncMock).toHaveBeenCalledTimes(1); + // shouldn't call fast-glob again due to the caching + expect(fastGlobSyncMock).toHaveBeenCalledTimes(1); }); }); }); diff --git a/yarn.lock b/yarn.lock index e61cb0a1d696..4db1bcc4beb1 100644 --- a/yarn.lock +++ b/yarn.lock @@ -5908,8 +5908,8 @@ __metadata: "@typescript-eslint/types": 8.2.0 "@typescript-eslint/visitor-keys": 8.2.0 debug: ^4.3.4 + fast-glob: ^3.3.2 glob: "*" - globby: ^11.1.0 is-glob: ^4.0.3 jest: 29.7.0 minimatch: ^9.0.4 From 0f018eb8c98cb8958b5bc73e9463ba013030b2cf Mon Sep 17 00:00:00 2001 From: SukkaW Date: Wed, 10 Jul 2024 21:33:48 +0800 Subject: [PATCH 2/3] fix(estree): call fast glob properly --- .../src/parseSettings/resolveProjectList.ts | 38 ++++++++++--------- .../typescript-estree/tests/lib/parse.test.ts | 25 +++++++----- 2 files changed, 37 insertions(+), 26 deletions(-) diff --git a/packages/typescript-estree/src/parseSettings/resolveProjectList.ts b/packages/typescript-estree/src/parseSettings/resolveProjectList.ts index 99c077d24112..7e9a1032325d 100644 --- a/packages/typescript-estree/src/parseSettings/resolveProjectList.ts +++ b/packages/typescript-estree/src/parseSettings/resolveProjectList.ts @@ -56,15 +56,15 @@ export function resolveProjectList( const projectFolderIgnoreList = ( options.projectFolderIgnoreList ?? ['**/node_modules/**'] - ) - .reduce((acc, folder) => { - if (typeof folder === 'string') { - acc.push(folder); - } - return acc; - }, []) - // prefix with a ! for not match glob - .map(folder => (folder.startsWith('!') ? folder : `!${folder}`)); + ).reduce((acc, folder) => { + if (typeof folder === 'string') { + acc.push( + // prefix with a ! for not match glob + folder.startsWith('!') ? folder : `!${folder}`, + ); + } + return acc; + }, []); const cacheKey = getHash({ project: sanitizedProjects, @@ -93,16 +93,20 @@ export function resolveProjectList( const nonGlobProjects = sanitizedProjects.filter(project => !isGlob(project)); const globProjects = sanitizedProjects.filter(project => isGlob(project)); + let globProjectPaths: string[] = []; + + if (globProjects.length > 0) { + globProjectPaths = globProjects.flatMap(pattern => + globSync(pattern, { + cwd: options.tsconfigRootDir, + ignore: projectFolderIgnoreList, + }), + ); + } + const uniqueCanonicalProjectPaths = new Map( nonGlobProjects - .concat( - globProjects.length === 0 - ? [] - : globSync([...globProjects, ...projectFolderIgnoreList], { - cwd: options.tsconfigRootDir, - dot: true, - }), - ) + .concat(globProjectPaths) .map(project => [ getCanonicalFileName( ensureAbsolutePath(project, options.tsconfigRootDir), diff --git a/packages/typescript-estree/tests/lib/parse.test.ts b/packages/typescript-estree/tests/lib/parse.test.ts index 98e4d8969c44..430939d412d7 100644 --- a/packages/typescript-estree/tests/lib/parse.test.ts +++ b/packages/typescript-estree/tests/lib/parse.test.ts @@ -685,6 +685,9 @@ describe('parseAndGenerateServices', () => { describe('cacheLifetime', () => { describe('glob', () => { + const project = ['./**/tsconfig.json', './**/tsconfig.extra.json']; + const expectFastGlobCalls = project.length; + function doParse(lifetime: CacheDurationSeconds): void { parser.parseAndGenerateServices('const x = 1', { cacheLifetime: { @@ -693,52 +696,56 @@ describe('parseAndGenerateServices', () => { disallowAutomaticSingleRunInference: true, filePath: join(FIXTURES_DIR, 'file.ts'), tsconfigRootDir: FIXTURES_DIR, - project: ['./**/tsconfig.json', './**/tsconfig.extra.json'], + project, }); } it('should cache globs if the lifetime is non-zero', () => { doParse(30); - expect(fastGlobSyncMock).toHaveBeenCalledTimes(1); + expect(fastGlobSyncMock).toHaveBeenCalledTimes(expectFastGlobCalls); doParse(30); // shouldn't call fast-glob again due to the caching - expect(fastGlobSyncMock).toHaveBeenCalledTimes(1); + expect(fastGlobSyncMock).toHaveBeenCalledTimes(expectFastGlobCalls); }); it('should not cache globs if the lifetime is zero', () => { doParse(0); - expect(fastGlobSyncMock).toHaveBeenCalledTimes(1); + expect(fastGlobSyncMock).toHaveBeenCalledTimes(expectFastGlobCalls); doParse(0); // should call fast-glob again because we specified immediate cache expiry - expect(fastGlobSyncMock).toHaveBeenCalledTimes(2); + expect(fastGlobSyncMock).toHaveBeenCalledTimes( + expectFastGlobCalls * 2, + ); }); it('should evict the cache if the entry expires', () => { hrtimeSpy.mockReturnValueOnce([1, 0]); doParse(30); - expect(fastGlobSyncMock).toHaveBeenCalledTimes(1); + expect(fastGlobSyncMock).toHaveBeenCalledTimes(expectFastGlobCalls); // wow so much time has passed hrtimeSpy.mockReturnValueOnce([Number.MAX_VALUE, 0]); doParse(30); // shouldn't call fast-glob again due to the caching - expect(fastGlobSyncMock).toHaveBeenCalledTimes(2); + expect(fastGlobSyncMock).toHaveBeenCalledTimes( + expectFastGlobCalls * 2, + ); }); it('should infinitely cache if passed Infinity', () => { hrtimeSpy.mockReturnValueOnce([1, 0]); doParse('Infinity'); - expect(fastGlobSyncMock).toHaveBeenCalledTimes(1); + expect(fastGlobSyncMock).toHaveBeenCalledTimes(expectFastGlobCalls); // wow so much time has passed hrtimeSpy.mockReturnValueOnce([Number.MAX_VALUE, 0]); doParse('Infinity'); // shouldn't call fast-glob again due to the caching - expect(fastGlobSyncMock).toHaveBeenCalledTimes(1); + expect(fastGlobSyncMock).toHaveBeenCalledTimes(expectFastGlobCalls); }); }); }); From 15ac7bef6dcb6cfc9ad91ec891ddc0fb640421e2 Mon Sep 17 00:00:00 2001 From: SukkaW Date: Wed, 10 Jul 2024 21:39:31 +0800 Subject: [PATCH 3/3] chore(estree): add comments --- .../src/parseSettings/resolveProjectList.ts | 3 +++ packages/typescript-estree/tests/lib/parse.test.ts | 5 ++++- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/packages/typescript-estree/src/parseSettings/resolveProjectList.ts b/packages/typescript-estree/src/parseSettings/resolveProjectList.ts index 7e9a1032325d..c625914f3fa5 100644 --- a/packages/typescript-estree/src/parseSettings/resolveProjectList.ts +++ b/packages/typescript-estree/src/parseSettings/resolveProjectList.ts @@ -96,6 +96,9 @@ export function resolveProjectList( let globProjectPaths: string[] = []; if (globProjects.length > 0) { + // Although fast-glob supports multiple patterns, fast-glob returns arbitrary order of results + // to improve performance. To ensure the order is correct, we need to call fast-glob for each pattern + // separately and then concatenate the results in patterns' order. globProjectPaths = globProjects.flatMap(pattern => globSync(pattern, { cwd: options.tsconfigRootDir, diff --git a/packages/typescript-estree/tests/lib/parse.test.ts b/packages/typescript-estree/tests/lib/parse.test.ts index 430939d412d7..e5aa4c0442fa 100644 --- a/packages/typescript-estree/tests/lib/parse.test.ts +++ b/packages/typescript-estree/tests/lib/parse.test.ts @@ -686,8 +686,11 @@ describe('parseAndGenerateServices', () => { describe('cacheLifetime', () => { describe('glob', () => { const project = ['./**/tsconfig.json', './**/tsconfig.extra.json']; + // fast-glob returns arbitrary order of results to improve performance. + // `resolveProjectList()` calls fast-glob for each pattern to ensure the + // order is correct. + // Thus the expected call time of spy is the number of patterns. const expectFastGlobCalls = project.length; - function doParse(lifetime: CacheDurationSeconds): void { parser.parseAndGenerateServices('const x = 1', { cacheLifetime: {