Skip to content

Commit 476fc62

Browse files
authored
Eagerly resolve module specifiers for auto-import completions in --moduleResolution node12+ (#48752)
* Add failing test * Block auto-import module specifiers including node_modules path * Eagerly resolve module specifiers in completions in nodenext so failures can be filtered * Add completion info flags for telemetry * Update API baseline * Update completions baselines * Fix missed boolean flip * Fix remaining tests
1 parent 717a1be commit 476fc62

26 files changed

+239
-73
lines changed

src/compiler/types.ts

+2-2
Original file line numberDiff line numberDiff line change
@@ -8470,7 +8470,7 @@ namespace ts {
84708470
export interface ResolvedModuleSpecifierInfo {
84718471
modulePaths: readonly ModulePath[] | undefined;
84728472
moduleSpecifiers: readonly string[] | undefined;
8473-
isAutoImportable: boolean | undefined;
8473+
isBlockedByPackageJsonDependencies: boolean | undefined;
84748474
}
84758475

84768476
/* @internal */
@@ -8482,7 +8482,7 @@ namespace ts {
84828482
export interface ModuleSpecifierCache {
84838483
get(fromFileName: Path, toFileName: Path, preferences: UserPreferences, options: ModuleSpecifierOptions): Readonly<ResolvedModuleSpecifierInfo> | undefined;
84848484
set(fromFileName: Path, toFileName: Path, preferences: UserPreferences, options: ModuleSpecifierOptions, modulePaths: readonly ModulePath[], moduleSpecifiers: readonly string[]): void;
8485-
setIsAutoImportable(fromFileName: Path, toFileName: Path, preferences: UserPreferences, options: ModuleSpecifierOptions, isAutoImportable: boolean): void;
8485+
setBlockedByPackageJsonDependencies(fromFileName: Path, toFileName: Path, preferences: UserPreferences, options: ModuleSpecifierOptions, isBlockedByPackageJsonDependencies: boolean): void;
84868486
setModulePaths(fromFileName: Path, toFileName: Path, preferences: UserPreferences, options: ModuleSpecifierOptions, modulePaths: readonly ModulePath[]): void;
84878487
clear(): void;
84888488
count(): number;

src/server/moduleSpecifierCache.ts

+7-7
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ namespace ts.server {
1414
return cache.get(toFileName);
1515
},
1616
set(fromFileName, toFileName, preferences, options, modulePaths, moduleSpecifiers) {
17-
ensureCache(fromFileName, preferences, options).set(toFileName, createInfo(modulePaths, moduleSpecifiers, /*isAutoImportable*/ true));
17+
ensureCache(fromFileName, preferences, options).set(toFileName, createInfo(modulePaths, moduleSpecifiers, /*isBlockedByPackageJsonDependencies*/ false));
1818

1919
// If any module specifiers were generated based off paths in node_modules,
2020
// a package.json file in that package was read and is an input to the cached.
@@ -43,17 +43,17 @@ namespace ts.server {
4343
info.modulePaths = modulePaths;
4444
}
4545
else {
46-
cache.set(toFileName, createInfo(modulePaths, /*moduleSpecifiers*/ undefined, /*isAutoImportable*/ undefined));
46+
cache.set(toFileName, createInfo(modulePaths, /*moduleSpecifiers*/ undefined, /*isBlockedByPackageJsonDependencies*/ undefined));
4747
}
4848
},
49-
setIsAutoImportable(fromFileName, toFileName, preferences, options, isAutoImportable) {
49+
setBlockedByPackageJsonDependencies(fromFileName, toFileName, preferences, options, isBlockedByPackageJsonDependencies) {
5050
const cache = ensureCache(fromFileName, preferences, options);
5151
const info = cache.get(toFileName);
5252
if (info) {
53-
info.isAutoImportable = isAutoImportable;
53+
info.isBlockedByPackageJsonDependencies = isBlockedByPackageJsonDependencies;
5454
}
5555
else {
56-
cache.set(toFileName, createInfo(/*modulePaths*/ undefined, /*moduleSpecifiers*/ undefined, isAutoImportable));
56+
cache.set(toFileName, createInfo(/*modulePaths*/ undefined, /*moduleSpecifiers*/ undefined, isBlockedByPackageJsonDependencies));
5757
}
5858
},
5959
clear() {
@@ -87,9 +87,9 @@ namespace ts.server {
8787
function createInfo(
8888
modulePaths: readonly ModulePath[] | undefined,
8989
moduleSpecifiers: readonly string[] | undefined,
90-
isAutoImportable: boolean | undefined,
90+
isBlockedByPackageJsonDependencies: boolean | undefined,
9191
): ResolvedModuleSpecifierInfo {
92-
return { modulePaths, moduleSpecifiers, isAutoImportable };
92+
return { modulePaths, moduleSpecifiers, isBlockedByPackageJsonDependencies };
9393
}
9494
}
9595
}

src/server/protocol.ts

+1
Original file line numberDiff line numberDiff line change
@@ -2403,6 +2403,7 @@ namespace ts.server.protocol {
24032403
}
24042404

24052405
export interface CompletionInfo {
2406+
readonly flags?: number;
24062407
readonly isGlobalCompletion: boolean;
24072408
readonly isMemberCompletion: boolean;
24082409
readonly isNewIdentifierLocation: boolean;

src/services/codefixes/importFixes.ts

+13-12
Original file line numberDiff line numberDiff line change
@@ -644,6 +644,7 @@ namespace ts.codefix {
644644
const compilerOptions = program.getCompilerOptions();
645645
const moduleSpecifierResolutionHost = createModuleSpecifierResolutionHost(program, host);
646646
const getChecker = memoizeOne((isFromPackageJson: boolean) => isFromPackageJson ? host.getPackageJsonAutoImportProvider!()!.getTypeChecker() : program.getTypeChecker());
647+
const rejectNodeModulesRelativePaths = moduleResolutionUsesNodeModules(getEmitModuleResolutionKind(compilerOptions));
647648
const getModuleSpecifiers = fromCacheOnly
648649
? (moduleSymbol: Symbol) => ({ moduleSpecifiers: moduleSpecifiers.tryGetModuleSpecifiersFromCache(moduleSymbol, sourceFile, moduleSpecifierResolutionHost, preferences), computedWithoutCache: false })
649650
: (moduleSymbol: Symbol, checker: TypeChecker) => moduleSpecifiers.getModuleSpecifiersWithCacheInfo(moduleSymbol, checker, compilerOptions, sourceFile, moduleSpecifierResolutionHost, preferences);
@@ -655,19 +656,19 @@ namespace ts.codefix {
655656
const importedSymbolHasValueMeaning = !!(exportInfo.targetFlags & SymbolFlags.Value);
656657
const addAsTypeOnly = getAddAsTypeOnly(isValidTypeOnlyUseSite, /*isForNewImportDeclaration*/ true, exportInfo.symbol, exportInfo.targetFlags, checker, compilerOptions);
657658
computedWithoutCacheCount += computedWithoutCache ? 1 : 0;
658-
return moduleSpecifiers?.map((moduleSpecifier): FixAddNewImport | FixAddJsdocTypeImport =>
659+
return mapDefined(moduleSpecifiers, (moduleSpecifier): FixAddNewImport | FixAddJsdocTypeImport | undefined =>
660+
rejectNodeModulesRelativePaths && pathContainsNodeModules(moduleSpecifier) ? undefined :
659661
// `position` should only be undefined at a missing jsx namespace, in which case we shouldn't be looking for pure types.
660-
!importedSymbolHasValueMeaning && isJs && position !== undefined
661-
? { kind: ImportFixKind.JsdocTypeImport, moduleSpecifier, position, exportInfo, isReExport: i > 0 }
662-
: {
663-
kind: ImportFixKind.AddNew,
664-
moduleSpecifier,
665-
importKind: getImportKind(sourceFile, exportInfo.exportKind, compilerOptions),
666-
useRequire,
667-
addAsTypeOnly,
668-
exportInfo,
669-
isReExport: i > 0,
670-
}
662+
!importedSymbolHasValueMeaning && isJs && position !== undefined ? { kind: ImportFixKind.JsdocTypeImport, moduleSpecifier, position, exportInfo, isReExport: i > 0 } :
663+
{
664+
kind: ImportFixKind.AddNew,
665+
moduleSpecifier,
666+
importKind: getImportKind(sourceFile, exportInfo.exportKind, compilerOptions),
667+
useRequire,
668+
addAsTypeOnly,
669+
exportInfo,
670+
isReExport: i > 0,
671+
}
671672
);
672673
});
673674

0 commit comments

Comments
 (0)