From 736c8a12a386d0ebb0a3db7aaf7697b8e73169f5 Mon Sep 17 00:00:00 2001 From: Andrew Branch Date: Fri, 6 May 2022 10:47:46 -0700 Subject: [PATCH 1/2] Respect importModuleSpecifierEnding inside node_modules packages --- src/compiler/moduleSpecifiers.ts | 47 ++++++++++--------- .../fourslash/importNameCodeFixJsEnding.ts | 17 +++++++ 2 files changed, 41 insertions(+), 23 deletions(-) create mode 100644 tests/cases/fourslash/importNameCodeFixJsEnding.ts diff --git a/src/compiler/moduleSpecifiers.ts b/src/compiler/moduleSpecifiers.ts index 542afb796a933..ed8d9f90b0ae2 100644 --- a/src/compiler/moduleSpecifiers.ts +++ b/src/compiler/moduleSpecifiers.ts @@ -105,7 +105,7 @@ namespace ts.moduleSpecifiers { const info = getInfo(importingSourceFile.path, host); const modulePaths = getAllModulePaths(importingSourceFile.path, nodeModulesFileName, host, preferences, options); return firstDefined(modulePaths, - modulePath => tryGetModuleNameAsNodeModule(modulePath, info, importingSourceFile, host, compilerOptions, /*packageNameOnly*/ true, options.overrideImportMode)); + modulePath => tryGetModuleNameAsNodeModule(modulePath, info, importingSourceFile, host, compilerOptions, preferences, /*packageNameOnly*/ true, options.overrideImportMode)); } function getModuleSpecifierWorker( @@ -120,7 +120,7 @@ namespace ts.moduleSpecifiers { ): string { const info = getInfo(importingSourceFileName, host); const modulePaths = getAllModulePaths(importingSourceFileName, toFileName, host, userPreferences, options); - return firstDefined(modulePaths, modulePath => tryGetModuleNameAsNodeModule(modulePath, info, importingSourceFile, host, compilerOptions, /*packageNameOnly*/ undefined, options.overrideImportMode)) || + return firstDefined(modulePaths, modulePath => tryGetModuleNameAsNodeModule(modulePath, info, importingSourceFile, host, compilerOptions, userPreferences, /*packageNameOnly*/ undefined, options.overrideImportMode)) || getLocalModuleSpecifier(toFileName, info, compilerOptions, host, preferences); } @@ -248,7 +248,7 @@ namespace ts.moduleSpecifiers { let pathsSpecifiers: string[] | undefined; let relativeSpecifiers: string[] | undefined; for (const modulePath of modulePaths) { - const specifier = tryGetModuleNameAsNodeModule(modulePath, info, importingSourceFile, host, compilerOptions, /*packageNameOnly*/ undefined, options.overrideImportMode); + const specifier = tryGetModuleNameAsNodeModule(modulePath, info, importingSourceFile, host, compilerOptions, userPreferences, /*packageNameOnly*/ undefined, options.overrideImportMode); nodeModulesSpecifiers = append(nodeModulesSpecifiers, specifier); if (specifier && modulePath.isRedirect) { // If we got a specifier for a redirect, it was a bare package specifier (e.g. "@foo/bar", @@ -666,7 +666,7 @@ namespace ts.moduleSpecifiers { : removeFileExtension(relativePath); } - function tryGetModuleNameAsNodeModule({ path, isRedirect }: ModulePath, { getCanonicalFileName, sourceDirectory }: Info, importingSourceFile: SourceFile , host: ModuleSpecifierResolutionHost, options: CompilerOptions, packageNameOnly?: boolean, overrideMode?: ModuleKind.ESNext | ModuleKind.CommonJS): string | undefined { + function tryGetModuleNameAsNodeModule({ path, isRedirect }: ModulePath, { getCanonicalFileName, sourceDirectory }: Info, importingSourceFile: SourceFile , host: ModuleSpecifierResolutionHost, options: CompilerOptions, userPreferences: UserPreferences, packageNameOnly?: boolean, overrideMode?: ModuleKind.ESNext | ModuleKind.CommonJS): string | undefined { if (!host.fileExists || !host.readFile) { return undefined; } @@ -680,8 +680,9 @@ namespace ts.moduleSpecifiers { let moduleSpecifier = path; let isPackageRootPath = false; if (!packageNameOnly) { + const preferences = getPreferences(host, userPreferences, options, importingSourceFile); let packageRootIndex = parts.packageRootIndex; - let moduleFileNameForExtensionless: string | undefined; + let moduleFileName: string | undefined; while (true) { // If the module could be imported by a directory name, use that directory's name const { moduleFileToTry, packageRootPath, blockedByExports, verbatimFromExports } = tryDirectoryWithPackageJson(packageRootIndex); @@ -698,12 +699,12 @@ namespace ts.moduleSpecifiers { isPackageRootPath = true; break; } - if (!moduleFileNameForExtensionless) moduleFileNameForExtensionless = moduleFileToTry; + if (!moduleFileName) moduleFileName = moduleFileToTry; // try with next level of directory packageRootIndex = path.indexOf(directorySeparator, packageRootIndex + 1); if (packageRootIndex === -1) { - moduleSpecifier = getExtensionlessFileName(moduleFileNameForExtensionless); + moduleSpecifier = removeExtensionAndIndexPostFix(moduleFileName, preferences.ending, options, host); break; } } @@ -768,7 +769,7 @@ namespace ts.moduleSpecifiers { } } // If the file is the main module, it can be imported by the package name - const mainFileRelative = packageJsonContent.typings || packageJsonContent.types || packageJsonContent.main; + const mainFileRelative = packageJsonContent.typings || packageJsonContent.types || packageJsonContent.main || "index.js"; if (isString(mainFileRelative)) { const mainExportFile = toPath(mainFileRelative, packageRootPath, getCanonicalFileName); if (removeFileExtension(mainExportFile) === removeFileExtension(getCanonicalFileName(moduleFileToTry))) { @@ -776,20 +777,14 @@ namespace ts.moduleSpecifiers { } } } - return { moduleFileToTry }; - } - - function getExtensionlessFileName(path: string): string { - // We still have a file name - remove the extension - const fullModulePathWithoutExtension = removeFileExtension(path); - - // If the file is /index, it can be imported by its directory name - // IFF there is not _also_ a file by the same name - if (getCanonicalFileName(fullModulePathWithoutExtension.substring(parts.fileNameIndex)) === "/index" && !tryGetAnyFileFromPath(host, fullModulePathWithoutExtension.substring(0, parts.fileNameIndex))) { - return fullModulePathWithoutExtension.substring(0, parts.fileNameIndex); + else { + // No package.json exists; an index.js will still resolve as the package name + const fileName = getCanonicalFileName(moduleFileToTry.substring(parts.packageRootIndex + 1)); + if (fileName === "index.d.ts" || fileName === "index.js" || fileName === "index.ts" || fileName === "index.tsx") { + return { moduleFileToTry, packageRootPath }; + } } - - return fullModulePathWithoutExtension; + return { moduleFileToTry }; } } @@ -812,13 +807,19 @@ namespace ts.moduleSpecifiers { }); } - function removeExtensionAndIndexPostFix(fileName: string, ending: Ending, options: CompilerOptions): string { + function removeExtensionAndIndexPostFix(fileName: string, ending: Ending, options: CompilerOptions, host?: ModuleSpecifierResolutionHost): string { if (fileExtensionIsOneOf(fileName, [Extension.Json, Extension.Mjs, Extension.Cjs])) return fileName; const noExtension = removeFileExtension(fileName); if (fileExtensionIsOneOf(fileName, [Extension.Dmts, Extension.Mts, Extension.Dcts, Extension.Cts])) return noExtension + getJSExtensionForFile(fileName, options); switch (ending) { case Ending.Minimal: - return removeSuffix(noExtension, "/index"); + const withoutIndex = removeSuffix(noExtension, "/index"); + if (host && withoutIndex !== noExtension && tryGetAnyFileFromPath(host, withoutIndex)) { + // Can't remove index if there's a file by the same name as the directory. + // Probably more callers should pass `host` so we can determine this? + return noExtension; + } + return withoutIndex; case Ending.Index: return noExtension; case Ending.JsExtension: diff --git a/tests/cases/fourslash/importNameCodeFixJsEnding.ts b/tests/cases/fourslash/importNameCodeFixJsEnding.ts new file mode 100644 index 0000000000000..3ab4215e7f758 --- /dev/null +++ b/tests/cases/fourslash/importNameCodeFixJsEnding.ts @@ -0,0 +1,17 @@ +/// + +// @module: commonjs + +// @Filename: /node_modules/lit/package.json +//// { "name": "lit", "version": "1.0.0" } + +// @Filename: /node_modules/lit/index.d.ts +//// import "./decorators"; + +// @Filename: /node_modules/lit/decorators.d.ts +//// export declare function customElement(name: string): any; + +// @Filename: /a.ts +//// customElement/**/ + +verify.importFixModuleSpecifiers("", ["lit/decorators.js"], { importModuleSpecifierEnding: "js" }) From bf6b47f15d05bc3d01f3c1f8ad460b1e87a0e7ca Mon Sep 17 00:00:00 2001 From: Andrew Branch Date: Fri, 6 May 2022 11:27:41 -0700 Subject: [PATCH 2/2] Add tests for missing package.json --- .../fourslash/autoImportNoPackageJson_commonjs.ts | 11 +++++++++++ .../fourslash/autoImportNoPackageJson_nodenext.ts | 11 +++++++++++ 2 files changed, 22 insertions(+) create mode 100644 tests/cases/fourslash/autoImportNoPackageJson_commonjs.ts create mode 100644 tests/cases/fourslash/autoImportNoPackageJson_nodenext.ts diff --git a/tests/cases/fourslash/autoImportNoPackageJson_commonjs.ts b/tests/cases/fourslash/autoImportNoPackageJson_commonjs.ts new file mode 100644 index 0000000000000..1d9a0ad323303 --- /dev/null +++ b/tests/cases/fourslash/autoImportNoPackageJson_commonjs.ts @@ -0,0 +1,11 @@ +/// + +// @module: commonjs + +// @Filename: /node_modules/lit/index.d.cts +//// export declare function customElement(name: string): any; + +// @Filename: /a.ts +//// customElement/**/ + +verify.importFixModuleSpecifiers("", ["lit/index.cjs"]); diff --git a/tests/cases/fourslash/autoImportNoPackageJson_nodenext.ts b/tests/cases/fourslash/autoImportNoPackageJson_nodenext.ts new file mode 100644 index 0000000000000..ff57e8fe71070 --- /dev/null +++ b/tests/cases/fourslash/autoImportNoPackageJson_nodenext.ts @@ -0,0 +1,11 @@ +/// + +// @module: nodenext + +// @Filename: /node_modules/lit/index.d.cts +//// export declare function customElement(name: string): any; + +// @Filename: /a.ts +//// customElement/**/ + +verify.importFixModuleSpecifiers("", ["lit/index.cjs"]);