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.

Sign up for GitHub

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

Already on GitHub? Sign in to your account

Don’t try to recreate AutoImportProvider after giving up because of too many dependencies until program structure changes #59692

Merged
merged 9 commits into from
Aug 21, 2024
Merged

Don’t try to recreate AutoImportProvider after giving up because of too many dependencies until program structure changes #59692

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
0551b3b
Fix max dependencies comparison
andrewbranch Aug 19, 2024
9ccaf09
Add test
andrewbranch Aug 19, 2024
d44feec
Don’t try to recreate AutoImportProvider after giving up because of t…
andrewbranch Aug 19, 2024
1a7af52
Demonstrate cache reevaluates after program structure change
andrewbranch Aug 19, 2024
a263ca2
Update baselines
andrewbranch Aug 19, 2024
4e5b8aa
Fix onPackageJsonChange
andrewbranch Aug 19, 2024
84c3f82
Simplify dirty marking calls
andrewbranch Aug 20, 2024
94b6dee
Merge branch 'main' into bug/auto-import-provider-duplicate-creation-…
andrewbranch Aug 20, 2024
91774f0
Update new baselines
andrewbranch Aug 20, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
The table of contents is too big for display.
Diff view
Diff view
  •  
  •  
  •  
15 changes: 4 additions & 11 deletions src/server/project.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1365,20 +1365,13 @@ export abstract class Project implements LanguageServiceHost, ModuleResolutionHo

/** @internal */
onAutoImportProviderSettingsChanged() {
if (this.autoImportProviderHost === false) {
this.autoImportProviderHost = undefined;
}
else {
this.autoImportProviderHost?.markAsDirty();
}
this.markAutoImportProviderAsDirty();
}

/** @internal */
onPackageJsonChange() {
this.moduleSpecifierCache.clear();
if (this.autoImportProviderHost) {
this.autoImportProviderHost.markAsDirty();
}
this.markAutoImportProviderAsDirty();
}

/** @internal */
Expand Down Expand Up @@ -2275,7 +2268,7 @@ export abstract class Project implements LanguageServiceHost, ModuleResolutionHo
if (dependencySelection) {
tracing?.push(tracing.Phase.Session, "getPackageJsonAutoImportProvider");
const start = timestamp();
this.autoImportProviderHost = AutoImportProviderProject.create(dependencySelection, this, this.getHostForAutoImportProvider(), this.documentRegistry);
this.autoImportProviderHost = AutoImportProviderProject.create(dependencySelection, this, this.getHostForAutoImportProvider(), this.documentRegistry) ?? false;
if (this.autoImportProviderHost) {
updateProjectIfDirty(this.autoImportProviderHost);
this.sendPerformanceEvent("CreatePackageJsonAutoImportProvider", timestamp() - start);
Expand Down Expand Up @@ -2552,7 +2545,7 @@ export class AutoImportProviderProject extends Project {
const symlinkCache = hostProject.getSymlinkCache();
for (const name of arrayFrom(dependencyNames.keys())) {
// Avoid creating a large project that would significantly slow down time to editor interactivity
if (dependencySelection === PackageJsonAutoImportPreference.Auto && dependenciesAdded > this.maxDependencies) {
if (dependencySelection === PackageJsonAutoImportPreference.Auto && dependenciesAdded >= this.maxDependencies) {
hostProject.log(`AutoImportProviderProject: attempted to add more than ${this.maxDependencies} dependencies. Aborting.`);
return ts.emptyArray;
}
Expand Down
3 changes: 2 additions & 1 deletion src/testRunner/unittests/tsserver/autoImportProvider.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,7 @@ describe("unittests:: tsserver:: autoImportProvider", () => {

host.writeFile(packageJson.path, packageJson.content);
session.host.baselineHost("Before getAutoImportProvider");
host.runQueuedTimeoutCallbacks();
assert.ok(session.getProjectService().configuredProjects.get(tsconfig.path)!.getLanguageService().getAutoImportProvider());
session.host.baselineHost("After getAutoImportProvider");
baselineTsserverLogs("autoImportProvider", "Responds to package_json changes", session);
Expand Down Expand Up @@ -272,7 +273,7 @@ describe("unittests:: tsserver:: autoImportProvider", () => {
];

const packages = [];
for (let i = 0; i < 11; i++) {
for (let i = 0; i < 10; i++) {
packages.push(createPackage(i));
}

Expand Down
4 changes: 4 additions & 0 deletions ...elines/reference/tsserver/applyChangesToOpenFiles/with-applyChangedToOpenFiles-request.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -193,6 +193,7 @@ Projects::
/a/b/tsconfig.json (Configured) *new*
projectStateVersion: 1
projectProgramVersion: 1
autoImportProviderHost: false

ScriptInfos::
/a/b/app.ts (Open) *new*
Expand Down Expand Up @@ -285,6 +286,7 @@ Projects::
/a/b/tsconfig.json (Configured) *changed*
projectStateVersion: 2 *changed*
projectProgramVersion: 1
autoImportProviderHost: false

ScriptInfos::
/a/b/app.ts (Open)
Expand Down Expand Up @@ -412,6 +414,7 @@ Projects::
/a/b/tsconfig.json (Configured) *changed*
projectStateVersion: 3 *changed*
projectProgramVersion: 1
autoImportProviderHost: false

ScriptInfos::
/a/b/app.ts (Open) *changed*
Expand Down Expand Up @@ -490,6 +493,7 @@ Projects::
/a/b/tsconfig.json (Configured) *changed*
projectStateVersion: 4 *changed*
projectProgramVersion: 1
autoImportProviderHost: false

ScriptInfos::
/a/b/app.ts (Open)
Expand Down
4 changes: 4 additions & 0 deletions tests/baselines/reference/tsserver/applyChangesToOpenFiles/with-updateOpen-request.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -193,6 +193,7 @@ Projects::
/a/b/tsconfig.json (Configured) *new*
projectStateVersion: 1
projectProgramVersion: 1
autoImportProviderHost: false

ScriptInfos::
/a/b/app.ts (Open) *new*
Expand Down Expand Up @@ -285,6 +286,7 @@ Projects::
/a/b/tsconfig.json (Configured) *changed*
projectStateVersion: 2 *changed*
projectProgramVersion: 1
autoImportProviderHost: false

ScriptInfos::
/a/b/app.ts (Open)
Expand Down Expand Up @@ -420,6 +422,7 @@ Projects::
/a/b/tsconfig.json (Configured) *changed*
projectStateVersion: 3 *changed*
projectProgramVersion: 1
autoImportProviderHost: false

ScriptInfos::
/a/b/app.ts (Open) *changed*
Expand Down Expand Up @@ -498,6 +501,7 @@ Projects::
/a/b/tsconfig.json (Configured) *changed*
projectStateVersion: 4 *changed*
projectProgramVersion: 1
autoImportProviderHost: false

ScriptInfos::
/a/b/app.ts (Open)
Expand Down
1 change: 1 addition & 0 deletions ...tsserver/autoImportProvider/Auto-importable-file-is-in-inferred-project-until-imported.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -460,6 +460,7 @@ Projects::
/tsconfig.json (Configured) *new*
projectStateVersion: 1
projectProgramVersion: 1
autoImportProviderHost: false

ScriptInfos::
/index.ts (Open) *new*
Expand Down
10 changes: 2 additions & 8 deletions ...ortProvider/Does-not-create-an-auto-import-provider-if-there-are-too-many-dependencies.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,12 +61,6 @@ Before request
//// [/node_modules/package9/index.d.ts]


//// [/node_modules/package10/package.json]
{ "name": "package10" }

//// [/node_modules/package10/index.d.ts]


//// [/index.ts]


Expand All @@ -84,8 +78,7 @@ Before request
"package6": "*",
"package7": "*",
"package8": "*",
"package9": "*",
"package10": "*"
"package9": "*"
}


Expand Down Expand Up @@ -306,6 +299,7 @@ Projects::
/tsconfig.json (Configured) *new*
projectStateVersion: 1
projectProgramVersion: 1
autoImportProviderHost: false

ScriptInfos::
/index.ts (Open) *new*
Expand Down
53 changes: 1 addition & 52 deletions ...ider/Does-not-schedule-ensureProjectForOpenFiles-on-AutoImportProviderProject-creation.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -228,6 +228,7 @@ Projects::
/tsconfig.json (Configured) *new*
projectStateVersion: 1
projectProgramVersion: 1
autoImportProviderHost: false

ScriptInfos::
/index.ts (Open) *new*
Expand Down Expand Up @@ -298,56 +299,4 @@ FsWatchesRecursive::
/:
{}

Info seq [hh:mm:ss:mss] AutoImportProviderProject: found 1 root files in 1 dependencies 0 referenced projects in * ms
Info seq [hh:mm:ss:mss] DirectoryWatcher:: Added:: WatchInfo: /node_modules 1 undefined WatchType: node_modules for closed script infos and package.jsons affecting module specifier cache
Info seq [hh:mm:ss:mss] Elapsed:: *ms DirectoryWatcher:: Added:: WatchInfo: /node_modules 1 undefined WatchType: node_modules for closed script infos and package.jsons affecting module specifier cache
Info seq [hh:mm:ss:mss] Starting updateGraphWorker: Project: /dev/null/autoImportProviderProject1*
Info seq [hh:mm:ss:mss] FileWatcher:: Added:: WatchInfo: /node_modules/@angular/forms/package.json 2000 undefined Project: /dev/null/autoImportProviderProject1* WatchType: File location affecting resolution
Info seq [hh:mm:ss:mss] Finishing updateGraphWorker: Project: /dev/null/autoImportProviderProject1* projectStateVersion: 1 projectProgramVersion: 0 structureChanged: true structureIsReused:: Not Elapsed:: *ms
Info seq [hh:mm:ss:mss] Project '/dev/null/autoImportProviderProject1*' (AutoImportProvider)
Info seq [hh:mm:ss:mss] Files (1)
/node_modules/@angular/forms/forms.d.ts Text-1 "export declare class PatternValidator {}"


node_modules/@angular/forms/forms.d.ts
Root file specified for compilation

Info seq [hh:mm:ss:mss] -----------------------------------------------
After getPackageJsonAutoImportProvider

PolledWatches::
/a/lib/lib.d.ts:
{"pollingInterval":500}

FsWatches::
/node_modules/@angular/forms/package.json: *new*
{}
/package.json:
{}
/tsconfig.json:
{}

FsWatchesRecursive::
/:
{}
/node_modules: *new*
{}

Projects::
/dev/null/autoImportProviderProject1* (AutoImportProvider) *new*
projectStateVersion: 1
projectProgramVersion: 1
/tsconfig.json (Configured) *changed*
projectStateVersion: 1
projectProgramVersion: 1
autoImportProviderHost: /dev/null/autoImportProviderProject1* *changed*

ScriptInfos::
/index.ts (Open)
version: SVC-1-0
containingProjects: 1
/tsconfig.json *default*
/node_modules/@angular/forms/forms.d.ts *new*
version: Text-1
containingProjects: 1
/dev/null/autoImportProviderProject1*
7 changes: 7 additions & 0 deletions ...elines/reference/tsserver/autoImportProvider/Recovers-from-an-unparseable-package_json.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -234,6 +234,7 @@ Projects::
/tsconfig.json (Configured) *new*
projectStateVersion: 1
projectProgramVersion: 1
autoImportProviderHost: false

ScriptInfos::
/index.ts (Open) *new*
Expand All @@ -250,6 +251,12 @@ Before getAutoImportProvider
{ "dependencies": { "@angular/forms": "*", "@angular/core": "*" } }


Projects::
/tsconfig.json (Configured) *changed*
projectStateVersion: 1
projectProgramVersion: 1
autoImportProviderHost: undefined *changed*

Info seq [hh:mm:ss:mss] AutoImportProviderProject: found 1 root files in 1 dependencies 0 referenced projects in * ms
Info seq [hh:mm:ss:mss] DirectoryWatcher:: Added:: WatchInfo: /node_modules 1 undefined WatchType: node_modules for closed script infos and package.jsons affecting module specifier cache
Info seq [hh:mm:ss:mss] Elapsed:: *ms DirectoryWatcher:: Added:: WatchInfo: /node_modules 1 undefined WatchType: node_modules for closed script infos and package.jsons affecting module specifier cache
Expand Down
11 changes: 11 additions & 0 deletions tests/baselines/reference/tsserver/autoImportProvider/Responds-to-package_json-changes.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -234,6 +234,7 @@ Projects::
/tsconfig.json (Configured) *new*
projectStateVersion: 1
projectProgramVersion: 1
autoImportProviderHost: false

ScriptInfos::
/index.ts (Open) *new*
Expand All @@ -250,6 +251,16 @@ Before getAutoImportProvider
{ "dependencies": { "@angular/forms": "*", "@angular/core": "*" } }


Projects::
/tsconfig.json (Configured) *changed*
projectStateVersion: 1
projectProgramVersion: 1
autoImportProviderHost: undefined *changed*

Before running Timeout callback:: count: 0

After running Timeout callback:: count: 0

Info seq [hh:mm:ss:mss] AutoImportProviderProject: found 1 root files in 1 dependencies 0 referenced projects in * ms
Info seq [hh:mm:ss:mss] DirectoryWatcher:: Added:: WatchInfo: /node_modules 1 undefined WatchType: node_modules for closed script infos and package.jsons affecting module specifier cache
Info seq [hh:mm:ss:mss] Elapsed:: *ms DirectoryWatcher:: Added:: WatchInfo: /node_modules 1 undefined WatchType: node_modules for closed script infos and package.jsons affecting module specifier cache
Expand Down
1 change: 1 addition & 0 deletions ...selines/reference/tsserver/autoImportProvider/dependencies-are-already-in-main-program.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -244,6 +244,7 @@ Projects::
/tsconfig.json (Configured) *new*
projectStateVersion: 1
projectProgramVersion: 1
autoImportProviderHost: false

ScriptInfos::
/index.ts (Open) *new*
Expand Down
1 change: 1 addition & 0 deletions tests/baselines/reference/tsserver/autoImportProvider/without-dependencies-listed.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -234,6 +234,7 @@ Projects::
/tsconfig.json (Configured) *new*
projectStateVersion: 1
projectProgramVersion: 1
autoImportProviderHost: false

ScriptInfos::
/index.ts (Open) *new*
Expand Down
5 changes: 5 additions & 0 deletions ...elines/reference/tsserver/auxiliaryProject/does-not-remove-scrips-from-InferredProject.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ Projects::
/dev/null/inferredProject1* (Inferred) *new*
projectStateVersion: 1
projectProgramVersion: 1
autoImportProviderHost: false

ScriptInfos::
/a.ts (Open) *new*
Expand Down Expand Up @@ -157,6 +158,7 @@ Projects::
projectProgramVersion: 1
documentPositionMappers: 1 *changed*
/b.d.ts: identitySourceMapConsumer *new*
autoImportProviderHost: false
noDtsResolutionProject: /dev/null/auxiliaryProject1* *changed*

ScriptInfos::
Expand Down Expand Up @@ -221,6 +223,7 @@ Projects::
/dev/null/inferredProject1* (Inferred)
projectStateVersion: 1
projectProgramVersion: 1
autoImportProviderHost: false
noDtsResolutionProject: /dev/null/auxiliaryProject1*
/dev/null/inferredProject2* (Inferred) *new*
projectStateVersion: 1
Expand Down Expand Up @@ -406,7 +409,9 @@ Projects::
/dev/null/inferredProject1* (Inferred)
projectStateVersion: 1
projectProgramVersion: 1
autoImportProviderHost: false
noDtsResolutionProject: /dev/null/auxiliaryProject1*
/dev/null/inferredProject2* (Inferred) *changed*
projectStateVersion: 1
projectProgramVersion: 1 *changed*
autoImportProviderHost: false *changed*
3 changes: 3 additions & 0 deletions ...nes/reference/tsserver/auxiliaryProject/file-is-added-later-through-finding-definition.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,7 @@ Projects::
/dev/null/inferredProject1* (Inferred) *new*
projectStateVersion: 1
projectProgramVersion: 1
autoImportProviderHost: false

ScriptInfos::
/a/lib/lib.d.ts *new*
Expand Down Expand Up @@ -277,6 +278,7 @@ Projects::
projectProgramVersion: 1
documentPositionMappers: 1 *changed*
/user/users/projects/myproject/node_modules/@types/yargs/callback.d.ts: identitySourceMapConsumer *new*
autoImportProviderHost: false
noDtsResolutionProject: /dev/null/auxiliaryProject1* *changed*

ScriptInfos::
Expand Down Expand Up @@ -398,6 +400,7 @@ Projects::
documentPositionMappers: 2 *changed*
/user/users/projects/myproject/node_modules/@types/yargs/callback.d.ts: identitySourceMapConsumer
/user/users/projects/myproject/node_modules/@types/yargs/index.d.ts: identitySourceMapConsumer *new*
autoImportProviderHost: false
noDtsResolutionProject: /dev/null/auxiliaryProject1*

ScriptInfos::
Expand Down
2 changes: 2 additions & 0 deletions ...selines/reference/tsserver/auxiliaryProject/resolution-is-reused-from-different-folder.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,7 @@ Projects::
/dev/null/inferredProject1* (Inferred) *new*
projectStateVersion: 1
projectProgramVersion: 1
autoImportProviderHost: false

ScriptInfos::
/a/lib/lib.d.ts *new*
Expand Down Expand Up @@ -316,6 +317,7 @@ Projects::
projectProgramVersion: 1
documentPositionMappers: 1 *changed*
/user/users/projects/myproject/node_modules/@types/yargs/callback.d.ts: identitySourceMapConsumer *new*
autoImportProviderHost: false
noDtsResolutionProject: /dev/null/auxiliaryProject1* *changed*

ScriptInfos::
Expand Down
3 changes: 3 additions & 0 deletions ...ileSystemInformation/includes-the-parent-folder-FLLs-in-Classic-module-resolution-mode.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,7 @@ Projects::
/users/username/projects/proj/tsconfig.json (Configured) *new*
projectStateVersion: 1
projectProgramVersion: 1
autoImportProviderHost: false

ScriptInfos::
/a/lib/lib.d.ts *new*
Expand Down Expand Up @@ -294,6 +295,7 @@ Projects::
projectStateVersion: 2 *changed*
projectProgramVersion: 1
dirty: true *changed*
autoImportProviderHost: false

Before running Timeout callback:: count: 2
5: /users/username/projects/proj/tsconfig.json
Expand Down Expand Up @@ -397,6 +399,7 @@ Projects::
projectStateVersion: 2
projectProgramVersion: 2 *changed*
dirty: false *changed*
autoImportProviderHost: undefined *changed*

ScriptInfos::
/a/lib/lib.d.ts
Expand Down
Loading