Skip to content

Expand auto-import to all package.json dependencies #38923

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

Merged
merged 63 commits into from
Jun 22, 2020
Merged

Expand auto-import to all package.json dependencies #38923

Changes from 1 commit
Commits
Show all changes
63 commits
Select commit Hold shift + click to select a range
f67fb5b
Start experiment
andrewbranch May 12, 2020
8e128d8
Add logging
andrewbranch May 13, 2020
23f72c7
Go back to a single program
andrewbranch May 14, 2020
913c380
Fix forEachExternalModuleToImportFrom
andrewbranch May 18, 2020
c1d1721
Move auxiliary program to language service
andrewbranch May 19, 2020
feb719a
Add logging
andrewbranch May 19, 2020
3bad9ee
Don’t use resolution cache
andrewbranch May 20, 2020
d0c2d50
Fix(?) containingProjects for ScriptInfo in auxiliary program
andrewbranch May 21, 2020
3554044
Fix ScriptInfo project inclusion
andrewbranch May 26, 2020
3d03027
Add test for default project of auto-importable ScriptInfo
andrewbranch May 26, 2020
bba64e2
Add fourslash server test
andrewbranch May 26, 2020
304b8a3
Don’t create auto import provider inside node_modules
andrewbranch May 26, 2020
4fed93d
Add monorepo-like test
andrewbranch May 26, 2020
958adc0
WIP
andrewbranch May 28, 2020
1ff2db5
Naively ensure autoImportProvider is up to date after package.json ch…
andrewbranch May 29, 2020
af64cd2
Start limiting when auto update provider gets updated
andrewbranch Jun 1, 2020
3d07e97
Respond to changes in node_modules
andrewbranch Jun 2, 2020
936763a
Don’t create auto-import provider until a file is open that would use it
andrewbranch Jun 3, 2020
c0066b3
Merge branch 'master' into experiment/auto-import/program-per-package…
andrewbranch Jun 3, 2020
86151e1
Clean up naming, @internal marking, and fix empty project creation bug
andrewbranch Jun 3, 2020
9c946d6
Drop devDependencies, include peerDependencies
andrewbranch Jun 5, 2020
d3c6785
Add additional compiler options
andrewbranch Jun 5, 2020
9882aeb
Merge branch 'master' into experiment/auto-import/program-per-package…
andrewbranch Jun 5, 2020
6b0441e
Fix interaction with importSuggestionsCache
andrewbranch Jun 5, 2020
40d6363
Move option to UserPreferences, allow inclusion of devDependencies
andrewbranch Jun 8, 2020
d4800ab
Don’t filter out peerDependencies
andrewbranch Jun 9, 2020
dd7240e
Watch unparseable package.jsons
andrewbranch Jun 9, 2020
8823473
But don’t filter packages out due to an invalid package.json
andrewbranch Jun 9, 2020
00b57e3
Update test
andrewbranch Jun 9, 2020
242127d
Merge branch 'master' into experiment/auto-import/program-per-package…
andrewbranch Jun 9, 2020
8d59f50
Don’t use autoImportProvider in codefixes where it can never be used …
andrewbranch Jun 9, 2020
0d4fff9
Add CompletionEntry property for telemetry
andrewbranch Jun 11, 2020
e9c7f6a
Add assertion for isPackageJsonImport to fourslash
andrewbranch Jun 11, 2020
6448144
Fix missing pushSymbol argument
andrewbranch Jun 11, 2020
955edbc
Add isPackageJsonImport to tests and API baselines
andrewbranch Jun 11, 2020
fb8fb38
Fix unit test
andrewbranch Jun 11, 2020
75f12f5
Host auto import provider in new Project kind
andrewbranch Jun 16, 2020
327efa9
Fix InferredProject attaching on AutoImportProvider-included files, l…
andrewbranch Jun 16, 2020
f638d46
Update Public APIs
andrewbranch Jun 16, 2020
b788b05
Simplify PackageJsonCache host
andrewbranch Jun 16, 2020
01a0e44
Merge branch 'master' into experiment/auto-import/program-per-package…
andrewbranch Jun 18, 2020
b6ac999
Remove unneeded markAsDirty
andrewbranch Jun 18, 2020
6e6a71d
Defer project finished event until after AutoImportProvider is created
andrewbranch Jun 18, 2020
c11a701
Make AutoImportProviderProject always report isOrphan = true
andrewbranch Jun 18, 2020
4dc2ed2
Close and remove AutoImportProviderProject when host project closes
andrewbranch Jun 18, 2020
e360fa1
Don’t set pendingEnsureProjectForOpenFiles
andrewbranch Jun 18, 2020
0c05e3a
Use hasAddedOrRemovedFiles instead of hasNewProgram
andrewbranch Jun 18, 2020
e480830
Use host-wide watchOptions for package.json watching
andrewbranch Jun 18, 2020
0cf3cf6
Add to `printProjects`
andrewbranch Jun 18, 2020
83fc465
Clean up
andrewbranch Jun 18, 2020
00e1707
Get autoImportProvider directly from LanguageServiceHost
andrewbranch Jun 18, 2020
737f2ed
Clean up
andrewbranch Jun 18, 2020
f957bd4
Clean up
andrewbranch Jun 22, 2020
8f0ad77
Close auto import provider on disableLanguageService
andrewbranch Jun 22, 2020
f7b6946
Move AutoImportProvider preload to project updateGraph
andrewbranch Jun 22, 2020
e9557ff
Clear auto import suggestion cache when provider program changes
andrewbranch Jun 22, 2020
bd15ac4
Merge branch 'master' into experiment/auto-import/program-per-package…
andrewbranch Jun 22, 2020
3f1821e
Fix tests
andrewbranch Jun 22, 2020
b2d8e34
Revert yet-unneeded change
andrewbranch Jun 22, 2020
4a667e3
Use projectService host for module resolution host
andrewbranch Jun 22, 2020
3ed641d
Don’t re-resolve type directives if nothing has changed
andrewbranch Jun 22, 2020
7b1aaf4
Update src/server/project.ts
andrewbranch Jun 22, 2020
85c6938
Use ts.emptyArray
andrewbranch Jun 22, 2020
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
18 changes: 7 additions & 11 deletions src/server/project.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1634,7 +1634,7 @@ namespace ts.server {

const dependencySelection = this.includePackageJsonAutoImports();
if (dependencySelection) {
this.autoImportProviderHost = AutoImportProviderProject.create(dependencySelection, this, this.documentRegistry);
this.autoImportProviderHost = AutoImportProviderProject.create(dependencySelection, this, this.projectService.host, this.documentRegistry);
if (this.autoImportProviderHost) {
updateProjectIfDirty(this.autoImportProviderHost);
return this.autoImportProviderHost.getCurrentProgram();
Expand Down Expand Up @@ -1793,7 +1793,7 @@ namespace ts.server {
private static readonly newName = createProjectNameFactoryWithCounter(makeAutoImportProviderProjectName);

/*@internal*/
static getRootFileNames(dependencySelection: PackageJsonAutoImportPreference, hostProject: Project, compilerOptions: CompilerOptions): string[] {
static getRootFileNames(dependencySelection: PackageJsonAutoImportPreference, hostProject: Project, moduleResolutionHost: ModuleResolutionHost, compilerOptions: CompilerOptions): string[] {
if (!dependencySelection) {
return ts.emptyArray;
}
Expand All @@ -1815,7 +1815,7 @@ namespace ts.server {
name,
rootFileName,
compilerOptions,
hostProject));
moduleResolutionHost));
Copy link
Member

Choose a reason for hiding this comment

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

You dont need to pass moduleResolutionHost here but instead you want this to also handle trace (if logging enabled going to log) and currentDIrectory
ModuleResolutionHost = hostProject.projectService ?

Copy link
Member Author

Choose a reason for hiding this comment

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

I don’t think I understand the suggestion—ProjectService isn’t assignable to ModuleResolutionHost.

Copy link
Member Author

Choose a reason for hiding this comment

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

Merging for the beta, feel free to follow up with me

Copy link
Member

Choose a reason for hiding this comment

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

You want it to be {
boundMethods from hostProject.projectService.host
getCurrentDirectory : () => hostProject.getCurrentDirectory(),
trace: mayBeBind(hostProject, hostProject.trace)
}

Copy link
Member Author

Choose a reason for hiding this comment

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

Is there a realistic example of why hostProject itself wouldn’t work? If it wants to mimic the existence of some files, why shouldn’t it be allowed to do that during type directive resolution?


for (const resolution of resolutions) {
if (resolution.resolvedTypeReferenceDirective?.resolvedFileName && !hostProject.getCurrentProgram()!.getSourceFile(resolution.resolvedTypeReferenceDirective.resolvedFileName)) {
Expand All @@ -1834,7 +1834,7 @@ namespace ts.server {
}

/*@internal*/
static create(dependencySelection: PackageJsonAutoImportPreference, hostProject: Project, documentRegistry: DocumentRegistry): AutoImportProviderProject | undefined {
static create(dependencySelection: PackageJsonAutoImportPreference, hostProject: Project, moduleResolutionHost: ModuleResolutionHost, documentRegistry: DocumentRegistry): AutoImportProviderProject | undefined {
if (dependencySelection === PackageJsonAutoImportPreference.None) {
return undefined;
}
Expand All @@ -1849,7 +1849,7 @@ namespace ts.server {
sourceMap: false
};

const rootNames = this.getRootFileNames(dependencySelection, hostProject, compilerOptions);
const rootNames = this.getRootFileNames(dependencySelection, hostProject, moduleResolutionHost, compilerOptions);
if (!rootNames.length) {
return undefined;
}
Expand Down Expand Up @@ -1891,6 +1891,7 @@ namespace ts.server {
rootFileNames = AutoImportProviderProject.getRootFileNames(
this.hostProject.includePackageJsonAutoImports(),
this.hostProject,
this.projectService.host,
this.getCompilationSettings());
}

Expand Down Expand Up @@ -2062,14 +2063,9 @@ namespace ts.server {
result = super.updateGraph();
}
this.compilerHost = undefined;
this.onFinishedLoading();
return result;
}

/*@internal*/
onFinishedLoading() {
this.projectService.sendProjectLoadingFinishEvent(this);
this.projectService.sendProjectTelemetry(this);
return result;
}

/*@internal*/
Expand Down