Skip to content

Commit 7b0df1f

Browse files
sheetalkamatsheetalkamat
authored
Pass in information for the module name resolution when resolutions from file are partially used (#49738)
* Test showing wrong resolution is returned because of incorrect mode calculation Test for #48229 * Pass in information for the module name resolution when resolutions from file are partially used Fixes #48229 * Make the resolution info complete
1 parent fa4b49d commit 7b0df1f

File tree

14 files changed

+559
-97
lines changed

14 files changed

+559
-97
lines changed

‎src/compiler/moduleNameResolver.ts

+17-8
Original file line numberDiff line numberDiff line change
@@ -734,8 +734,9 @@ namespace ts {
734734

735735
/* @internal */
736736
export function createModeAwareCache<T>(): ModeAwareCache<T> {
737-
const underlying = new Map<string, T>();
738-
const memoizedReverseKeys = new Map<string, [specifier: string, mode: ModuleKind.CommonJS | ModuleKind.ESNext | undefined]>();
737+
const underlying = new Map<ModeAwareCacheKey, T>();
738+
type ModeAwareCacheKey = string & { __modeAwareCacheKey: any; };
739+
const memoizedReverseKeys = new Map<ModeAwareCacheKey, [specifier: string, mode: ModuleKind.CommonJS | ModuleKind.ESNext | undefined]>();
739740

740741
const cache: ModeAwareCache<T> = {
741742
get(specifier, mode) {
@@ -765,22 +766,30 @@ namespace ts {
765766
return cache;
766767

767768
function getUnderlyingCacheKey(specifier: string, mode: ModuleKind.CommonJS | ModuleKind.ESNext | undefined) {
768-
const result = mode === undefined ? specifier : `${mode}|${specifier}`;
769+
const result = (mode === undefined ? specifier : `${mode}|${specifier}`) as ModeAwareCacheKey;
769770
memoizedReverseKeys.set(result, [specifier, mode]);
770771
return result;
771772
}
772773
}
773774

774775
/* @internal */
775-
export function zipToModeAwareCache<V>(file: SourceFile, keys: readonly string[] | readonly FileReference[], values: readonly V[]): ModeAwareCache<V> {
776+
export function getResolutionName(entry: FileReference | StringLiteralLike) {
777+
// We lower-case all type references because npm automatically lowercases all packages. See GH#9824.
778+
return isStringLiteralLike(entry) ? entry.text : entry.fileName.toLowerCase();
779+
}
780+
781+
/* @internal */
782+
export function getResolutionMode(entry: FileReference | StringLiteralLike, file: SourceFile) {
783+
return isStringLiteralLike(entry) ? getModeForUsageLocation(file, entry) : entry.resolutionMode || file.impliedNodeFormat;
784+
}
785+
786+
/* @internal */
787+
export function zipToModeAwareCache<V>(file: SourceFile, keys: readonly StringLiteralLike[] | readonly FileReference[], values: readonly V[]): ModeAwareCache<V> {
776788
Debug.assert(keys.length === values.length);
777789
const map = createModeAwareCache<V>();
778790
for (let i = 0; i < keys.length; ++i) {
779791
const entry = keys[i];
780-
// We lower-case all type references because npm automatically lowercases all packages. See GH#9824.
781-
const name = !isString(entry) ? entry.fileName.toLowerCase() : entry;
782-
const mode = !isString(entry) ? entry.resolutionMode || file.impliedNodeFormat : getModeForResolutionAtIndex(file, i);
783-
map.set(name, mode, values[i]);
792+
map.set(getResolutionName(entry), getResolutionMode(entry, file), values[i]);
784793
}
785794
return map;
786795
}

‎src/compiler/program.ts

+65-50
Large diffs are not rendered by default.

‎src/compiler/resolutionCache.ts

+41-14
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,14 @@ namespace ts {
55
startRecordingFilesWithChangedResolutions(): void;
66
finishRecordingFilesWithChangedResolutions(): Path[] | undefined;
77

8-
resolveModuleNames(moduleNames: string[], containingFile: string, reusedNames: string[] | undefined, redirectedReference?: ResolvedProjectReference, containingSourceFile?: SourceFile): (ResolvedModuleFull | undefined)[];
8+
resolveModuleNames(
9+
moduleNames: string[],
10+
containingFile: string,
11+
reusedNames: string[] | undefined,
12+
redirectedReference: ResolvedProjectReference | undefined,
13+
containingSourceFile: SourceFile | undefined,
14+
resolutionInfo: ModuleResolutionInfo | undefined
15+
): (ResolvedModuleFull | undefined)[];
916
getResolvedModuleWithFailedLookupLocationsFromCache(moduleName: string, containingFile: string, resolutionMode?: ModuleKind.CommonJS | ModuleKind.ESNext): CachedResolvedModuleWithFailedLookupLocations | undefined;
1017
resolveTypeReferenceDirectives(typeDirectiveNames: string[] | readonly FileReference[], containingFile: string, redirectedReference?: ResolvedProjectReference, containingFileMode?: SourceFile["impliedNodeFormat"]): (ResolvedTypeReferenceDirective | undefined)[];
1118

@@ -409,6 +416,7 @@ namespace ts {
409416
getResolutionWithResolvedFileName: GetResolutionWithResolvedFileName<T, R>;
410417
shouldRetryResolution: (t: T) => boolean;
411418
reusedNames?: readonly string[];
419+
resolutionInfo?: ModuleResolutionInfo;
412420
logChanges?: boolean;
413421
containingSourceFile?: SourceFile;
414422
containingSourceFileMode?: SourceFile["impliedNodeFormat"];
@@ -417,7 +425,7 @@ namespace ts {
417425
names, containingFile, redirectedReference,
418426
cache, perDirectoryCacheWithRedirects,
419427
loader, getResolutionWithResolvedFileName,
420-
shouldRetryResolution, reusedNames, logChanges, containingSourceFile, containingSourceFileMode
428+
shouldRetryResolution, reusedNames, resolutionInfo, logChanges, containingSourceFile, containingSourceFileMode
421429
}: ResolveNamesWithLocalCacheInput<T, R>): (R | undefined)[] {
422430
const path = resolutionHost.toPath(containingFile);
423431
const resolutionsInFile = cache.get(path) || cache.set(path, createModeAwareCache()).get(path)!;
@@ -441,15 +449,20 @@ namespace ts {
441449

442450
const seenNamesInFile = createModeAwareCache<true>();
443451
let i = 0;
444-
for (const entry of names) {
445-
const name = isString(entry) ? entry : entry.fileName.toLowerCase();
452+
for (const entry of containingSourceFile && resolutionInfo ? resolutionInfo.names : names) {
453+
const name = !isString(entry) ? getResolutionName(entry) : entry;
446454
// Imports supply a `containingSourceFile` but no `containingSourceFileMode` - it would be redundant
447455
// they require calculating the mode for a given import from it's position in the resolution table, since a given
448456
// import's syntax may override the file's default mode.
449457
// Type references instead supply a `containingSourceFileMode` and a non-string entry which contains
450458
// a default file mode override if applicable.
451-
const mode = !isString(entry) ? getModeForFileReference(entry, containingSourceFileMode) :
452-
containingSourceFile ? getModeForResolutionAtIndex(containingSourceFile, i) : undefined;
459+
const mode = !isString(entry) ?
460+
isStringLiteralLike(entry) ?
461+
getModeForUsageLocation(containingSourceFile!, entry) :
462+
getModeForFileReference(entry, containingSourceFileMode) :
463+
containingSourceFile ?
464+
getModeForResolutionAtIndex(containingSourceFile, i) :
465+
undefined;
453466
i++;
454467
let resolution = resolutionsInFile.get(name, mode);
455468
// Resolution is valid if it is present and not invalidated
@@ -533,13 +546,19 @@ namespace ts {
533546
resolvedModules.push(getResolutionWithResolvedFileName(resolution));
534547
}
535548

536-
// Stop watching and remove the unused name
537-
resolutionsInFile.forEach((resolution, name, mode) => {
538-
if (!seenNamesInFile.has(name, mode) && !contains(reusedNames, name)) {
539-
stopWatchFailedLookupLocationOfResolution(resolution, path, getResolutionWithResolvedFileName);
540-
resolutionsInFile.delete(name, mode);
541-
}
542-
});
549+
if (containingSourceFile && resolutionInfo) {
550+
resolutionInfo.reusedNames?.forEach(literal => seenNamesInFile.set(literal.text, getModeForUsageLocation(containingSourceFile, literal), true));
551+
reusedNames = undefined;
552+
}
553+
if (resolutionsInFile.size() !== seenNamesInFile.size()) {
554+
// Stop watching and remove the unused name
555+
resolutionsInFile.forEach((resolution, name, mode) => {
556+
if (!seenNamesInFile.has(name, mode) && !contains(reusedNames, name)) {
557+
stopWatchFailedLookupLocationOfResolution(resolution, path, getResolutionWithResolvedFileName);
558+
resolutionsInFile.delete(name, mode);
559+
}
560+
});
561+
}
543562

544563
return resolvedModules;
545564

@@ -576,7 +595,14 @@ namespace ts {
576595
});
577596
}
578597

579-
function resolveModuleNames(moduleNames: string[], containingFile: string, reusedNames: string[] | undefined, redirectedReference?: ResolvedProjectReference, containingSourceFile?: SourceFile): (ResolvedModuleFull | undefined)[] {
598+
function resolveModuleNames(
599+
moduleNames: string[],
600+
containingFile: string,
601+
reusedNames: string[] | undefined,
602+
redirectedReference?: ResolvedProjectReference,
603+
containingSourceFile?: SourceFile,
604+
resolutionInfo?: ModuleResolutionInfo
605+
): (ResolvedModuleFull | undefined)[] {
580606
return resolveNamesWithLocalCache<CachedResolvedModuleWithFailedLookupLocations, ResolvedModuleFull>({
581607
names: moduleNames,
582608
containingFile,
@@ -587,6 +613,7 @@ namespace ts {
587613
getResolutionWithResolvedFileName: getResolvedModule,
588614
shouldRetryResolution: resolution => !resolution.resolvedModule || !resolutionExtensionIsTSOrJson(resolution.resolvedModule.extension),
589615
reusedNames,
616+
resolutionInfo,
590617
logChanges: logChangesWhenResolvingModule,
591618
containingSourceFile,
592619
});

‎src/compiler/tsbuildPublic.ts

+4-3
Original file line numberDiff line numberDiff line change
@@ -306,9 +306,10 @@ namespace ts {
306306
const moduleResolutionCache = !compilerHost.resolveModuleNames ? createModuleResolutionCache(currentDirectory, getCanonicalFileName) : undefined;
307307
const typeReferenceDirectiveResolutionCache = !compilerHost.resolveTypeReferenceDirectives ? createTypeReferenceDirectiveResolutionCache(currentDirectory, getCanonicalFileName, /*options*/ undefined, moduleResolutionCache?.getPackageJsonInfoCache()) : undefined;
308308
if (!compilerHost.resolveModuleNames) {
309-
const loader = (moduleName: string, resolverMode: ModuleKind.CommonJS | ModuleKind.ESNext | undefined, containingFile: string, redirectedReference: ResolvedProjectReference | undefined) => resolveModuleName(moduleName, containingFile, state.projectCompilerOptions, compilerHost, moduleResolutionCache, redirectedReference, resolverMode).resolvedModule!;
310-
compilerHost.resolveModuleNames = (moduleNames, containingFile, _reusedNames, redirectedReference, _options, containingSourceFile) =>
311-
loadWithModeAwareCache<ResolvedModuleFull>(Debug.checkEachDefined(moduleNames), Debug.checkDefined(containingSourceFile), containingFile, redirectedReference, loader);
309+
const loader = (moduleName: string, resolverMode: ModuleKind.CommonJS | ModuleKind.ESNext | undefined, containingFile: string, redirectedReference: ResolvedProjectReference | undefined) =>
310+
resolveModuleName(moduleName, containingFile, state.projectCompilerOptions, compilerHost, moduleResolutionCache, redirectedReference, resolverMode).resolvedModule;
311+
compilerHost.resolveModuleNames = (moduleNames, containingFile, _reusedNames, redirectedReference, _options, containingSourceFile, resolutionInfo) =>
312+
loadWithModeAwareCache(Debug.checkEachDefined(moduleNames), Debug.checkDefined(containingSourceFile), containingFile, redirectedReference, resolutionInfo, loader);
312313
compilerHost.getModuleResolutionCache = () => moduleResolutionCache;
313314
}
314315
if (!compilerHost.resolveTypeReferenceDirectives) {

‎src/compiler/types.ts

+6-1
Original file line numberDiff line numberDiff line change
@@ -7193,6 +7193,11 @@ namespace ts {
71937193
/* @internal */
71947194
export type HasChangedAutomaticTypeDirectiveNames = () => boolean;
71957195

7196+
export interface ModuleResolutionInfo {
7197+
names: readonly StringLiteralLike[];
7198+
reusedNames: readonly StringLiteralLike[] | undefined;
7199+
}
7200+
71967201
export interface CompilerHost extends ModuleResolutionHost {
71977202
getSourceFile(fileName: string, languageVersionOrOptions: ScriptTarget | CreateSourceFileOptions, onError?: (message: string) => void, shouldCreateNewSourceFile?: boolean): SourceFile | undefined;
71987203
getSourceFileByPath?(fileName: string, path: Path, languageVersionOrOptions: ScriptTarget | CreateSourceFileOptions, onError?: (message: string) => void, shouldCreateNewSourceFile?: boolean): SourceFile | undefined;
@@ -7213,7 +7218,7 @@ namespace ts {
72137218
* If resolveModuleNames is implemented then implementation for members from ModuleResolutionHost can be just
72147219
* 'throw new Error("NotImplemented")'
72157220
*/
7216-
resolveModuleNames?(moduleNames: string[], containingFile: string, reusedNames: string[] | undefined, redirectedReference: ResolvedProjectReference | undefined, options: CompilerOptions, containingSourceFile?: SourceFile): (ResolvedModule | undefined)[];
7221+
resolveModuleNames?(moduleNames: string[], containingFile: string, reusedNames: string[] | undefined, redirectedReference: ResolvedProjectReference | undefined, options: CompilerOptions, containingSourceFile?: SourceFile, resolutionInfo?: ModuleResolutionInfo): (ResolvedModule | undefined)[];
72177222
/**
72187223
* Returns the module resolution cache used by a provided `resolveModuleNames` implementation so that any non-name module resolution operations (eg, package.json lookup) can reuse it
72197224
*/

‎src/compiler/utilities.ts

+4-5
Original file line numberDiff line numberDiff line change
@@ -211,19 +211,18 @@ namespace ts {
211211
}
212212

213213
export function hasChangesInResolutions<T>(
214-
names: readonly string[] | readonly FileReference[],
214+
names: readonly StringLiteralLike[] | readonly FileReference[],
215+
newSourceFile: SourceFile,
215216
newResolutions: readonly T[],
216217
oldResolutions: ModeAwareCache<T> | undefined,
217-
oldSourceFile: SourceFile | undefined,
218218
comparer: (oldResolution: T, newResolution: T) => boolean): boolean {
219219
Debug.assert(names.length === newResolutions.length);
220220

221221
for (let i = 0; i < names.length; i++) {
222222
const newResolution = newResolutions[i];
223223
const entry = names[i];
224-
// We lower-case all type references because npm automatically lowercases all packages. See GH#9824.
225-
const name = !isString(entry) ? entry.fileName.toLowerCase() : entry;
226-
const mode = !isString(entry) ? getModeForFileReference(entry, oldSourceFile?.impliedNodeFormat) : oldSourceFile && getModeForResolutionAtIndex(oldSourceFile, i);
224+
const name = getResolutionName(entry);
225+
const mode = getResolutionMode(entry, newSourceFile);
227226
const oldResolution = oldResolutions && oldResolutions.get(name, mode);
228227
const changed =
229228
oldResolution

‎src/compiler/utilitiesPublic.ts

+2-2
Original file line numberDiff line numberDiff line change
@@ -2072,8 +2072,8 @@ namespace ts {
20722072
return indentation === MAX_SMI_X86 ? undefined : indentation;
20732073
}
20742074

2075-
export function isStringLiteralLike(node: Node): node is StringLiteralLike {
2076-
return node.kind === SyntaxKind.StringLiteral || node.kind === SyntaxKind.NoSubstitutionTemplateLiteral;
2075+
export function isStringLiteralLike(node: Node | FileReference): node is StringLiteralLike {
2076+
return (node as Node).kind === SyntaxKind.StringLiteral || (node as Node).kind === SyntaxKind.NoSubstitutionTemplateLiteral;
20772077
}
20782078

20792079
export function isJSDocLinkLike(node: Node): node is JSDocLink | JSDocLinkCode | JSDocLinkPlain {

‎src/compiler/watchPublic.ts

+2-2
Original file line numberDiff line numberDiff line change
@@ -109,7 +109,7 @@ namespace ts {
109109
getEnvironmentVariable?(name: string): string | undefined;
110110

111111
/** If provided, used to resolve the module names, otherwise typescript's default module resolution */
112-
resolveModuleNames?(moduleNames: string[], containingFile: string, reusedNames: string[] | undefined, redirectedReference: ResolvedProjectReference | undefined, options: CompilerOptions, containingSourceFile?: SourceFile): (ResolvedModule | undefined)[];
112+
resolveModuleNames?(moduleNames: string[], containingFile: string, reusedNames: string[] | undefined, redirectedReference: ResolvedProjectReference | undefined, options: CompilerOptions, containingSourceFile?: SourceFile, resolutionInfo?: ModuleResolutionInfo): (ResolvedModule | undefined)[];
113113
/** If provided, used to resolve type reference directives, otherwise typescript's default resolution */
114114
resolveTypeReferenceDirectives?(typeReferenceDirectiveNames: string[] | readonly FileReference[], containingFile: string, redirectedReference: ResolvedProjectReference | undefined, options: CompilerOptions, containingFileMode?: SourceFile["impliedNodeFormat"] | undefined): (ResolvedTypeReferenceDirective | undefined)[];
115115
/** If provided along with custom resolveModuleNames or resolveTypeReferenceDirectives, used to determine if unchanged file path needs to re-resolve modules/type reference directives */
@@ -365,7 +365,7 @@ namespace ts {
365365
// Resolve module using host module resolution strategy if provided otherwise use resolution cache to resolve module names
366366
compilerHost.resolveModuleNames = host.resolveModuleNames ?
367367
((...args) => host.resolveModuleNames!(...args)) :
368-
((moduleNames, containingFile, reusedNames, redirectedReference, _options, sourceFile) => resolutionCache.resolveModuleNames(moduleNames, containingFile, reusedNames, redirectedReference, sourceFile));
368+
((moduleNames, containingFile, reusedNames, redirectedReference, _options, sourceFile, resolutionInfo) => resolutionCache.resolveModuleNames(moduleNames, containingFile, reusedNames, redirectedReference, sourceFile, resolutionInfo));
369369
compilerHost.resolveTypeReferenceDirectives = host.resolveTypeReferenceDirectives ?
370370
((...args) => host.resolveTypeReferenceDirectives!(...args)) :
371371
((typeDirectiveNames, containingFile, redirectedReference, _options, containingFileMode) => resolutionCache.resolveTypeReferenceDirectives(typeDirectiveNames, containingFile, redirectedReference, containingFileMode));

‎src/server/project.ts

+2-2
Original file line numberDiff line numberDiff line change
@@ -506,8 +506,8 @@ namespace ts.server {
506506
return !this.isWatchedMissingFile(path) && this.directoryStructureHost.fileExists(file);
507507
}
508508

509-
resolveModuleNames(moduleNames: string[], containingFile: string, reusedNames?: string[], redirectedReference?: ResolvedProjectReference, _options?: CompilerOptions, containingSourceFile?: SourceFile): (ResolvedModuleFull | undefined)[] {
510-
return this.resolutionCache.resolveModuleNames(moduleNames, containingFile, reusedNames, redirectedReference, containingSourceFile);
509+
resolveModuleNames(moduleNames: string[], containingFile: string, reusedNames?: string[], redirectedReference?: ResolvedProjectReference, _options?: CompilerOptions, containingSourceFile?: SourceFile, resolutionInfo?: ModuleResolutionInfo): (ResolvedModuleFull | undefined)[] {
510+
return this.resolutionCache.resolveModuleNames(moduleNames, containingFile, reusedNames, redirectedReference, containingSourceFile, resolutionInfo);
511511
}
512512

513513
getModuleResolutionCache(): ModuleResolutionCache | undefined {

0 commit comments

Comments
 (0)