From 248488aa061d2f5a334de8ee26345f93e58b0be4 Mon Sep 17 00:00:00 2001 From: Andrew Branch Date: Tue, 3 Oct 2023 14:10:06 -0700 Subject: [PATCH] Prefer local module specifier over relative node_modules ones in auto-import, even when it reaches into a monorepo package (#55969) --- src/compiler/moduleSpecifiers.ts | 15 +++++++++-- src/services/codefixes/importFixes.ts | 2 +- ...autoImportRelativePathToMonorepoPackage.ts | 27 +++++++++++++++++++ 3 files changed, 41 insertions(+), 3 deletions(-) create mode 100644 tests/cases/fourslash/server/autoImportRelativePathToMonorepoPackage.ts diff --git a/src/compiler/moduleSpecifiers.ts b/src/compiler/moduleSpecifiers.ts index 2a59e6c5be814..97e8e6b5edc81 100644 --- a/src/compiler/moduleSpecifiers.ts +++ b/src/compiler/moduleSpecifiers.ts @@ -315,6 +315,7 @@ export function getModuleSpecifiers( host, userPreferences, options, + /*forAutoImport*/ false, ).moduleSpecifiers; } @@ -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); @@ -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 }; } @@ -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); @@ -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. diff --git a/src/services/codefixes/importFixes.ts b/src/services/codefixes/importFixes.ts index b0466d065d982..a93a2ad44afc9 100644 --- a/src/services/codefixes/importFixes.ts +++ b/src/services/codefixes/importFixes.ts @@ -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) => { diff --git a/tests/cases/fourslash/server/autoImportRelativePathToMonorepoPackage.ts b/tests/cases/fourslash/server/autoImportRelativePathToMonorepoPackage.ts new file mode 100644 index 0000000000000..5b03927647c70 --- /dev/null +++ b/tests/cases/fourslash/server/autoImportRelativePathToMonorepoPackage.ts @@ -0,0 +1,27 @@ +/// + +// @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"]);