Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

feat(typescript-estree): replaceglobby w/fast-glob#9518

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
Show file tree
Hide file tree
Changes fromall commits
Commits
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletionpackages/typescript-estree/package.json
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -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",
Expand Down
View file
Open in desktop
Original file line numberDiff line numberDiff line change
@@ -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';
Expand DownExpand Up@@ -56,15 +56,15 @@ export function resolveProjectList(

const projectFolderIgnoreList = (
options.projectFolderIgnoreList ?? ['**/node_modules/**']
)
.reduce<string[]>((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<string[]>((acc, folder) => {
if (typeof folder === 'string') {
acc.push(
// prefix with a ! for not match glob
folder.startsWith('!') ? folder : `!${folder}`,
);
}
return acc;
}, []);
Comment on lines +59 to +67
Copy link
ContributorAuthor

Choose a reason for hiding this comment

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

A bonus change, replace the.reduce.map chain with a single.reduce to avoid unnecessary iteration.

JoshuaKGoldberg reacted with rocket emoji

const cacheKey = getHash({
project: sanitizedProjects,
Expand DownExpand Up@@ -93,16 +93,23 @@ export function resolveProjectList(
const nonGlobProjects = sanitizedProjects.filter(project => !isGlob(project));
const globProjects = sanitizedProjects.filter(project => isGlob(project));

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

Choose a reason for hiding this comment

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

OOC, does the order matter?

Copy link
ContributorAuthor

Choose a reason for hiding this comment

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

There is a unit test case against the order, so yes, it does matter.

Choose a reason for hiding this comment

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

But why does the order matter? Shouldn't the unit test just check that the needed files are linted?

Copy link
Member

@JoshuaKGoldbergJoshuaKGoldbergAug 30, 2024•
edited
Loading

Choose a reason for hiding this comment

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

Folks sometimes do things like['packages/*/tsconfig.json', 'tsconfig.json'] - and/or entries like'tsconfig.eslint.json'. The order intent is to first try the lower-level TSConfigs first.

Choose a reason for hiding this comment

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

Gotcha, thanks!

// separately and then concatenate the results in patterns' order.
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),
Expand Down
48 changes: 29 additions & 19 deletionspackages/typescript-estree/tests/lib/parse.test.ts
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -2,7 +2,7 @@ import { join, resolve } from 'node:path';

import type { CacheDurationSeconds } from '@typescript-eslint/types';
import debug from 'debug';
import * asglobbyModule from 'globby';
import * asfastGlobModule from 'fast-glob';
import type * as typescriptModule from 'typescript';

import * as parser from '../../src';
Expand DownExpand Up@@ -39,11 +39,11 @@ jest.mock('typescript', () => {
};
});

jest.mock('globby', () => {
constglobby = jest.requireActual<typeofglobbyModule>('globby');
jest.mock('fast-glob', () => {
constfastGlob = jest.requireActual<typeoffastGlobModule>('fast-glob');
return {
...globby,
sync: jest.fn(globby.sync),
...fastGlob,
sync: jest.fn(fastGlob.sync),
};
});

Expand All@@ -52,7 +52,7 @@ const hrtimeSpy = jest.spyOn(process, 'hrtime');
const createDefaultCompilerOptionsFromExtra = jest.mocked(
sharedParserUtilsModule.createDefaultCompilerOptionsFromExtra,
);
constglobbySyncMock = jest.mocked(globbyModule.sync);
constfastGlobSyncMock = jest.mocked(fastGlobModule.sync);

/**
* Aligns paths between environments, node for windows uses `\`, for linux and mac uses `/`
Expand DownExpand Up@@ -685,6 +685,12 @@ 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: {
Expand All@@ -693,52 +699,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(globbySyncMock).toHaveBeenCalledTimes(1);
expect(fastGlobSyncMock).toHaveBeenCalledTimes(expectFastGlobCalls);
doParse(30);
// shouldn't callglobby again due to the caching
expect(globbySyncMock).toHaveBeenCalledTimes(1);
// shouldn't callfast-glob again due to the caching
expect(fastGlobSyncMock).toHaveBeenCalledTimes(expectFastGlobCalls);
});

it('should not cache globs if the lifetime is zero', () => {
doParse(0);
expect(globbySyncMock).toHaveBeenCalledTimes(1);
expect(fastGlobSyncMock).toHaveBeenCalledTimes(expectFastGlobCalls);
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(
expectFastGlobCalls * 2,
);
});

it('should evict the cache if the entry expires', () => {
hrtimeSpy.mockReturnValueOnce([1, 0]);

doParse(30);
expect(globbySyncMock).toHaveBeenCalledTimes(1);
expect(fastGlobSyncMock).toHaveBeenCalledTimes(expectFastGlobCalls);

// 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(
expectFastGlobCalls * 2,
);
});

it('should infinitely cache if passed Infinity', () => {
hrtimeSpy.mockReturnValueOnce([1, 0]);

doParse('Infinity');
expect(globbySyncMock).toHaveBeenCalledTimes(1);
expect(fastGlobSyncMock).toHaveBeenCalledTimes(expectFastGlobCalls);

// wow so much time has passed
hrtimeSpy.mockReturnValueOnce([Number.MAX_VALUE, 0]);

doParse('Infinity');
// shouldn't callglobby again due to the caching
expect(globbySyncMock).toHaveBeenCalledTimes(1);
// shouldn't callfast-glob again due to the caching
expect(fastGlobSyncMock).toHaveBeenCalledTimes(expectFastGlobCalls);
});
});
});
Expand Down
2 changes: 1 addition & 1 deletionyarn.lock
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -5915,8 +5915,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
Expand Down
Loading
[8]ページ先頭
©2009-2025 Movatter.jp