Skip to content

Commit

Permalink
Fix error message for type-only import of ES module from CJS (#59711)
Browse files Browse the repository at this point in the history
  • Loading branch information
andrewbranch authored Aug 23, 2024
1 parent 3abe069 commit a86b5e2
Show file tree
Hide file tree
Showing 12 changed files with 284 additions and 10 deletions.
10 changes: 5 additions & 5 deletions src/compiler/checker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4677,14 +4677,14 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
if (ext === Extension.Ts || ext === Extension.Js || ext === Extension.Tsx || ext === Extension.Jsx) {
diagnosticDetails = createModeMismatchDetails(currentSourceFile);
}

const message = overrideHost?.kind === SyntaxKind.ImportDeclaration && overrideHost.importClause?.isTypeOnly ? Diagnostics.Type_only_import_of_an_ECMAScript_module_from_a_CommonJS_module_must_have_a_resolution_mode_attribute :
overrideHost?.kind === SyntaxKind.ImportType ? Diagnostics.Type_import_of_an_ECMAScript_module_from_a_CommonJS_module_must_have_a_resolution_mode_attribute :
Diagnostics.The_current_file_is_a_CommonJS_module_whose_imports_will_produce_require_calls_however_the_referenced_file_is_an_ECMAScript_module_and_cannot_be_imported_with_require_Consider_writing_a_dynamic_import_0_call_instead;
diagnostics.add(createDiagnosticForNodeFromMessageChain(
getSourceFileOfNode(errorNode),
errorNode,
chainDiagnosticMessages(
diagnosticDetails,
Diagnostics.The_current_file_is_a_CommonJS_module_whose_imports_will_produce_require_calls_however_the_referenced_file_is_an_ECMAScript_module_and_cannot_be_imported_with_require_Consider_writing_a_dynamic_import_0_call_instead,
moduleReference,
),
chainDiagnosticMessages(diagnosticDetails, message, moduleReference),
));
}
}
Expand Down
16 changes: 16 additions & 0 deletions src/compiler/diagnosticMessages.json
Original file line number Diff line number Diff line change
Expand Up @@ -1818,6 +1818,14 @@
"code": 1540,
"reportsDeprecated": true
},
"Type-only import of an ECMAScript module from a CommonJS module must have a 'resolution-mode' attribute.": {
"category": "Error",
"code": 1541
},
"Type import of an ECMAScript module from a CommonJS module must have a 'resolution-mode' attribute.": {
"category": "Error",
"code": 1542
},

"The types of '{0}' are incompatible between these types.": {
"category": "Error",
Expand Down Expand Up @@ -8197,6 +8205,14 @@
"category": "Message",
"code": 95195
},
"Add 'resolution-mode' import attribute": {
"category": "Message",
"code": 95196
},
"Add 'resolution-mode' import attribute to all type-only imports that need it": {
"category": "Message",
"code": 95197
},

"No value exists in scope for the shorthand property '{0}'. Either declare one or provide an initializer.": {
"category": "Error",
Expand Down
6 changes: 3 additions & 3 deletions src/harness/fourslashImpl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3368,8 +3368,8 @@ export class TestState {
this.verifyTextMatches(this.rangeText(this.getOnlyRange()), !!includeWhiteSpace, expectedText);
}

private getOnlyRange() {
const ranges = this.getRanges();
private getOnlyRange(fileName?: string) {
const ranges = fileName ? this.getRangesInFile(fileName) : this.getRanges();
if (ranges.length !== 1) {
this.raiseError("Exactly one range should be specified in the testfile.");
}
Expand Down Expand Up @@ -3455,7 +3455,7 @@ export class TestState {
const change = ts.first(changes);
assert(change.fileName = this.activeFile.fileName);
const newText = ts.textChanges.applyChanges(this.getFileContent(this.activeFile.fileName), change.textChanges);
const newRange = updateTextRangeForTextChanges(this.getOnlyRange(), change.textChanges);
const newRange = updateTextRangeForTextChanges(this.getOnlyRange(this.activeFile.fileName), change.textChanges);
const actualText = newText.slice(newRange.pos, newRange.end);
this.verifyTextMatches(actualText, /*includeWhitespace*/ true, newRangeContent);
}
Expand Down
1 change: 1 addition & 0 deletions src/services/_namespaces/ts.codefix.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ export * from "../codefixes/addMissingAwait.js";
export * from "../codefixes/addMissingConst.js";
export * from "../codefixes/addMissingDeclareProperty.js";
export * from "../codefixes/addMissingInvocationForDecorator.js";
export * from "../codefixes/addMissingResolutionModeImportAttribute.js";
export * from "../codefixes/addNameToNamelessParameter.js";
export * from "../codefixes/addOptionalPropertyUndefined.js";
export * from "../codefixes/annotateWithTypeFromJSDoc.js";
Expand Down
109 changes: 109 additions & 0 deletions src/services/codefixes/addMissingResolutionModeImportAttribute.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,109 @@
import {
codeFixAll,
createCodeFixAction,
registerCodeFix,
} from "../_namespaces/ts.codefix.js";
import {
Debug,
Diagnostics,
factory,
findAncestor,
getQuotePreference,
getTokenAtPosition,
isImportDeclaration,
isImportTypeNode,
LanguageServiceHost,
ModuleKind,
or,
Program,
QuotePreference,
resolveModuleName,
SourceFile,
SyntaxKind,
textChanges,
tryGetModuleSpecifierFromDeclaration,
UserPreferences,
} from "../_namespaces/ts.js";

const fixId = "addMissingResolutionModeImportAttribute";
const errorCodes = [
Diagnostics.Type_only_import_of_an_ECMAScript_module_from_a_CommonJS_module_must_have_a_resolution_mode_attribute.code,
Diagnostics.Type_import_of_an_ECMAScript_module_from_a_CommonJS_module_must_have_a_resolution_mode_attribute.code,
];

registerCodeFix({
errorCodes,
getCodeActions: function getCodeActionsToAddMissingResolutionModeImportAttribute(context) {
const changes = textChanges.ChangeTracker.with(context, t => makeChange(t, context.sourceFile, context.span.start, context.program, context.host, context.preferences));
return [createCodeFixAction(fixId, changes, Diagnostics.Add_resolution_mode_import_attribute, fixId, Diagnostics.Add_resolution_mode_import_attribute_to_all_type_only_imports_that_need_it)];
},
fixIds: [fixId],
getAllCodeActions: context => codeFixAll(context, errorCodes, (changes, diag) => makeChange(changes, diag.file, diag.start, context.program, context.host, context.preferences)),
});

function makeChange(changeTracker: textChanges.ChangeTracker, sourceFile: SourceFile, pos: number, program: Program, host: LanguageServiceHost, preferences: UserPreferences) {
const token = getTokenAtPosition(sourceFile, pos);
const importNode = findAncestor(token, or(isImportDeclaration, isImportTypeNode))!;
Debug.assert(!!importNode, "Expected position to be owned by an ImportDeclaration or ImportType.");
const useSingleQuotes = getQuotePreference(sourceFile, preferences) === QuotePreference.Single;
const moduleSpecifier = tryGetModuleSpecifierFromDeclaration(importNode);
const canUseImportMode = !moduleSpecifier || (resolveModuleName(
moduleSpecifier.text,
sourceFile.fileName,
program.getCompilerOptions(),
host,
program.getModuleResolutionCache(),
/*redirectedReference*/ undefined,
ModuleKind.ESNext,
).resolvedModule?.resolvedFileName === program.getResolvedModuleFromModuleSpecifier(
moduleSpecifier,
sourceFile,
)?.resolvedModule?.resolvedFileName);

const attributes = importNode.attributes
? factory.updateImportAttributes(
importNode.attributes,
factory.createNodeArray([
...importNode.attributes.elements,
factory.createImportAttribute(
factory.createStringLiteral("resolution-mode", useSingleQuotes),
factory.createStringLiteral(canUseImportMode ? "import" : "require", useSingleQuotes),
),
], importNode.attributes.elements.hasTrailingComma),
importNode.attributes.multiLine,
)
: factory.createImportAttributes(
factory.createNodeArray([
factory.createImportAttribute(
factory.createStringLiteral("resolution-mode", useSingleQuotes),
factory.createStringLiteral(canUseImportMode ? "import" : "require", useSingleQuotes),
),
]),
);
if (importNode.kind === SyntaxKind.ImportDeclaration) {
changeTracker.replaceNode(
sourceFile,
importNode,
factory.updateImportDeclaration(
importNode,
importNode.modifiers,
importNode.importClause,
importNode.moduleSpecifier,
attributes,
),
);
}
else {
changeTracker.replaceNode(
sourceFile,
importNode,
factory.updateImportTypeNode(
importNode,
importNode.argument,
attributes,
importNode.qualifier,
importNode.typeArguments,
),
);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -396,7 +396,7 @@ File '/user/username/projects/myproject/packages/pkg2/build/const.d.cts' exists
File '/a/lib/package.json' does not exist.
File '/a/package.json' does not exist.
File '/package.json' does not exist.
[96mpackages/pkg1/index.ts[0m:[93m1[0m:[93m29[0m - [91merror[0m[90m TS1479: [0mThe current file is a CommonJS module whose imports will produce 'require' calls; however, the referenced file is an ECMAScript module and cannot be imported with 'require'. Consider writing a dynamic 'import("pkg2")' call instead.
[96mpackages/pkg1/index.ts[0m:[93m1[0m:[93m29[0m - [91merror[0m[90m TS1541: [0mType-only import of an ECMAScript module from a CommonJS module must have a 'resolution-mode' attribute.
To convert this file to an ECMAScript module, change its file extension to '.mts' or create a local package.json file with `{ "type": "module" }`.

1 import type { TheNum } from 'pkg2'
Expand Down Expand Up @@ -631,7 +631,7 @@ File '/user/username/projects/myproject/packages/pkg2/build/const.d.cts' exists
File '/a/lib/package.json' does not exist.
File '/a/package.json' does not exist.
File '/package.json' does not exist.
[96mpackages/pkg1/index.ts[0m:[93m1[0m:[93m29[0m - [91merror[0m[90m TS1479: [0mThe current file is a CommonJS module whose imports will produce 'require' calls; however, the referenced file is an ECMAScript module and cannot be imported with 'require'. Consider writing a dynamic 'import("pkg2")' call instead.
[96mpackages/pkg1/index.ts[0m:[93m1[0m:[93m29[0m - [91merror[0m[90m TS1541: [0mType-only import of an ECMAScript module from a CommonJS module must have a 'resolution-mode' attribute.
To convert this file to an ECMAScript module, change its file extension to '.mts' or create a local package.json file with `{ "type": "module" }`.

1 import type { TheNum } from 'pkg2'
Expand Down
19 changes: 19 additions & 0 deletions tests/baselines/reference/typeOnlyESMImportFromCJS.errors.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
common.cts(1,21): error TS1541: Type-only import of an ECMAScript module from a CommonJS module must have a 'resolution-mode' attribute.
common.cts(4,25): error TS1542: Type import of an ECMAScript module from a CommonJS module must have a 'resolution-mode' attribute.


==== module.mts (0 errors) ====
export {};

==== common.cts (2 errors) ====
import type {} from "./module.mts";
~~~~~~~~~~~~~~
!!! error TS1541: Type-only import of an ECMAScript module from a CommonJS module must have a 'resolution-mode' attribute.
import type {} from "./module.mts" with { "resolution-mode": "import" };
import type {} from "./module.mts" with { "resolution-mode": "require" };
type _1 = typeof import("./module.mts");
~~~~~~~~~~~~~~
!!! error TS1542: Type import of an ECMAScript module from a CommonJS module must have a 'resolution-mode' attribute.
type _2 = typeof import("./module.mts", { with: { "resolution-mode": "import" } });
type _3 = typeof import("./module.mts", { with: { "resolution-mode": "require" } });

19 changes: 19 additions & 0 deletions tests/baselines/reference/typeOnlyESMImportFromCJS.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
//// [tests/cases/conformance/externalModules/typeOnly/typeOnlyESMImportFromCJS.ts] ////

//// [module.mts]
export {};

//// [common.cts]
import type {} from "./module.mts";
import type {} from "./module.mts" with { "resolution-mode": "import" };
import type {} from "./module.mts" with { "resolution-mode": "require" };
type _1 = typeof import("./module.mts");
type _2 = typeof import("./module.mts", { with: { "resolution-mode": "import" } });
type _3 = typeof import("./module.mts", { with: { "resolution-mode": "require" } });


//// [module.mjs]
export {};
//// [common.cjs]
"use strict";
Object.defineProperty(exports, "__esModule", { value: true });
19 changes: 19 additions & 0 deletions tests/baselines/reference/typeOnlyESMImportFromCJS.symbols
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
//// [tests/cases/conformance/externalModules/typeOnly/typeOnlyESMImportFromCJS.ts] ////

=== module.mts ===

export {};

=== common.cts ===
import type {} from "./module.mts";
import type {} from "./module.mts" with { "resolution-mode": "import" };
import type {} from "./module.mts" with { "resolution-mode": "require" };
type _1 = typeof import("./module.mts");
>_1 : Symbol(_1, Decl(common.cts, 2, 73))

type _2 = typeof import("./module.mts", { with: { "resolution-mode": "import" } });
>_2 : Symbol(_2, Decl(common.cts, 3, 40))

type _3 = typeof import("./module.mts", { with: { "resolution-mode": "require" } });
>_3 : Symbol(_3, Decl(common.cts, 4, 83))

22 changes: 22 additions & 0 deletions tests/baselines/reference/typeOnlyESMImportFromCJS.types
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
//// [tests/cases/conformance/externalModules/typeOnly/typeOnlyESMImportFromCJS.ts] ////

=== module.mts ===

export {};

=== common.cts ===
import type {} from "./module.mts";
import type {} from "./module.mts" with { "resolution-mode": "import" };
import type {} from "./module.mts" with { "resolution-mode": "require" };
type _1 = typeof import("./module.mts");
>_1 : typeof import("module", { with: { "resolution-mode": "import" } })
> : ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

type _2 = typeof import("./module.mts", { with: { "resolution-mode": "import" } });
>_2 : typeof import("module", { with: { "resolution-mode": "import" } })
> : ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

type _3 = typeof import("./module.mts", { with: { "resolution-mode": "require" } });
>_3 : typeof import("module", { with: { "resolution-mode": "import" } })
> : ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
// @module: nodenext

// @Filename: module.mts
export {};

// @Filename: common.cts
import type {} from "./module.mts";
import type {} from "./module.mts" with { "resolution-mode": "import" };
import type {} from "./module.mts" with { "resolution-mode": "require" };
type _1 = typeof import("./module.mts");
type _2 = typeof import("./module.mts", { with: { "resolution-mode": "import" } });
type _3 = typeof import("./module.mts", { with: { "resolution-mode": "require" } });
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
/// <reference path='fourslash.ts' />

// @module: nodenext

// @Filename: /package.json
//// { "type": "module" }

// @Filename: /module.ts
//// export {};

// @Filename: /common1.cts
//// [|import type { } from "./module.ts"/*1*/;|]

// @Filename: /common2.cts
//// [|import type { } from "./module.ts"/*2*/ with { "type": "typescript" };|]

// @Filename: /common3.cts
//// [|import type { } from "./module"/*3*/;|]

// @Filename: /common4.cts
//// [|type _1 = typeof import("./module.ts"/*4*/);|]

goTo.marker("1");
verify.codeFix({
index: 0,
errorCode: ts.Diagnostics.Type_only_import_of_an_ECMAScript_module_from_a_CommonJS_module_must_have_a_resolution_mode_attribute.code,
applyChanges: false,
description: "Add 'resolution-mode' import attribute",
newRangeContent: `import type { } from "./module.ts" with { "resolution-mode": "import" };`,
});

goTo.marker("2");
verify.codeFix({
index: 0,
errorCode: ts.Diagnostics.Type_only_import_of_an_ECMAScript_module_from_a_CommonJS_module_must_have_a_resolution_mode_attribute.code,
applyChanges: false,
description: "Add 'resolution-mode' import attribute",
newRangeContent: `import type { } from "./module.ts" with { "type": "typescript", "resolution-mode": "import" };`,
});

goTo.marker("3");
verify.codeFix({
index: 0,
errorCode: ts.Diagnostics.Type_only_import_of_an_ECMAScript_module_from_a_CommonJS_module_must_have_a_resolution_mode_attribute.code,
applyChanges: false,
description: "Add 'resolution-mode' import attribute",
newRangeContent: `import type { } from "./module" with { "resolution-mode": "require" };`,
});

goTo.marker("4");
verify.codeFix({
index: 0,
errorCode: ts.Diagnostics.Type_import_of_an_ECMAScript_module_from_a_CommonJS_module_must_have_a_resolution_mode_attribute.code,
applyChanges: false,
description: "Add 'resolution-mode' import attribute",
newRangeContent: `type _1 = typeof import("./module.ts", { with: { "resolution-mode": "import" } });`,
});

0 comments on commit a86b5e2

Please sign in to comment.