Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 24 additions & 23 deletions src/compiler/moduleSpecifiers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand All @@ -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);
}

Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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 {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this line is comically long

if (!host.fileExists || !host.readFile) {
return undefined;
}
Expand All @@ -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);
Expand All @@ -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;
}
}
Expand Down Expand Up @@ -768,28 +769,22 @@ 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))) {
return { packageRootPath, moduleFileToTry };
}
}
}
return { moduleFileToTry };
}

function getExtensionlessFileName(path: string): string {
Comment thread
andrewbranch marked this conversation as resolved.
// 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 };
}
}

Expand All @@ -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:
Expand Down
11 changes: 11 additions & 0 deletions tests/cases/fourslash/autoImportNoPackageJson_commonjs.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
/// <reference path="fourslash.ts" />

// @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"]);
11 changes: 11 additions & 0 deletions tests/cases/fourslash/autoImportNoPackageJson_nodenext.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
/// <reference path="fourslash.ts" />

// @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"]);
17 changes: 17 additions & 0 deletions tests/cases/fourslash/importNameCodeFixJsEnding.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
/// <reference path="fourslash.ts"/>

// @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" })

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should there be a test for .cjs/.mjs as well, since you mentioned that they were broken before?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also, are there already tests for filename/package conflicts? I assume yes.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are .cjs tests. Filename/package conflicts aren’t related to this change, I think.