From 95dc77cbb483aae55ded927e1d316140b5c586b5 Mon Sep 17 00:00:00 2001 From: Andrew Branch Date: Fri, 31 Jul 2020 14:16:54 -0700 Subject: [PATCH 1/7] Limit auto import provider project size --- src/server/project.ts | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/server/project.ts b/src/server/project.ts index 16f3178d7f786..e3b946f69b7ad 100644 --- a/src/server/project.ts +++ b/src/server/project.ts @@ -1831,6 +1831,8 @@ namespace ts.server { export class AutoImportProviderProject extends Project { private static readonly newName = createProjectNameFactoryWithCounter(makeAutoImportProviderProjectName); + private static readonly maxDependencies = 10; + /*@internal*/ static getRootFileNames(dependencySelection: PackageJsonAutoImportPreference, hostProject: Project, moduleResolutionHost: ModuleResolutionHost, compilerOptions: CompilerOptions): string[] { if (!dependencySelection) { @@ -1862,6 +1864,10 @@ namespace ts.server { const fileName = moduleResolutionHost.realpath?.(resolvedFileName) || resolvedFileName; if (!hostProject.getCurrentProgram()!.getSourceFile(fileName) && !hostProject.getCurrentProgram()!.getSourceFile(resolvedFileName)) { rootNames = append(rootNames, fileName); + // Avoid creating a large project that would significantly slow down time to editor interactivity + if (rootNames.length > this.maxDependencies) { + return ts.emptyArray; + } } } } From 787ea50675a4dc631037d41e829598d7440f583e Mon Sep 17 00:00:00 2001 From: Andrew Branch Date: Fri, 31 Jul 2020 14:38:31 -0700 Subject: [PATCH 2/7] Add test --- src/server/project.ts | 1 + .../unittests/tsserver/autoImportProvider.ts | 20 +++++++++++++++++++ 2 files changed, 21 insertions(+) diff --git a/src/server/project.ts b/src/server/project.ts index e3b946f69b7ad..8aed8205c190e 100644 --- a/src/server/project.ts +++ b/src/server/project.ts @@ -1831,6 +1831,7 @@ namespace ts.server { export class AutoImportProviderProject extends Project { private static readonly newName = createProjectNameFactoryWithCounter(makeAutoImportProviderProjectName); + /*@internal*/ private static readonly maxDependencies = 10; /*@internal*/ diff --git a/src/testRunner/unittests/tsserver/autoImportProvider.ts b/src/testRunner/unittests/tsserver/autoImportProvider.ts index 031af230c5239..3325f88e958ee 100644 --- a/src/testRunner/unittests/tsserver/autoImportProvider.ts +++ b/src/testRunner/unittests/tsserver/autoImportProvider.ts @@ -229,6 +229,26 @@ namespace ts.projectSystem { host.writeFile(packageJson.path, packageJson.content); assert.ok(projectService.configuredProjects.get(tsconfig.path)!.getLanguageService().getAutoImportProvider()); }); + + it("Does not create an auto import provider if there are too many dependencies", () => { + const createPackage = (i: number): File[] => ([ + { path: `/node_modules/package${i}/package.json`, content: `{ "name": "package${i}" }` }, + { path: `/node_modules/package${i}/index.d.ts`, content: `` } + ]); + + const packages = []; + for (let i = 0; i < 11; i++) { + packages.push(createPackage(i)); + } + + const dependencies = packages.reduce((hash, p) => ({ ...hash, [JSON.parse(p[0].content).name]: "*" }), {}); + const packageJson: File = { path: "/package.json", content: JSON.stringify(dependencies) }; + const { projectService, session } = setup([ ...flatten(packages), indexTs, tsconfig, packageJson ]); + + openFilesForSession([indexTs], session); + const project = projectService.configuredProjects.get(tsconfig.path)!; + assert.isUndefined(project.getPackageJsonAutoImportProvider()); + }); }); describe("unittests:: tsserver:: autoImportProvider - monorepo", () => { From 23d4e0d4ebbf8ed5396a967f7c33566eb2fa76f9 Mon Sep 17 00:00:00 2001 From: Andrew Branch Date: Fri, 31 Jul 2020 15:23:22 -0700 Subject: [PATCH 3/7] Make option configurable --- src/compiler/types.ts | 2 +- src/server/editorServices.ts | 6 +++--- src/server/project.ts | 11 ++++------- src/server/protocol.ts | 2 +- src/services/types.ts | 6 +++--- 5 files changed, 12 insertions(+), 15 deletions(-) diff --git a/src/compiler/types.ts b/src/compiler/types.ts index 30a0dc4e7e952..d9468bd51234e 100644 --- a/src/compiler/types.ts +++ b/src/compiler/types.ts @@ -8058,7 +8058,7 @@ namespace ts { readonly importModuleSpecifierEnding?: "auto" | "minimal" | "index" | "js"; readonly allowTextChangesInNewFiles?: boolean; readonly providePrefixAndSuffixTextForRename?: boolean; - readonly includePackageJsonAutoImports?: "exclude-dev" | "all" | "none"; + readonly includePackageJsonAutoImports?: "auto" | "on" | "off"; readonly provideRefactorNotApplicableReason?: boolean; } diff --git a/src/server/editorServices.ts b/src/server/editorServices.ts index fc79dc7c49159..236751b2b0198 100644 --- a/src/server/editorServices.ts +++ b/src/server/editorServices.ts @@ -3825,9 +3825,9 @@ namespace ts.server { /*@internal*/ includePackageJsonAutoImports(): PackageJsonAutoImportPreference { switch (this.hostConfiguration.preferences.includePackageJsonAutoImports) { - case "none": return PackageJsonAutoImportPreference.None; - case "all": return PackageJsonAutoImportPreference.All; - default: return PackageJsonAutoImportPreference.ExcludeDevDependencies; + case "on": return PackageJsonAutoImportPreference.On; + case "off": return PackageJsonAutoImportPreference.Off; + default: return PackageJsonAutoImportPreference.Auto; } } diff --git a/src/server/project.ts b/src/server/project.ts index 8aed8205c190e..c0cd303d6a900 100644 --- a/src/server/project.ts +++ b/src/server/project.ts @@ -1631,11 +1631,11 @@ namespace ts.server { /*@internal*/ includePackageJsonAutoImports(): PackageJsonAutoImportPreference { - if (this.projectService.includePackageJsonAutoImports() === PackageJsonAutoImportPreference.None || + if (this.projectService.includePackageJsonAutoImports() === PackageJsonAutoImportPreference.Off || !this.languageServiceEnabled || isInsideNodeModules(this.currentDirectory) || !this.isDefaultProjectForOpenFiles()) { - return PackageJsonAutoImportPreference.None; + return PackageJsonAutoImportPreference.Off; } return this.projectService.includePackageJsonAutoImports(); } @@ -1847,9 +1847,6 @@ namespace ts.server { for (const packageJson of packageJsons) { packageJson.dependencies?.forEach((_, dependenyName) => addDependency(dependenyName)); packageJson.peerDependencies?.forEach((_, dependencyName) => addDependency(dependencyName)); - if (dependencySelection === PackageJsonAutoImportPreference.All) { - packageJson.devDependencies?.forEach((_, dependencyName) => addDependency(dependencyName)); - } } if (dependencyNames) { @@ -1884,7 +1881,7 @@ namespace ts.server { /*@internal*/ static create(dependencySelection: PackageJsonAutoImportPreference, hostProject: Project, moduleResolutionHost: ModuleResolutionHost, documentRegistry: DocumentRegistry): AutoImportProviderProject | undefined { - if (dependencySelection === PackageJsonAutoImportPreference.None) { + if (dependencySelection === PackageJsonAutoImportPreference.Off) { return undefined; } @@ -1981,7 +1978,7 @@ namespace ts.server { /*@internal*/ includePackageJsonAutoImports() { - return PackageJsonAutoImportPreference.None; + return PackageJsonAutoImportPreference.Off; } getTypeAcquisition(): TypeAcquisition { diff --git a/src/server/protocol.ts b/src/server/protocol.ts index cdeaad1a4fc22..b288124049e67 100644 --- a/src/server/protocol.ts +++ b/src/server/protocol.ts @@ -3218,7 +3218,7 @@ namespace ts.server.protocol { readonly lazyConfiguredProjectsFromExternalProject?: boolean; readonly providePrefixAndSuffixTextForRename?: boolean; readonly allowRenameOfImportPath?: boolean; - readonly includePackageJsonAutoImports?: "exclude-dev" | "all" | "none"; + readonly includePackageJsonAutoImports?: "auto" | "on" | "off"; } export interface CompilerOptions { diff --git a/src/services/types.ts b/src/services/types.ts index 0ebd80372cbea..41884a729c4e8 100644 --- a/src/services/types.ts +++ b/src/services/types.ts @@ -211,9 +211,9 @@ namespace ts { /* @internal */ export const enum PackageJsonAutoImportPreference { - None, - ExcludeDevDependencies, - All + Off, + On, + Auto, } export interface PerformanceEvent { From c47656c6c21a04bf9b7b97d314bb305ed563490c Mon Sep 17 00:00:00 2001 From: Andrew Branch Date: Fri, 31 Jul 2020 15:42:08 -0700 Subject: [PATCH 4/7] Fix test --- .../unittests/tsserver/cachingFileSystemInformation.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/testRunner/unittests/tsserver/cachingFileSystemInformation.ts b/src/testRunner/unittests/tsserver/cachingFileSystemInformation.ts index c3fcfb5192ce1..b4a6c2f12f20a 100644 --- a/src/testRunner/unittests/tsserver/cachingFileSystemInformation.ts +++ b/src/testRunner/unittests/tsserver/cachingFileSystemInformation.ts @@ -544,7 +544,7 @@ namespace ts.projectSystem { const otherFiles = [packageJson]; const host = createServerHost(projectFiles.concat(otherFiles)); const projectService = createProjectService(host); - projectService.setHostConfiguration({ preferences: { includePackageJsonAutoImports: "none" } }); + projectService.setHostConfiguration({ preferences: { includePackageJsonAutoImports: "off" } }); const { configFileName } = projectService.openClientFile(app.path); assert.equal(configFileName, tsconfigJson.path as server.NormalizedPath, `should find config`); // TODO: GH#18217 const recursiveWatchedDirectories: string[] = [`${appFolder}`, `${appFolder}/node_modules`].concat(getNodeModuleDirectories(getDirectoryPath(appFolder))); From d4483484fbe6685942071c8f3f80d081d655c30b Mon Sep 17 00:00:00 2001 From: Andrew Branch Date: Fri, 31 Jul 2020 15:46:52 -0700 Subject: [PATCH 5/7] Only bail when setting is auto --- src/server/project.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/server/project.ts b/src/server/project.ts index c0cd303d6a900..e868005544229 100644 --- a/src/server/project.ts +++ b/src/server/project.ts @@ -1863,7 +1863,7 @@ namespace ts.server { if (!hostProject.getCurrentProgram()!.getSourceFile(fileName) && !hostProject.getCurrentProgram()!.getSourceFile(resolvedFileName)) { rootNames = append(rootNames, fileName); // Avoid creating a large project that would significantly slow down time to editor interactivity - if (rootNames.length > this.maxDependencies) { + if (dependencySelection === PackageJsonAutoImportPreference.Auto && rootNames.length > this.maxDependencies) { return ts.emptyArray; } } From 8d85576d748384286d27f800f1a25f5dcb94ddcb Mon Sep 17 00:00:00 2001 From: Andrew Branch Date: Fri, 31 Jul 2020 15:53:37 -0700 Subject: [PATCH 6/7] Fix other test --- src/testRunner/unittests/tsserver/typingsInstaller.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/testRunner/unittests/tsserver/typingsInstaller.ts b/src/testRunner/unittests/tsserver/typingsInstaller.ts index 1ca1ce143a75c..e9c77566b70ba 100644 --- a/src/testRunner/unittests/tsserver/typingsInstaller.ts +++ b/src/testRunner/unittests/tsserver/typingsInstaller.ts @@ -130,7 +130,7 @@ namespace ts.projectSystem { })(); const projectService = createProjectService(host, { useSingleInferredProject: true, typingsInstaller: installer }); - projectService.setHostConfiguration({ preferences: { includePackageJsonAutoImports: "none" } }); + projectService.setHostConfiguration({ preferences: { includePackageJsonAutoImports: "off" } }); projectService.openClientFile(file1.path); checkNumberOfProjects(projectService, { configuredProjects: 1 }); From 5abf67c15951cc3c204aab89b28be9fdd59d939c Mon Sep 17 00:00:00 2001 From: Andrew Branch Date: Fri, 31 Jul 2020 16:10:03 -0700 Subject: [PATCH 7/7] Update API baseline --- tests/baselines/reference/api/tsserverlibrary.d.ts | 4 ++-- tests/baselines/reference/api/typescript.d.ts | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/baselines/reference/api/tsserverlibrary.d.ts b/tests/baselines/reference/api/tsserverlibrary.d.ts index 5a803b4c659ed..f3ec0a6b0cec1 100644 --- a/tests/baselines/reference/api/tsserverlibrary.d.ts +++ b/tests/baselines/reference/api/tsserverlibrary.d.ts @@ -3818,7 +3818,7 @@ declare namespace ts { readonly importModuleSpecifierEnding?: "auto" | "minimal" | "index" | "js"; readonly allowTextChangesInNewFiles?: boolean; readonly providePrefixAndSuffixTextForRename?: boolean; - readonly includePackageJsonAutoImports?: "exclude-dev" | "all" | "none"; + readonly includePackageJsonAutoImports?: "auto" | "on" | "off"; readonly provideRefactorNotApplicableReason?: boolean; } /** Represents a bigint literal value without requiring bigint support */ @@ -8897,7 +8897,7 @@ declare namespace ts.server.protocol { readonly lazyConfiguredProjectsFromExternalProject?: boolean; readonly providePrefixAndSuffixTextForRename?: boolean; readonly allowRenameOfImportPath?: boolean; - readonly includePackageJsonAutoImports?: "exclude-dev" | "all" | "none"; + readonly includePackageJsonAutoImports?: "auto" | "on" | "off"; } interface CompilerOptions { allowJs?: boolean; diff --git a/tests/baselines/reference/api/typescript.d.ts b/tests/baselines/reference/api/typescript.d.ts index bb9aa1f2d2537..8863114c90e3f 100644 --- a/tests/baselines/reference/api/typescript.d.ts +++ b/tests/baselines/reference/api/typescript.d.ts @@ -3818,7 +3818,7 @@ declare namespace ts { readonly importModuleSpecifierEnding?: "auto" | "minimal" | "index" | "js"; readonly allowTextChangesInNewFiles?: boolean; readonly providePrefixAndSuffixTextForRename?: boolean; - readonly includePackageJsonAutoImports?: "exclude-dev" | "all" | "none"; + readonly includePackageJsonAutoImports?: "auto" | "on" | "off"; readonly provideRefactorNotApplicableReason?: boolean; } /** Represents a bigint literal value without requiring bigint support */