Skip to content
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 our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(typescript-estree): add maximumDefaultProjectFileMatchCount and wide allowDefaultProjectForFiles glob restrictions #8925

Merged
Show file tree
Hide file tree
Changes from 2 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
9 changes: 9 additions & 0 deletions docs/packages/TypeScript_ESTree.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -288,6 +288,15 @@ interface ProjectServiceOptions {
* Path to a TSConfig to use instead of TypeScript's default project configuration.
*/
defaultProject?: string;

/**
* The maximum number of files {@link allowDefaultProjectForFiles} may match.
* Each file match slows down linting, so if you do need to use this, please
* file an informative issue on typescript-eslint explaining why - so we can
* help you avoid using it!
* @default 8
*/
maximumDefaultProjectFileMatchCount_THIS_WILL_SLOW_DOWN_LINTING?: number;
}

interface ParserServices {
Expand Down
36 changes: 35 additions & 1 deletion docs/troubleshooting/FAQ.mdx
Copy link
Member Author

Choose a reason for hiding this comment

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

Aside: filed #8926 to reduce the scale of this kind of huge FAQs list.

Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,40 @@ If you don't find an existing extension rule, or the extension rule doesn't work
> We release a new version our tooling every week.
> _Please_ ensure that you [check our the latest list of "extension" rules](/rules/#extension-rules) **_before_** filing an issue.

<HiddenHeading id="allowdefaultprojectforfiles-glob-too-wide" />

## I get errors telling me "Having many files run with the default project is known to cause performance issues and slow down linting."

These errors are caused by using the [`EXPERIMENTAL_useProjectService`](../packages/Parser.mdx#experimental_useprojectservice) `allowDefaultProjectForFiles` with an excessively wide glob.
`allowDefaultProjectForFiles` causes a new TypeScript "program" to be built for each "out of project" file it includes, which incurs a performance overhead for each file.

To resolve this error, narrow the glob(s) used for `allowDefaultProjectForFiles` to include fewer files.
For example:

```diff title="eslint.config.js"
parserOptions: {
EXPERIMENTAL_useProjectService: {
allowDefaultProjectForFiles: [
- "**/*.js",
+ "./*.js"
]
}
}
```

You may also need to include more files in your `tsconfig.json`.
For example:

```diff title="tsconfig.json"
"include": [
"src",
+ "*.js"
]
```

If you cannot do this, please [file an issue on typescript-eslint's typescript-estree package](https://github.com/typescript-eslint/typescript-eslint/issues/new?assignees=&labels=enhancement%2Ctriage&projects=&template=07-enhancement-other.yaml&title=Enhancement%3A+%3Ca+short+description+of+my+proposal%3E) telling us your use case and why you need more out-of-project files linted.
Be sure to include a minimal reproduction we can work with to understand your use case!

## I get errors telling me "ESLint was configured to run ... However, that TSConfig does not / none of those TSConfigs include this file"

These errors are caused by an ESLint config requesting type information be generated for a file that isn't included in the TypeScript configuration.
Expand Down Expand Up @@ -499,6 +533,6 @@ If you think you're having issues with performance, see our [Performance Trouble

## Are TypeScript project references supported?

No, TypeScript project references are not yet supported.
Yes, but only with [`EXPERIMENTAL_useProjectService`](../packages/Parser.mdx#experimental_useprojectservice).
Copy link
Member Author

Choose a reason for hiding this comment

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

Aside: we'll definitely want more explanation here 馃槃 but quickly noted this in the meantime. Once this & a few more v8 milestone PRs are in, #7350 tracks a full blog post / explainer / revamp.


See [issue #2094 discussing project references](https://github.com/typescript-eslint/typescript-eslint/issues/2094) for more details.
6 changes: 5 additions & 1 deletion packages/typescript-estree/src/clear-caches.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
import { clearWatchCaches } from './create-program/getWatchProgramsForProjects';
import { clearProgramCache as clearProgramCacheOriginal } from './parser';
import {
clearDefaultProjectMatchedFiles,
clearProgramCache as clearProgramCacheOriginal,
} from './parser';
import {
clearTSConfigMatchCache,
clearTSServerProjectService,
Expand All @@ -14,6 +17,7 @@ import { clearGlobCache } from './parseSettings/resolveProjectList';
* - In custom lint tooling that iteratively lints one project at a time to prevent OOMs.
*/
export function clearCaches(): void {
clearDefaultProjectMatchedFiles();
clearProgramCacheOriginal();
clearWatchCaches();
clearTSConfigMatchCache();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,9 @@ import os from 'node:os';
import type * as ts from 'typescript/lib/tsserverlibrary';

import type { ProjectServiceOptions } from '../parser-options';
import { validateDefaultProjectForFilesGlob } from './validateDefaultProjectForFilesGlob';

const DEFAULT_PROJECT_MATCHED_FILES_THRESHOLD = 8;

const doNothing = (): void => {};

Expand All @@ -15,13 +18,17 @@ export type TypeScriptProjectService = ts.server.ProjectService;

export interface ProjectServiceSettings {
allowDefaultProjectForFiles: string[] | undefined;
maximumDefaultProjectFileMatchCount: number;
service: TypeScriptProjectService;
}

export function createProjectService(
options: boolean | ProjectServiceOptions | undefined,
optionsRaw: boolean | ProjectServiceOptions | undefined,
jsDocParsingMode: ts.JSDocParsingMode | undefined,
): ProjectServiceSettings {
const options = typeof optionsRaw === 'object' ? optionsRaw : {};
validateDefaultProjectForFilesGlob(options);

// We import this lazily to avoid its cost for users who don't use the service
// TODO: Once we drop support for TS<5.3 we can import from "typescript" directly
const tsserver = require('typescript/lib/tsserverlibrary') as typeof ts;
Expand Down Expand Up @@ -60,7 +67,7 @@ export function createProjectService(
jsDocParsingMode,
});

if (typeof options === 'object' && options.defaultProject) {
if (options.defaultProject) {
let configRead;

try {
Expand Down Expand Up @@ -97,10 +104,10 @@ export function createProjectService(
}

return {
allowDefaultProjectForFiles:
typeof options === 'object'
? options.allowDefaultProjectForFiles
: undefined,
allowDefaultProjectForFiles: options.allowDefaultProjectForFiles,
maximumDefaultProjectFileMatchCount:
options.maximumDefaultProjectFileMatchCount_THIS_WILL_SLOW_DOWN_LINTING ??
DEFAULT_PROJECT_MATCHED_FILES_THRESHOLD,
service,
};
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
import type { ProjectServiceOptions } from '../parser-options';

export const DEFAULT_PROJECT_FILES_ERROR_EXPLANATION = `

Having many files run with the default project is known to cause performance issues and slow down linting.

See https://typescript-eslint.io/troubleshooting/#allowdefaultprojectforfiles-glob-too-wide
`;

export function validateDefaultProjectForFilesGlob(
options: ProjectServiceOptions,
): void {
if (!options.allowDefaultProjectForFiles?.length) {
return;
}

for (const glob of options.allowDefaultProjectForFiles) {
if (glob === '*') {
throw new Error(
`allowDefaultProjectForFiles glob '${glob}' is the overly wide '*'.${DEFAULT_PROJECT_FILES_ERROR_EXPLANATION}`,
);
}
if (glob.includes('**')) {
throw new Error(
`allowDefaultProjectForFiles glob '${glob}' contains a disallowed '**'.${DEFAULT_PROJECT_FILES_ERROR_EXPLANATION}`,
);
}
}
}
9 changes: 9 additions & 0 deletions packages/typescript-estree/src/parser-options.ts
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,15 @@ export interface ProjectServiceOptions {
* Path to a TSConfig to use instead of TypeScript's default project configuration.
*/
defaultProject?: string;

/**
* The maximum number of files {@link allowDefaultProjectForFiles} may match.
* Each file match slows down linting, so if you do need to use this, please
* file an informative issue on typescript-eslint explaining why - so we can
* help you avoid using it!
* @default 8
*/
maximumDefaultProjectFileMatchCount_THIS_WILL_SLOW_DOWN_LINTING?: number;
}

interface ParseAndGenerateServicesOptions extends ParseOptions {
Expand Down
7 changes: 7 additions & 0 deletions packages/typescript-estree/src/parser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,11 @@ function clearProgramCache(): void {
existingPrograms.clear();
}

const defaultProjectMatchedFiles = new Set<string>();
function clearDefaultProjectMatchedFiles(): void {
defaultProjectMatchedFiles.clear();
}

/**
* @param parseSettings Internal settings for parsing the file
* @param hasFullTypeInformation True if the program should be attempted to be calculated from provided tsconfig files
Expand All @@ -54,6 +59,7 @@ function getProgramAndAST(
parseSettings.EXPERIMENTAL_projectService,
parseSettings,
hasFullTypeInformation,
defaultProjectMatchedFiles,
);
if (fromProjectService) {
return fromProjectService;
Expand Down Expand Up @@ -286,6 +292,7 @@ export {
parse,
parseAndGenerateServices,
ParseAndGenerateServicesResult,
clearDefaultProjectMatchedFiles,
clearProgramCache,
clearParseAndGenerateServicesCalls,
};
22 changes: 21 additions & 1 deletion packages/typescript-estree/src/useProgramFromProjectService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,16 +5,22 @@ import path from 'path';
import { createProjectProgram } from './create-program/createProjectProgram';
import type { ProjectServiceSettings } from './create-program/createProjectService';
import type { ASTAndDefiniteProgram } from './create-program/shared';
import { DEFAULT_PROJECT_FILES_ERROR_EXPLANATION } from './create-program/validateDefaultProjectForFilesGlob';
import type { MutableParseSettings } from './parseSettings';

const log = debug(
'typescript-eslint:typescript-estree:useProgramFromProjectService',
);

export function useProgramFromProjectService(
{ allowDefaultProjectForFiles, service }: ProjectServiceSettings,
{
allowDefaultProjectForFiles,
maximumDefaultProjectFileMatchCount,
service,
}: ProjectServiceSettings,
parseSettings: Readonly<MutableParseSettings>,
hasFullTypeInformation: boolean,
defaultProjectMatchedFiles: Set<string>,
): ASTAndDefiniteProgram | undefined {
// We don't canonicalize the filename because it caused a performance regression.
// See https://github.com/typescript-eslint/typescript-eslint/issues/8519
Expand Down Expand Up @@ -77,6 +83,20 @@ export function useProgramFromProjectService(
return undefined;
}

defaultProjectMatchedFiles.add(filePathAbsolute);
if (defaultProjectMatchedFiles.size > maximumDefaultProjectFileMatchCount) {
throw new Error(
`Too many files (>${maximumDefaultProjectFileMatchCount}) have matched the default project.${DEFAULT_PROJECT_FILES_ERROR_EXPLANATION}
Matching files:
${Array.from(defaultProjectMatchedFiles)
.map(file => `- ${file}`)
.join('\n')}

If you absolutely need more files included, set parserOptions.EXPERIMENTAL_useProjectService.maximumDefaultProjectFileMatchCount_THIS_WILL_SLOW_DOWN_LINTING to a larger value.
`,
);
}

log('Found project service program for: %s', filePathAbsolute);

return createProjectProgram(parseSettings, [program]);
Expand Down
Loading
Loading