From 6c52f0162dfd54d2ce2aafeba6ae002d3e73403f Mon Sep 17 00:00:00 2001 From: Andrew Branch Date: Thu, 28 Oct 2021 17:24:33 -0700 Subject: [PATCH 1/2] Set hasAddedOrRemovedSymlinks when discovering an existing file by its link --- src/compiler/program.ts | 5 +- src/compiler/resolutionCache.ts | 11 +++ src/server/project.ts | 5 ++ ...mport_addToNamedWithDifferentCacheValue.ts | 87 +++++++++++++++++++ 4 files changed, 106 insertions(+), 2 deletions(-) create mode 100644 tests/cases/fourslash/server/completionsImport_addToNamedWithDifferentCacheValue.ts diff --git a/src/compiler/program.ts b/src/compiler/program.ts index 0847e2dbfc3d7..83d57f669d966 100644 --- a/src/compiler/program.ts +++ b/src/compiler/program.ts @@ -1666,6 +1666,7 @@ namespace ts { if (resolutionsChanged) { structureIsReused = StructureIsReused.SafeModules; newSourceFile.resolvedModules = zipToModeAwareCache(newSourceFile, moduleNames, resolutions); + symlinks } else { newSourceFile.resolvedModules = oldSourceFile.resolvedModules; @@ -1674,8 +1675,8 @@ namespace ts { const typesReferenceDirectives = map(newSourceFile.typeReferenceDirectives, ref => toFileNameLowerCase(ref.fileName)); const typeReferenceResolutions = resolveTypeReferenceDirectiveNamesWorker(typesReferenceDirectives, newSourceFile); // ensure that types resolutions are still correct - const typeReferenceEesolutionsChanged = hasChangesInResolutions(typesReferenceDirectives, typeReferenceResolutions, oldSourceFile.resolvedTypeReferenceDirectiveNames, oldSourceFile, typeDirectiveIsEqualTo); - if (typeReferenceEesolutionsChanged) { + const typeReferenceResolutionsChanged = hasChangesInResolutions(typesReferenceDirectives, typeReferenceResolutions, oldSourceFile.resolvedTypeReferenceDirectiveNames, oldSourceFile, typeDirectiveIsEqualTo); + if (typeReferenceResolutionsChanged) { structureIsReused = StructureIsReused.SafeModules; newSourceFile.resolvedTypeReferenceDirectiveNames = zipToModeAwareCache(newSourceFile, typesReferenceDirectives, typeReferenceResolutions); } diff --git a/src/compiler/resolutionCache.ts b/src/compiler/resolutionCache.ts index 8c09ec3ef721d..bb49968b4b257 100644 --- a/src/compiler/resolutionCache.ts +++ b/src/compiler/resolutionCache.ts @@ -66,6 +66,7 @@ namespace ts { getCurrentProgram(): Program | undefined; fileIsOpen(filePath: Path): boolean; getCompilerHost?(): CompilerHost | undefined; + onDiscoveredSymlink(): void; } interface DirectoryWatchesOfFailedLookup { @@ -431,6 +432,9 @@ namespace ts { else { resolution = loader(name, containingFile, compilerOptions, resolutionHost.getCompilerHost?.() || resolutionHost, redirectedReference, containingSourceFile); perDirectoryResolution.set(name, mode, resolution); + if (resolutionIsSymlink(resolution)) { + resolutionHost.onDiscoveredSymlink(); + } } resolutionsInFile.set(name, mode, resolution); watchFailedLookupLocationsOfExternalModuleResolutions(name, resolution, path, getResolutionWithResolvedFileName); @@ -965,4 +969,11 @@ namespace ts { return dirPath === rootPath || canWatchDirectory(dirPath); } } + + function resolutionIsSymlink(resolution: ResolutionWithFailedLookupLocations) { + return !!( + (resolution as ResolvedModuleWithFailedLookupLocations).resolvedModule?.originalPath || + (resolution as ResolvedTypeReferenceDirectiveWithFailedLookupLocations).resolvedTypeReferenceDirective?.originalPath + ); + } } diff --git a/src/server/project.ts b/src/server/project.ts index 197585ff86b91..dc04d4a5a4df4 100644 --- a/src/server/project.ts +++ b/src/server/project.ts @@ -1031,6 +1031,11 @@ namespace ts.server { } } + /* @internal */ + onDiscoveredSymlink() { + this.hasAddedOrRemovedSymlinks = true; + } + /** * Updates set of files that contribute to this project * @returns: true if set of files in the project stays the same and false - otherwise. diff --git a/tests/cases/fourslash/server/completionsImport_addToNamedWithDifferentCacheValue.ts b/tests/cases/fourslash/server/completionsImport_addToNamedWithDifferentCacheValue.ts new file mode 100644 index 0000000000000..261ba00464b52 --- /dev/null +++ b/tests/cases/fourslash/server/completionsImport_addToNamedWithDifferentCacheValue.ts @@ -0,0 +1,87 @@ +/// + +// Notable lack of package.json referencing `mylib` as dependency here. +// This is not accurate to the original repro, but I think that's a separate +// bug, which, if fixed, would prevent the later crash. + +// @Filename: /tsconfig.json +//// { "compilerOptions": { "module": "commonjs" } } + +// @Filename: /packages/mylib/package.json +//// { "name": "mylib", "version": "1.0.0", "main": "index.js" } + +// @Filename: /packages/mylib/index.ts +//// export * from "./mySubDir"; + +// @Filename: /packages/mylib/mySubDir/index.ts +//// export * from "./myClass"; +//// export * from "./myClass2"; + +// @Filename: /packages/mylib/mySubDir/myClass.ts +//// export class MyClass {} + +// @Filename: /packages/mylib/mySubDir/myClass2.ts +//// export class MyClass2 {} + +// @link: /packages/mylib -> /node_modules/mylib + +// @Filename: /src/index.ts +//// +//// const a = new MyClass/*1*/(); +//// const b = new MyClass2/*2*/(); + +goTo.marker("1"); +format.setOption("newLineCharacter", "\n"); + +verify.completions({ + marker: "1", + includes: [{ + name: "MyClass", + source: "../packages/mylib", + sourceDisplay: "../packages/mylib", + hasAction: true, + sortText: completion.SortText.AutoImportSuggestions, + }], + preferences: { + includeCompletionsForModuleExports: true, + includeInsertTextCompletions: true, + allowIncompleteCompletions: true, + } +}); + +verify.applyCodeActionFromCompletion("1", { + name: "MyClass", + source: "../packages/mylib", + description: `Import 'MyClass' from module "../packages/mylib"`, + data: { + exportName: "MyClass", + fileName: "/packages/mylib/index.ts", + }, + preferences: { + includeCompletionsForModuleExports: true, + includeCompletionsWithInsertText: true, + allowIncompleteCompletions: true, + }, + newFileContent: `import { MyClass } from "../packages/mylib"; + +const a = new MyClass(); +const b = new MyClass2();`, +}); + +edit.replaceLine(0, `import { MyClass } from "mylib";`); + +verify.completions({ + marker: "2", + includes: [{ + name: "MyClass2", + source: "mylib", + sourceDisplay: "mylib", + hasAction: true, + sortText: completion.SortText.AutoImportSuggestions, + }], + preferences: { + includeCompletionsForModuleExports: true, + includeInsertTextCompletions: true, + allowIncompleteCompletions: true, + } +}); From dfa3ba03387823746f5b0216a87514c41ad4d6ee Mon Sep 17 00:00:00 2001 From: Andrew Branch Date: Thu, 28 Oct 2021 17:44:33 -0700 Subject: [PATCH 2/2] Make it optional --- src/compiler/program.ts | 1 - src/compiler/resolutionCache.ts | 4 ++-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/src/compiler/program.ts b/src/compiler/program.ts index 83d57f669d966..ee022e7ae2f54 100644 --- a/src/compiler/program.ts +++ b/src/compiler/program.ts @@ -1666,7 +1666,6 @@ namespace ts { if (resolutionsChanged) { structureIsReused = StructureIsReused.SafeModules; newSourceFile.resolvedModules = zipToModeAwareCache(newSourceFile, moduleNames, resolutions); - symlinks } else { newSourceFile.resolvedModules = oldSourceFile.resolvedModules; diff --git a/src/compiler/resolutionCache.ts b/src/compiler/resolutionCache.ts index bb49968b4b257..60d5e0ba24485 100644 --- a/src/compiler/resolutionCache.ts +++ b/src/compiler/resolutionCache.ts @@ -66,7 +66,7 @@ namespace ts { getCurrentProgram(): Program | undefined; fileIsOpen(filePath: Path): boolean; getCompilerHost?(): CompilerHost | undefined; - onDiscoveredSymlink(): void; + onDiscoveredSymlink?(): void; } interface DirectoryWatchesOfFailedLookup { @@ -432,7 +432,7 @@ namespace ts { else { resolution = loader(name, containingFile, compilerOptions, resolutionHost.getCompilerHost?.() || resolutionHost, redirectedReference, containingSourceFile); perDirectoryResolution.set(name, mode, resolution); - if (resolutionIsSymlink(resolution)) { + if (resolutionHost.onDiscoveredSymlink && resolutionIsSymlink(resolution)) { resolutionHost.onDiscoveredSymlink(); } }