Skip to content

Commit 84ab39d

Browse files
andrewbranchandrewbranch
authored
Respect importModuleSpecifierPreference in sort order between fixes for the same symbol from different files (microsoft#58597)
1 parent ba78807 commit 84ab39d

File tree

9 files changed

+229
-45
lines changed

9 files changed

+229
-45
lines changed

‎src/compiler/moduleSpecifiers.ts‎

Lines changed: 28 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -107,6 +107,7 @@ import {
107107
removeTrailingDirectorySeparator,
108108
replaceFirstStar,
109109
ResolutionMode,
110+
ResolvedModuleSpecifierInfo,
110111
resolveModuleName,
111112
resolvePath,
112113
ScriptKind,
@@ -287,14 +288,15 @@ export function tryGetModuleSpecifiersFromCache(
287288
host: ModuleSpecifierResolutionHost,
288289
userPreferences: UserPreferences,
289290
options: ModuleSpecifierOptions = {},
290-
): readonly string[] | undefined {
291-
return tryGetModuleSpecifiersFromCacheWorker(
291+
): ModuleSpecifierResult | undefined {
292+
const result = tryGetModuleSpecifiersFromCacheWorker(
292293
moduleSymbol,
293294
importingSourceFile,
294295
host,
295296
userPreferences,
296297
options,
297-
)[0];
298+
);
299+
return result[1] && { kind: result[0], moduleSpecifiers: result[1], computedWithoutCache: false };
298300
}
299301

300302
function tryGetModuleSpecifiersFromCacheWorker(
@@ -303,15 +305,15 @@ function tryGetModuleSpecifiersFromCacheWorker(
303305
host: ModuleSpecifierResolutionHost,
304306
userPreferences: UserPreferences,
305307
options: ModuleSpecifierOptions = {},
306-
): readonly [specifiers?: readonly string[], moduleFile?: SourceFile, modulePaths?: readonly ModulePath[], cache?: ModuleSpecifierCache] {
308+
): readonly [kind?: ModuleSpecifierResult["kind"], specifiers?: readonly string[], moduleFile?: SourceFile, modulePaths?: readonly ModulePath[], cache?: ModuleSpecifierCache] {
307309
const moduleSourceFile = getSourceFileOfModule(moduleSymbol);
308310
if (!moduleSourceFile) {
309311
return emptyArray as [];
310312
}
311313

312314
const cache = host.getModuleSpecifierCache?.();
313315
const cached = cache?.get(importingSourceFile.path, moduleSourceFile.path, userPreferences, options);
314-
return [cached?.moduleSpecifiers, moduleSourceFile, cached?.modulePaths, cache];
316+
return [cached?.kind, cached?.moduleSpecifiers, moduleSourceFile, cached?.modulePaths, cache];
315317
}
316318

317319
/**
@@ -340,6 +342,13 @@ export function getModuleSpecifiers(
340342
).moduleSpecifiers;
341343
}
342344

345+
/** @internal */
346+
export interface ModuleSpecifierResult {
347+
kind: ResolvedModuleSpecifierInfo["kind"];
348+
moduleSpecifiers: readonly string[];
349+
computedWithoutCache: boolean;
350+
}
351+
343352
/** @internal */
344353
export function getModuleSpecifiersWithCacheInfo(
345354
moduleSymbol: Symbol,
@@ -350,21 +359,21 @@ export function getModuleSpecifiersWithCacheInfo(
350359
userPreferences: UserPreferences,
351360
options: ModuleSpecifierOptions = {},
352361
forAutoImport: boolean,
353-
): { moduleSpecifiers: readonly string[]; computedWithoutCache: boolean; } {
362+
): ModuleSpecifierResult {
354363
let computedWithoutCache = false;
355364
const ambient = tryGetModuleNameFromAmbientModule(moduleSymbol, checker);
356-
if (ambient) return { moduleSpecifiers: [ambient], computedWithoutCache };
365+
if (ambient) return { kind: "ambient", moduleSpecifiers: [ambient], computedWithoutCache };
357366

358367
// eslint-disable-next-line prefer-const
359-
let [specifiers, moduleSourceFile, modulePaths, cache] = tryGetModuleSpecifiersFromCacheWorker(
368+
let [kind, specifiers, moduleSourceFile, modulePaths, cache] = tryGetModuleSpecifiersFromCacheWorker(
360369
moduleSymbol,
361370
importingSourceFile,
362371
host,
363372
userPreferences,
364373
options,
365374
);
366-
if (specifiers) return { moduleSpecifiers: specifiers, computedWithoutCache };
367-
if (!moduleSourceFile) return { moduleSpecifiers: emptyArray, computedWithoutCache };
375+
if (specifiers) return { kind, moduleSpecifiers: specifiers, computedWithoutCache };
376+
if (!moduleSourceFile) return { kind: undefined, moduleSpecifiers: emptyArray, computedWithoutCache };
368377

369378
computedWithoutCache = true;
370379
modulePaths ||= getAllModulePathsWorker(getInfo(importingSourceFile.fileName, host), moduleSourceFile.originalFileName, host, compilerOptions, options);
@@ -377,8 +386,8 @@ export function getModuleSpecifiersWithCacheInfo(
377386
options,
378387
forAutoImport,
379388
);
380-
cache?.set(importingSourceFile.path, moduleSourceFile.path, userPreferences, options, modulePaths, result);
381-
return { moduleSpecifiers: result, computedWithoutCache };
389+
cache?.set(importingSourceFile.path, moduleSourceFile.path, userPreferences, options, result.kind, modulePaths, result.moduleSpecifiers);
390+
return result;
382391
}
383392

384393
/** @internal */
@@ -409,7 +418,7 @@ function computeModuleSpecifiers(
409418
userPreferences: UserPreferences,
410419
options: ModuleSpecifierOptions = {},
411420
forAutoImport: boolean,
412-
): readonly string[] {
421+
): ModuleSpecifierResult {
413422
const info = getInfo(importingSourceFile.fileName, host);
414423
const preferences = getModuleSpecifierPreferences(userPreferences, host, compilerOptions, importingSourceFile);
415424
const existingSpecifier = isFullSourceFile(importingSourceFile) && forEach(modulePaths, modulePath =>
@@ -432,8 +441,7 @@ function computeModuleSpecifiers(
432441
},
433442
));
434443
if (existingSpecifier) {
435-
const moduleSpecifiers = [existingSpecifier];
436-
return moduleSpecifiers;
444+
return { kind: undefined, moduleSpecifiers: [existingSpecifier], computedWithoutCache: true };
437445
}
438446

439447
const importedFileIsInNodeModules = some(modulePaths, p => p.isInNodeModules);
@@ -455,7 +463,7 @@ function computeModuleSpecifiers(
455463
if (specifier && modulePath.isRedirect) {
456464
// If we got a specifier for a redirect, it was a bare package specifier (e.g. "@foo/bar",
457465
// not "@foo/bar/path/to/file"). No other specifier will be this good, so stop looking.
458-
return nodeModulesSpecifiers!;
466+
return { kind: "node_modules", moduleSpecifiers: nodeModulesSpecifiers!, computedWithoutCache: true };
459467
}
460468

461469
if (!specifier) {
@@ -501,10 +509,10 @@ function computeModuleSpecifiers(
501509
}
502510
}
503511

504-
return pathsSpecifiers?.length ? pathsSpecifiers :
505-
redirectPathsSpecifiers?.length ? redirectPathsSpecifiers :
506-
nodeModulesSpecifiers?.length ? nodeModulesSpecifiers :
507-
Debug.checkDefined(relativeSpecifiers);
512+
return pathsSpecifiers?.length ? { kind: "paths", moduleSpecifiers: pathsSpecifiers, computedWithoutCache: true } :
513+
redirectPathsSpecifiers?.length ? { kind: "redirect", moduleSpecifiers: redirectPathsSpecifiers, computedWithoutCache: true } :
514+
nodeModulesSpecifiers?.length ? { kind: "node_modules", moduleSpecifiers: nodeModulesSpecifiers, computedWithoutCache: true } :
515+
{ kind: "relative", moduleSpecifiers: Debug.checkDefined(relativeSpecifiers), computedWithoutCache: true };
508516
}
509517

510518
interface Info {

‎src/compiler/types.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9774,6 +9774,7 @@ export interface ModulePath {
97749774

97759775
/** @internal */
97769776
export interface ResolvedModuleSpecifierInfo {
9777+
kind: "node_modules" | "paths" | "redirect" | "relative" | "ambient" | undefined;
97779778
modulePaths: readonly ModulePath[] | undefined;
97789779
moduleSpecifiers: readonly string[] | undefined;
97799780
isBlockedByPackageJsonDependencies: boolean | undefined;
@@ -9787,7 +9788,7 @@ export interface ModuleSpecifierOptions {
97879788
/** @internal */
97889789
export interface ModuleSpecifierCache {
97899790
get(fromFileName: Path, toFileName: Path, preferences: UserPreferences, options: ModuleSpecifierOptions): Readonly<ResolvedModuleSpecifierInfo> | undefined;
9790-
set(fromFileName: Path, toFileName: Path, preferences: UserPreferences, options: ModuleSpecifierOptions, modulePaths: readonly ModulePath[], moduleSpecifiers: readonly string[]): void;
9791+
set(fromFileName: Path, toFileName: Path, preferences: UserPreferences, options: ModuleSpecifierOptions, kind: ResolvedModuleSpecifierInfo["kind"], modulePaths: readonly ModulePath[], moduleSpecifiers: readonly string[]): void;
97919792
setBlockedByPackageJsonDependencies(fromFileName: Path, toFileName: Path, preferences: UserPreferences, options: ModuleSpecifierOptions, isBlockedByPackageJsonDependencies: boolean): void;
97929793
setModulePaths(fromFileName: Path, toFileName: Path, preferences: UserPreferences, options: ModuleSpecifierOptions, modulePaths: readonly ModulePath[]): void;
97939794
clear(): void;

‎src/server/moduleSpecifierCache.ts‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -27,8 +27,8 @@ export function createModuleSpecifierCache(host: ModuleSpecifierResolutionCacheH
2727
if (!cache || currentKey !== key(fromFileName, preferences, options)) return undefined;
2828
return cache.get(toFileName);
2929
},
30-
set(fromFileName, toFileName, preferences, options, modulePaths, moduleSpecifiers) {
31-
ensureCache(fromFileName, preferences, options).set(toFileName, createInfo(modulePaths, moduleSpecifiers, /*isBlockedByPackageJsonDependencies*/ false));
30+
set(fromFileName, toFileName, preferences, options, kind, modulePaths, moduleSpecifiers) {
31+
ensureCache(fromFileName, preferences, options).set(toFileName, createInfo(kind, modulePaths, moduleSpecifiers, /*isBlockedByPackageJsonDependencies*/ false));
3232

3333
// If any module specifiers were generated based off paths in node_modules,
3434
// a package.json file in that package was read and is an input to the cached.
@@ -58,7 +58,7 @@ export function createModuleSpecifierCache(host: ModuleSpecifierResolutionCacheH
5858
info.modulePaths = modulePaths;
5959
}
6060
else {
61-
cache.set(toFileName, createInfo(modulePaths, /*moduleSpecifiers*/ undefined, /*isBlockedByPackageJsonDependencies*/ undefined));
61+
cache.set(toFileName, createInfo(/*kind*/ undefined, modulePaths, /*moduleSpecifiers*/ undefined, /*isBlockedByPackageJsonDependencies*/ undefined));
6262
}
6363
},
6464
setBlockedByPackageJsonDependencies(fromFileName, toFileName, preferences, options, isBlockedByPackageJsonDependencies) {
@@ -68,7 +68,7 @@ export function createModuleSpecifierCache(host: ModuleSpecifierResolutionCacheH
6868
info.isBlockedByPackageJsonDependencies = isBlockedByPackageJsonDependencies;
6969
}
7070
else {
71-
cache.set(toFileName, createInfo(/*modulePaths*/ undefined, /*moduleSpecifiers*/ undefined, isBlockedByPackageJsonDependencies));
71+
cache.set(toFileName, createInfo(/*kind*/ undefined, /*modulePaths*/ undefined, /*moduleSpecifiers*/ undefined, isBlockedByPackageJsonDependencies));
7272
}
7373
},
7474
clear() {
@@ -100,10 +100,11 @@ export function createModuleSpecifierCache(host: ModuleSpecifierResolutionCacheH
100100
}
101101

102102
function createInfo(
103+
kind: ResolvedModuleSpecifierInfo["kind"] | undefined,
103104
modulePaths: readonly ModulePath[] | undefined,
104105
moduleSpecifiers: readonly string[] | undefined,
105106
isBlockedByPackageJsonDependencies: boolean | undefined,
106107
): ResolvedModuleSpecifierInfo {
107-
return { modulePaths, moduleSpecifiers, isBlockedByPackageJsonDependencies };
108+
return { kind, modulePaths, moduleSpecifiers, isBlockedByPackageJsonDependencies };
108109
}
109110
}

0 commit comments

Comments
 (0)