Skip to content

Commit

Permalink
Prefer local module specifier over relative node_modules ones in auto…
Browse files Browse the repository at this point in the history
…-import, even when it reaches into a monorepo package (#55969)
  • Loading branch information
andrewbranch authored Oct 3, 2023
1 parent 3116b89 commit 248488a
Show file tree
Hide file tree
Showing 3 changed files with 41 additions and 3 deletions.
15 changes: 13 additions & 2 deletions src/compiler/moduleSpecifiers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -315,6 +315,7 @@ export function getModuleSpecifiers(
host,
userPreferences,
options,
/*forAutoImport*/ false,
).moduleSpecifiers;
}

Expand All @@ -327,6 +328,7 @@ export function getModuleSpecifiersWithCacheInfo(
host: ModuleSpecifierResolutionHost,
userPreferences: UserPreferences,
options: ModuleSpecifierOptions = {},
forAutoImport: boolean,
): { moduleSpecifiers: readonly string[]; computedWithoutCache: boolean; } {
let computedWithoutCache = false;
const ambient = tryGetModuleNameFromAmbientModule(moduleSymbol, checker);
Expand All @@ -345,7 +347,15 @@ export function getModuleSpecifiersWithCacheInfo(

computedWithoutCache = true;
modulePaths ||= getAllModulePathsWorker(importingSourceFile.path, moduleSourceFile.originalFileName, host);
const result = computeModuleSpecifiers(modulePaths, compilerOptions, importingSourceFile, host, userPreferences, options);
const result = computeModuleSpecifiers(
modulePaths,
compilerOptions,
importingSourceFile,
host,
userPreferences,
options,
forAutoImport,
);
cache?.set(importingSourceFile.path, moduleSourceFile.path, userPreferences, options, modulePaths, result);
return { moduleSpecifiers: result, computedWithoutCache };
}
Expand All @@ -357,6 +367,7 @@ function computeModuleSpecifiers(
host: ModuleSpecifierResolutionHost,
userPreferences: UserPreferences,
options: ModuleSpecifierOptions = {},
forAutoImport: boolean,
): readonly string[] {
const info = getInfo(importingSourceFile.path, host);
const preferences = getPreferences(userPreferences, compilerOptions, importingSourceFile);
Expand Down Expand Up @@ -421,7 +432,7 @@ function computeModuleSpecifiers(
else if (pathIsBareSpecifier(local)) {
pathsSpecifiers = append(pathsSpecifiers, local);
}
else if (!importedFileIsInNodeModules || modulePath.isInNodeModules) {
else if (forAutoImport || !importedFileIsInNodeModules || modulePath.isInNodeModules) {
// Why this extra conditional, not just an `else`? If some path to the file contained
// 'node_modules', but we can't create a non-relative specifier (e.g. "@foo/bar/path/to/file"),
// that means we had to go through a *sibling's* node_modules, not one we can access directly.
Expand Down
2 changes: 1 addition & 1 deletion src/services/codefixes/importFixes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -877,7 +877,7 @@ function getNewImportFixes(
const rejectNodeModulesRelativePaths = moduleResolutionUsesNodeModules(moduleResolution);
const getModuleSpecifiers = fromCacheOnly
? (moduleSymbol: Symbol) => ({ moduleSpecifiers: moduleSpecifiers.tryGetModuleSpecifiersFromCache(moduleSymbol, sourceFile, moduleSpecifierResolutionHost, preferences), computedWithoutCache: false })
: (moduleSymbol: Symbol, checker: TypeChecker) => moduleSpecifiers.getModuleSpecifiersWithCacheInfo(moduleSymbol, checker, compilerOptions, sourceFile, moduleSpecifierResolutionHost, preferences);
: (moduleSymbol: Symbol, checker: TypeChecker) => moduleSpecifiers.getModuleSpecifiersWithCacheInfo(moduleSymbol, checker, compilerOptions, sourceFile, moduleSpecifierResolutionHost, preferences, /*options*/ undefined, /*forAutoImport*/ true);

let computedWithoutCacheCount = 0;
const fixes = flatMap(exportInfo, (exportInfo, i) => {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
/// <reference path="../fourslash.ts" />

// @Filename: /tsconfig.json
//// {
//// "compilerOptions": {
//// "module": "nodenext"
//// }
//// }

// @Filename: /packages/app/dist/index.d.ts
//// import {} from "utils";
//// export const app: number;

// @Filename: /packages/utils/package.json
//// { "name": "utils", "version": "1.0.0", "main": "dist/index.js" }

// @Filename: /packages/utils/dist/index.d.ts
//// export const x: number;

// @link: /packages/utils -> /packages/app/node_modules/utils

// @Filename: /script.ts
//// import {} from "./packages/app/dist/index.js";
//// x/**/

goTo.marker("");
verify.importFixModuleSpecifiers("", ["./packages/utils/dist/index.js"]);

0 comments on commit 248488a

Please sign in to comment.