Skip to content

Commit 89ce16c

Browse files
authored
Fix auto-import when paths points to project reference redirect (#51492)
* Fix auto-import when `paths` points to project reference redirect * Put paths specifiers to redirects in lower priority bucket
1 parent 3fcd1b5 commit 89ce16c

File tree

4 files changed

+128
-16
lines changed

4 files changed

+128
-16
lines changed

src/compiler/moduleSpecifiers.ts

+40-16
Original file line numberDiff line numberDiff line change
@@ -275,6 +275,7 @@ function computeModuleSpecifiers(
275275
// 4. Relative paths
276276
let nodeModulesSpecifiers: string[] | undefined;
277277
let pathsSpecifiers: string[] | undefined;
278+
let redirectPathsSpecifiers: string[] | undefined;
278279
let relativeSpecifiers: string[] | undefined;
279280
for (const modulePath of modulePaths) {
280281
const specifier = tryGetModuleNameAsNodeModule(modulePath, info, importingSourceFile, host, compilerOptions, userPreferences, /*packageNameOnly*/ undefined, options.overrideImportMode);
@@ -285,9 +286,23 @@ function computeModuleSpecifiers(
285286
return nodeModulesSpecifiers!;
286287
}
287288

288-
if (!specifier && !modulePath.isRedirect) {
289-
const local = getLocalModuleSpecifier(modulePath.path, info, compilerOptions, host, options.overrideImportMode || importingSourceFile.impliedNodeFormat, preferences);
290-
if (pathIsBareSpecifier(local)) {
289+
if (!specifier) {
290+
const local = getLocalModuleSpecifier(
291+
modulePath.path,
292+
info,
293+
compilerOptions,
294+
host,
295+
options.overrideImportMode || importingSourceFile.impliedNodeFormat,
296+
preferences,
297+
/*pathsOnly*/ modulePath.isRedirect,
298+
);
299+
if (!local) {
300+
continue;
301+
}
302+
if (modulePath.isRedirect) {
303+
redirectPathsSpecifiers = append(redirectPathsSpecifiers, local);
304+
}
305+
else if (pathIsBareSpecifier(local)) {
291306
pathsSpecifiers = append(pathsSpecifiers, local);
292307
}
293308
else if (!importedFileIsInNodeModules || modulePath.isInNodeModules) {
@@ -306,6 +321,7 @@ function computeModuleSpecifiers(
306321
}
307322

308323
return pathsSpecifiers?.length ? pathsSpecifiers :
324+
redirectPathsSpecifiers?.length ? redirectPathsSpecifiers :
309325
nodeModulesSpecifiers?.length ? nodeModulesSpecifiers :
310326
Debug.checkDefined(relativeSpecifiers);
311327
}
@@ -322,32 +338,42 @@ function getInfo(importingSourceFileName: Path, host: ModuleSpecifierResolutionH
322338
return { getCanonicalFileName, importingSourceFileName, sourceDirectory };
323339
}
324340

325-
function getLocalModuleSpecifier(moduleFileName: string, info: Info, compilerOptions: CompilerOptions, host: ModuleSpecifierResolutionHost, importMode: ResolutionMode, { ending, relativePreference }: Preferences): string {
341+
function getLocalModuleSpecifier(moduleFileName: string, info: Info, compilerOptions: CompilerOptions, host: ModuleSpecifierResolutionHost, importMode: ResolutionMode, { ending, relativePreference }: Preferences): string;
342+
function getLocalModuleSpecifier(moduleFileName: string, info: Info, compilerOptions: CompilerOptions, host: ModuleSpecifierResolutionHost, importMode: ResolutionMode, { ending, relativePreference }: Preferences, pathsOnly?: boolean): string | undefined;
343+
function getLocalModuleSpecifier(moduleFileName: string, info: Info, compilerOptions: CompilerOptions, host: ModuleSpecifierResolutionHost, importMode: ResolutionMode, { ending, relativePreference }: Preferences, pathsOnly?: boolean): string | undefined {
326344
const { baseUrl, paths, rootDirs } = compilerOptions;
345+
if (pathsOnly && !paths) {
346+
return undefined;
347+
}
348+
327349
const { sourceDirectory, getCanonicalFileName } = info;
328350
const relativePath = rootDirs && tryGetModuleNameFromRootDirs(rootDirs, moduleFileName, sourceDirectory, getCanonicalFileName, ending, compilerOptions) ||
329351
removeExtensionAndIndexPostFix(ensurePathIsNonModuleName(getRelativePathFromDirectory(sourceDirectory, moduleFileName, getCanonicalFileName)), ending, compilerOptions);
330352
if (!baseUrl && !paths || relativePreference === RelativePreference.Relative) {
331-
return relativePath;
353+
return pathsOnly ? undefined : relativePath;
332354
}
333355

334356
const baseDirectory = getNormalizedAbsolutePath(getPathsBasePath(compilerOptions, host) || baseUrl!, host.getCurrentDirectory());
335357
const relativeToBaseUrl = getRelativePathIfInDirectory(moduleFileName, baseDirectory, getCanonicalFileName);
336358
if (!relativeToBaseUrl) {
337-
return relativePath;
359+
return pathsOnly ? undefined : relativePath;
338360
}
339361

340362
const fromPaths = paths && tryGetModuleNameFromPaths(relativeToBaseUrl, paths, getAllowedEndings(ending, compilerOptions, importMode), host, compilerOptions);
341-
const nonRelative = fromPaths === undefined && baseUrl !== undefined ? removeExtensionAndIndexPostFix(relativeToBaseUrl, ending, compilerOptions) : fromPaths;
342-
if (!nonRelative) {
363+
if (pathsOnly) {
364+
return fromPaths;
365+
}
366+
367+
const maybeNonRelative = fromPaths === undefined && baseUrl !== undefined ? removeExtensionAndIndexPostFix(relativeToBaseUrl, ending, compilerOptions) : fromPaths;
368+
if (!maybeNonRelative) {
343369
return relativePath;
344370
}
345371

346-
if (relativePreference === RelativePreference.NonRelative) {
347-
return nonRelative;
372+
if (relativePreference === RelativePreference.NonRelative && !pathIsRelative(maybeNonRelative)) {
373+
return maybeNonRelative;
348374
}
349375

350-
if (relativePreference === RelativePreference.ExternalNonRelative) {
376+
if (relativePreference === RelativePreference.ExternalNonRelative && !pathIsRelative(maybeNonRelative)) {
351377
const projectDirectory = compilerOptions.configFilePath ?
352378
toPath(getDirectoryPath(compilerOptions.configFilePath), host.getCurrentDirectory(), info.getCanonicalFileName) :
353379
info.getCanonicalFileName(host.getCurrentDirectory());
@@ -363,7 +389,7 @@ function getLocalModuleSpecifier(moduleFileName: string, info: Info, compilerOpt
363389
// lib/ | (path crosses tsconfig.json)
364390
// imported.ts <---
365391
//
366-
return nonRelative;
392+
return maybeNonRelative;
367393
}
368394

369395
const nearestTargetPackageJson = getNearestAncestorDirectoryWithPackageJson(host, getDirectoryPath(modulePath));
@@ -378,16 +404,14 @@ function getLocalModuleSpecifier(moduleFileName: string, info: Info, compilerOpt
378404
// package.json |
379405
// component.ts <---
380406
//
381-
return nonRelative;
407+
return maybeNonRelative;
382408
}
383409

384410
return relativePath;
385411
}
386412

387-
if (relativePreference !== RelativePreference.Shortest) Debug.assertNever(relativePreference);
388-
389413
// Prefer a relative import over a baseUrl import if it has fewer components.
390-
return isPathRelativeToParent(nonRelative) || countPathComponents(relativePath) < countPathComponents(nonRelative) ? relativePath : nonRelative;
414+
return isPathRelativeToParent(maybeNonRelative) || countPathComponents(relativePath) < countPathComponents(maybeNonRelative) ? relativePath : maybeNonRelative;
391415
}
392416

393417
/** @internal */

src/harness/fourslashImpl.ts

+4
Original file line numberDiff line numberDiff line change
@@ -3041,6 +3041,10 @@ export class TestState {
30413041
* @param fileName Path to file where error should be retrieved from.
30423042
*/
30433043
private getCodeFixes(fileName: string, errorCode?: number, preferences: ts.UserPreferences = ts.emptyOptions, position?: number): readonly ts.CodeFixAction[] {
3044+
if (this.testType === FourSlashTestType.Server) {
3045+
this.configure(preferences);
3046+
}
3047+
30443048
const diagnosticsForCodeFix = this.getDiagnostics(fileName, /*includeSuggestions*/ true).map(diagnostic => ({
30453049
start: diagnostic.start,
30463050
length: diagnostic.length,
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
/// <reference path="../fourslash.ts" />
2+
3+
// @Filename: /common/tsconfig.json
4+
//// {
5+
//// "compilerOptions": {
6+
//// "module": "commonjs",
7+
//// "outDir": "dist",
8+
//// "composite": true
9+
//// },
10+
//// "include": ["src"]
11+
//// }
12+
13+
// @Filename: /common/src/MyModule.ts
14+
//// export function square(n: number) {
15+
//// return n * 2;
16+
//// }
17+
18+
// @Filename: /web/tsconfig.json
19+
//// {
20+
//// "compilerOptions": {
21+
//// "module": "esnext",
22+
//// "moduleResolution": "node",
23+
//// "noEmit": true,
24+
//// "baseUrl": "."
25+
//// },
26+
//// "include": ["src"],
27+
//// "references": [{ "path": "../common" }]
28+
//// }
29+
30+
// @Filename: /web/src/MyApp.ts
31+
//// import { square } from "../../common/dist/src/MyModule";
32+
33+
// @Filename: /web/src/Helper.ts
34+
//// export function saveMe() {
35+
//// square/**/(2);
36+
//// }
37+
38+
goTo.file("/web/src/Helper.ts");
39+
verify.importFixModuleSpecifiers("", ["../../common/src/MyModule"], {
40+
importModuleSpecifierPreference: "non-relative"
41+
});
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
/// <reference path="../fourslash.ts" />
2+
3+
// @Filename: /common/tsconfig.json
4+
//// {
5+
//// "compilerOptions": {
6+
//// "module": "commonjs",
7+
//// "outDir": "dist",
8+
//// "composite": true
9+
//// },
10+
//// "include": ["src"]
11+
//// }
12+
13+
// @Filename: /common/src/MyModule.ts
14+
//// export function square(n: number) {
15+
//// return n * 2;
16+
//// }
17+
18+
// @Filename: /web/tsconfig.json
19+
//// {
20+
//// "compilerOptions": {
21+
//// "module": "esnext",
22+
//// "moduleResolution": "node",
23+
//// "noEmit": true,
24+
//// "paths": {
25+
//// "@common/*": ["../common/dist/src/*"]
26+
//// }
27+
//// },
28+
//// "include": ["src"],
29+
//// "references": [{ "path": "../common" }]
30+
//// }
31+
32+
// @Filename: /web/src/MyApp.ts
33+
//// import { square } from "@common/MyModule";
34+
35+
// @Filename: /web/src/Helper.ts
36+
//// export function saveMe() {
37+
//// square/**/(2);
38+
//// }
39+
40+
goTo.file("/web/src/Helper.ts");
41+
verify.importFixModuleSpecifiers("", ["@common/MyModule"], {
42+
importModuleSpecifierPreference: "non-relative"
43+
});

0 commit comments

Comments
 (0)