Skip to content

Commit 454a3a0

Browse files
authored
Fix auto-imports from auto type acquisition definitions (#33766)
* Fix auto-imports from ATA typings * Compare canonical filenames in isImportablePath
1 parent 6d969f7 commit 454a3a0

File tree

7 files changed

+47
-4
lines changed

7 files changed

+47
-4
lines changed

src/compiler/moduleSpecifiers.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -317,9 +317,13 @@ namespace ts.moduleSpecifiers {
317317

318318
// If the module could be imported by a directory name, use that directory's name
319319
const moduleSpecifier = packageNameOnly ? moduleFileName : getDirectoryOrExtensionlessFileName(moduleFileName);
320+
const globalTypingsCacheLocation = host.getGlobalTypingsCacheLocation && host.getGlobalTypingsCacheLocation();
320321
// Get a path that's relative to node_modules or the importing file's path
321322
// if node_modules folder is in this folder or any of its parent folders, no need to keep it.
322-
if (!startsWith(sourceDirectory, getCanonicalFileName(moduleSpecifier.substring(0, parts.topLevelNodeModulesIndex)))) return undefined;
323+
const pathToTopLevelNodeModules = getCanonicalFileName(moduleSpecifier.substring(0, parts.topLevelNodeModulesIndex));
324+
if (!(startsWith(sourceDirectory, pathToTopLevelNodeModules) || globalTypingsCacheLocation && startsWith(getCanonicalFileName(globalTypingsCacheLocation), pathToTopLevelNodeModules))) {
325+
return undefined;
326+
}
323327

324328
// If the module was found in @types, get the actual Node package name
325329
const nodeModulesDirectoryName = moduleSpecifier.substring(parts.topLevelPackageNameIndex + 1);

src/compiler/types.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6170,6 +6170,8 @@ namespace ts {
61706170
readFile?(path: string): string | undefined;
61716171
/* @internal */
61726172
getProbableSymlinks?(files: readonly SourceFile[]): ReadonlyMap<string>;
6173+
/* @internal */
6174+
getGlobalTypingsCacheLocation?(): string | undefined;
61736175
}
61746176

61756177
// Note: this used to be deprecated in our public API, but is still used internally

src/harness/harnessLanguageService.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -220,6 +220,10 @@ namespace Harness.LanguageService {
220220
return !!this.typesRegistry && this.typesRegistry.has(name);
221221
}
222222

223+
getGlobalTypingsCacheLocation() {
224+
return "/Library/Caches/typescript";
225+
}
226+
223227
installPackage = ts.notImplemented;
224228

225229
getCompilationSettings() { return this.settings; }

src/server/project.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -307,6 +307,11 @@ namespace ts.server {
307307
return this.typingsCache.installPackage({ ...options, projectName: this.projectName, projectRootPath: this.toPath(this.currentDirectory) });
308308
}
309309

310+
/*@internal*/
311+
getGlobalTypingsCacheLocation() {
312+
return this.getGlobalCache();
313+
}
314+
310315
private get typingsCache(): TypingsCache {
311316
return this.projectService.typingsCache;
312317
}

src/services/codefixes/importFixes.ts

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -631,6 +631,7 @@ namespace ts.codefix {
631631
let filteredCount = 0;
632632
const packageJson = filterByPackageJson && createAutoImportFilter(from, program, host);
633633
const allSourceFiles = program.getSourceFiles();
634+
const globalTypingsCache = host.getGlobalTypingsCacheLocation && host.getGlobalTypingsCacheLocation();
634635
forEachExternalModule(program.getTypeChecker(), allSourceFiles, (module, sourceFile) => {
635636
if (sourceFile === undefined) {
636637
if (!packageJson || packageJson.allowsImportingAmbientModule(module, allSourceFiles)) {
@@ -640,7 +641,10 @@ namespace ts.codefix {
640641
filteredCount++;
641642
}
642643
}
643-
else if (sourceFile && sourceFile !== from && isImportablePath(from.fileName, sourceFile.fileName)) {
644+
else if (sourceFile &&
645+
sourceFile !== from &&
646+
isImportablePath(from.fileName, sourceFile.fileName, hostGetCanonicalFileName(host), globalTypingsCache)
647+
) {
644648
if (!packageJson || packageJson.allowsImportingSourceFile(sourceFile, allSourceFiles)) {
645649
cb(module);
646650
}
@@ -669,10 +673,13 @@ namespace ts.codefix {
669673
* Don't include something from a `node_modules` that isn't actually reachable by a global import.
670674
* A relative import to node_modules is usually a bad idea.
671675
*/
672-
function isImportablePath(fromPath: string, toPath: string): boolean {
676+
function isImportablePath(fromPath: string, toPath: string, getCanonicalFileName: GetCanonicalFileName, globalCachePath?: string): boolean {
673677
// If it's in a `node_modules` but is not reachable from here via a global import, don't bother.
674678
const toNodeModules = forEachAncestorDirectory(toPath, ancestor => getBaseFileName(ancestor) === "node_modules" ? ancestor : undefined);
675-
return toNodeModules === undefined || startsWith(fromPath, getDirectoryPath(toNodeModules));
679+
const toNodeModulesParent = toNodeModules && getDirectoryPath(getCanonicalFileName(toNodeModules));
680+
return toNodeModulesParent === undefined
681+
|| startsWith(getCanonicalFileName(fromPath), toNodeModulesParent)
682+
|| (!!globalCachePath && startsWith(getCanonicalFileName(globalCachePath), toNodeModulesParent));
676683
}
677684

678685
export function moduleSymbolToValidIdentifier(moduleSymbol: Symbol, target: ScriptTarget): string {
@@ -718,6 +725,7 @@ namespace ts.codefix {
718725
readFile: maybeBind(host, host.readFile),
719726
useCaseSensitiveFileNames: maybeBind(host, host.useCaseSensitiveFileNames),
720727
getProbableSymlinks: maybeBind(host, host.getProbableSymlinks) || program.getProbableSymlinks,
728+
getGlobalTypingsCacheLocation: maybeBind(host, host.getGlobalTypingsCacheLocation),
721729
};
722730

723731
let usesNodeCoreModules: boolean | undefined;

src/services/types.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -243,6 +243,8 @@ namespace ts {
243243
resolveTypeReferenceDirectives?(typeDirectiveNames: string[], containingFile: string, redirectedReference: ResolvedProjectReference | undefined, options: CompilerOptions): (ResolvedTypeReferenceDirective | undefined)[];
244244
/* @internal */ hasInvalidatedResolution?: HasInvalidatedResolution;
245245
/* @internal */ hasChangedAutomaticTypeDirectiveNames?: boolean;
246+
/* @internal */
247+
getGlobalTypingsCacheLocation?(): string | undefined;
246248

247249
/*
248250
* Required for full import and type reference completions.
Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
/// <reference path="fourslash.ts" />
2+
3+
// @Filename: /project/tsconfig.json
4+
//// { "compilerOptions": { "allowJs": true, "checkJs": true } }
5+
6+
// @Filename: /Library/Caches/typescript/node_modules/@types/react-router-dom/package.json
7+
//// { "name": "react-router-dom" }
8+
9+
// @Filename: /Library/Caches/typescript/node_modules/@types/react-router-dom/index.d.ts
10+
////export class BrowserRouter {}
11+
12+
// @Filename: /project/index.js
13+
////BrowserRouter/**/
14+
15+
goTo.file("/project/index.js");
16+
verify.importFixAtPosition([`import { BrowserRouter } from "react-router-dom";
17+
18+
BrowserRouter`]);

0 commit comments

Comments
 (0)