Skip to content

Commit a86b5e2

Browse files
andrewbranchandrewbranch
authored
Fix error message for type-only import of ES module from CJS (#59711)
1 parent 3abe069 commit a86b5e2

12 files changed

+284
-10
lines changed

‎src/compiler/checker.ts‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4677,14 +4677,14 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
46774677
if (ext === Extension.Ts || ext === Extension.Js || ext === Extension.Tsx || ext === Extension.Jsx) {
46784678
diagnosticDetails = createModeMismatchDetails(currentSourceFile);
46794679
}
4680+
4681+
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 :
4682+
overrideHost?.kind === SyntaxKind.ImportType ? Diagnostics.Type_import_of_an_ECMAScript_module_from_a_CommonJS_module_must_have_a_resolution_mode_attribute :
4683+
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;
46804684
diagnostics.add(createDiagnosticForNodeFromMessageChain(
46814685
getSourceFileOfNode(errorNode),
46824686
errorNode,
4683-
chainDiagnosticMessages(
4684-
diagnosticDetails,
4685-
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,
4686-
moduleReference,
4687-
),
4687+
chainDiagnosticMessages(diagnosticDetails, message, moduleReference),
46884688
));
46894689
}
46904690
}

‎src/compiler/diagnosticMessages.json‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1818,6 +1818,14 @@
18181818
"code": 1540,
18191819
"reportsDeprecated": true
18201820
},
1821+
"Type-only import of an ECMAScript module from a CommonJS module must have a 'resolution-mode' attribute.": {
1822+
"category": "Error",
1823+
"code": 1541
1824+
},
1825+
"Type import of an ECMAScript module from a CommonJS module must have a 'resolution-mode' attribute.": {
1826+
"category": "Error",
1827+
"code": 1542
1828+
},
18211829

18221830
"The types of '{0}' are incompatible between these types.": {
18231831
"category": "Error",
@@ -8197,6 +8205,14 @@
81978205
"category": "Message",
81988206
"code": 95195
81998207
},
8208+
"Add 'resolution-mode' import attribute": {
8209+
"category": "Message",
8210+
"code": 95196
8211+
},
8212+
"Add 'resolution-mode' import attribute to all type-only imports that need it": {
8213+
"category": "Message",
8214+
"code": 95197
8215+
},
82008216

82018217
"No value exists in scope for the shorthand property '{0}'. Either declare one or provide an initializer.": {
82028218
"category": "Error",

‎src/harness/fourslashImpl.ts‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3368,8 +3368,8 @@ export class TestState {
33683368
this.verifyTextMatches(this.rangeText(this.getOnlyRange()), !!includeWhiteSpace, expectedText);
33693369
}
33703370

3371-
private getOnlyRange() {
3372-
const ranges = this.getRanges();
3371+
private getOnlyRange(fileName?: string) {
3372+
const ranges = fileName ? this.getRangesInFile(fileName) : this.getRanges();
33733373
if (ranges.length !== 1) {
33743374
this.raiseError("Exactly one range should be specified in the testfile.");
33753375
}
@@ -3455,7 +3455,7 @@ export class TestState {
34553455
const change = ts.first(changes);
34563456
assert(change.fileName = this.activeFile.fileName);
34573457
const newText = ts.textChanges.applyChanges(this.getFileContent(this.activeFile.fileName), change.textChanges);
3458-
const newRange = updateTextRangeForTextChanges(this.getOnlyRange(), change.textChanges);
3458+
const newRange = updateTextRangeForTextChanges(this.getOnlyRange(this.activeFile.fileName), change.textChanges);
34593459
const actualText = newText.slice(newRange.pos, newRange.end);
34603460
this.verifyTextMatches(actualText, /*includeWhitespace*/ true, newRangeContent);
34613461
}

‎src/services/_namespaces/ts.codefix.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ export * from "../codefixes/addMissingAwait.js";
88
export * from "../codefixes/addMissingConst.js";
99
export * from "../codefixes/addMissingDeclareProperty.js";
1010
export * from "../codefixes/addMissingInvocationForDecorator.js";
11+
export * from "../codefixes/addMissingResolutionModeImportAttribute.js";
1112
export * from "../codefixes/addNameToNamelessParameter.js";
1213
export * from "../codefixes/addOptionalPropertyUndefined.js";
1314
export * from "../codefixes/annotateWithTypeFromJSDoc.js";

‎src/services/codefixes/addMissingResolutionModeImportAttribute.ts‎

Lines changed: 109 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,109 @@
1+
import {
2+
codeFixAll,
3+
createCodeFixAction,
4+
registerCodeFix,
5+
} from "../_namespaces/ts.codefix.js";
6+
import {
7+
Debug,
8+
Diagnostics,
9+
factory,
10+
findAncestor,
11+
getQuotePreference,
12+
getTokenAtPosition,
13+
isImportDeclaration,
14+
isImportTypeNode,
15+
LanguageServiceHost,
16+
ModuleKind,
17+
or,
18+
Program,
19+
QuotePreference,
20+
resolveModuleName,
21+
SourceFile,
22+
SyntaxKind,
23+
textChanges,
24+
tryGetModuleSpecifierFromDeclaration,
25+
UserPreferences,
26+
} from "../_namespaces/ts.js";
27+
28+
const fixId = "addMissingResolutionModeImportAttribute";
29+
const errorCodes = [
30+
Diagnostics.Type_only_import_of_an_ECMAScript_module_from_a_CommonJS_module_must_have_a_resolution_mode_attribute.code,
31+
Diagnostics.Type_import_of_an_ECMAScript_module_from_a_CommonJS_module_must_have_a_resolution_mode_attribute.code,
32+
];
33+
34+
registerCodeFix({
35+
errorCodes,
36+
getCodeActions: function getCodeActionsToAddMissingResolutionModeImportAttribute(context) {
37+
const changes = textChanges.ChangeTracker.with(context, t => makeChange(t, context.sourceFile, context.span.start, context.program, context.host, context.preferences));
38+
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)];
39+
},
40+
fixIds: [fixId],
41+
getAllCodeActions: context => codeFixAll(context, errorCodes, (changes, diag) => makeChange(changes, diag.file, diag.start, context.program, context.host, context.preferences)),
42+
});
43+
44+
function makeChange(changeTracker: textChanges.ChangeTracker, sourceFile: SourceFile, pos: number, program: Program, host: LanguageServiceHost, preferences: UserPreferences) {
45+
const token = getTokenAtPosition(sourceFile, pos);
46+
const importNode = findAncestor(token, or(isImportDeclaration, isImportTypeNode))!;
47+
Debug.assert(!!importNode, "Expected position to be owned by an ImportDeclaration or ImportType.");
48+
const useSingleQuotes = getQuotePreference(sourceFile, preferences) === QuotePreference.Single;
49+
const moduleSpecifier = tryGetModuleSpecifierFromDeclaration(importNode);
50+
const canUseImportMode = !moduleSpecifier || (resolveModuleName(
51+
moduleSpecifier.text,
52+
sourceFile.fileName,
53+
program.getCompilerOptions(),
54+
host,
55+
program.getModuleResolutionCache(),
56+
/*redirectedReference*/ undefined,
57+
ModuleKind.ESNext,
58+
).resolvedModule?.resolvedFileName === program.getResolvedModuleFromModuleSpecifier(
59+
moduleSpecifier,
60+
sourceFile,
61+
)?.resolvedModule?.resolvedFileName);
62+
63+
const attributes = importNode.attributes
64+
? factory.updateImportAttributes(
65+
importNode.attributes,
66+
factory.createNodeArray([
67+
...importNode.attributes.elements,
68+
factory.createImportAttribute(
69+
factory.createStringLiteral("resolution-mode", useSingleQuotes),
70+
factory.createStringLiteral(canUseImportMode ? "import" : "require", useSingleQuotes),
71+
),
72+
], importNode.attributes.elements.hasTrailingComma),
73+
importNode.attributes.multiLine,
74+
)
75+
: factory.createImportAttributes(
76+
factory.createNodeArray([
77+
factory.createImportAttribute(
78+
factory.createStringLiteral("resolution-mode", useSingleQuotes),
79+
factory.createStringLiteral(canUseImportMode ? "import" : "require", useSingleQuotes),
80+
),
81+
]),
82+
);
83+
if (importNode.kind === SyntaxKind.ImportDeclaration) {
84+
changeTracker.replaceNode(
85+
sourceFile,
86+
importNode,
87+
factory.updateImportDeclaration(
88+
importNode,
89+
importNode.modifiers,
90+
importNode.importClause,
91+
importNode.moduleSpecifier,
92+
attributes,
93+
),
94+
);
95+
}
96+
else {
97+
changeTracker.replaceNode(
98+
sourceFile,
99+
importNode,
100+
factory.updateImportTypeNode(
101+
importNode,
102+
importNode.argument,
103+
attributes,
104+
importNode.qualifier,
105+
importNode.typeArguments,
106+
),
107+
);
108+
}
109+
}

‎tests/baselines/reference/tsbuildWatch/moduleResolution/resolves-specifier-in-output-declaration-file-from-referenced-project-correctly-with-cts-and-mts-extensions.js‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -396,7 +396,7 @@ File '/user/username/projects/myproject/packages/pkg2/build/const.d.cts' exists
396396
File '/a/lib/package.json' does not exist.
397397
File '/a/package.json' does not exist.
398398
File '/package.json' does not exist.
399-
[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.
399+
[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.
400400
To convert this file to an ECMAScript module, change its file extension to '.mts' or create a local package.json file with `{ "type": "module" }`.
401401

402402
1 import type { TheNum } from 'pkg2'
@@ -631,7 +631,7 @@ File '/user/username/projects/myproject/packages/pkg2/build/const.d.cts' exists
631631
File '/a/lib/package.json' does not exist.
632632
File '/a/package.json' does not exist.
633633
File '/package.json' does not exist.
634-
[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.
634+
[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.
635635
To convert this file to an ECMAScript module, change its file extension to '.mts' or create a local package.json file with `{ "type": "module" }`.
636636

637637
1 import type { TheNum } from 'pkg2'

‎tests/baselines/reference/typeOnlyESMImportFromCJS.errors.txt‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
common.cts(1,21): error TS1541: Type-only import of an ECMAScript module from a CommonJS module must have a 'resolution-mode' attribute.
2+
common.cts(4,25): error TS1542: Type import of an ECMAScript module from a CommonJS module must have a 'resolution-mode' attribute.
3+
4+
5+
==== module.mts (0 errors) ====
6+
export {};
7+
8+
==== common.cts (2 errors) ====
9+
import type {} from "./module.mts";
10+
~~~~~~~~~~~~~~
11+
!!! error TS1541: Type-only import of an ECMAScript module from a CommonJS module must have a 'resolution-mode' attribute.
12+
import type {} from "./module.mts" with { "resolution-mode": "import" };
13+
import type {} from "./module.mts" with { "resolution-mode": "require" };
14+
type _1 = typeof import("./module.mts");
15+
~~~~~~~~~~~~~~
16+
!!! error TS1542: Type import of an ECMAScript module from a CommonJS module must have a 'resolution-mode' attribute.
17+
type _2 = typeof import("./module.mts", { with: { "resolution-mode": "import" } });
18+
type _3 = typeof import("./module.mts", { with: { "resolution-mode": "require" } });
19+

‎tests/baselines/reference/typeOnlyESMImportFromCJS.js‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
//// [tests/cases/conformance/externalModules/typeOnly/typeOnlyESMImportFromCJS.ts] ////
2+
3+
//// [module.mts]
4+
export {};
5+
6+
//// [common.cts]
7+
import type {} from "./module.mts";
8+
import type {} from "./module.mts" with { "resolution-mode": "import" };
9+
import type {} from "./module.mts" with { "resolution-mode": "require" };
10+
type _1 = typeof import("./module.mts");
11+
type _2 = typeof import("./module.mts", { with: { "resolution-mode": "import" } });
12+
type _3 = typeof import("./module.mts", { with: { "resolution-mode": "require" } });
13+
14+
15+
//// [module.mjs]
16+
export {};
17+
//// [common.cjs]
18+
"use strict";
19+
Object.defineProperty(exports, "__esModule", { value: true });

‎tests/baselines/reference/typeOnlyESMImportFromCJS.symbols‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
//// [tests/cases/conformance/externalModules/typeOnly/typeOnlyESMImportFromCJS.ts] ////
2+
3+
=== module.mts ===
4+
5+
export {};
6+
7+
=== common.cts ===
8+
import type {} from "./module.mts";
9+
import type {} from "./module.mts" with { "resolution-mode": "import" };
10+
import type {} from "./module.mts" with { "resolution-mode": "require" };
11+
type _1 = typeof import("./module.mts");
12+
>_1 : Symbol(_1, Decl(common.cts, 2, 73))
13+
14+
type _2 = typeof import("./module.mts", { with: { "resolution-mode": "import" } });
15+
>_2 : Symbol(_2, Decl(common.cts, 3, 40))
16+
17+
type _3 = typeof import("./module.mts", { with: { "resolution-mode": "require" } });
18+
>_3 : Symbol(_3, Decl(common.cts, 4, 83))
19+

‎tests/baselines/reference/typeOnlyESMImportFromCJS.types‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
//// [tests/cases/conformance/externalModules/typeOnly/typeOnlyESMImportFromCJS.ts] ////
2+
3+
=== module.mts ===
4+
5+
export {};
6+
7+
=== common.cts ===
8+
import type {} from "./module.mts";
9+
import type {} from "./module.mts" with { "resolution-mode": "import" };
10+
import type {} from "./module.mts" with { "resolution-mode": "require" };
11+
type _1 = typeof import("./module.mts");
12+
>_1 : typeof import("module", { with: { "resolution-mode": "import" } })
13+
> : ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
14+
15+
type _2 = typeof import("./module.mts", { with: { "resolution-mode": "import" } });
16+
>_2 : typeof import("module", { with: { "resolution-mode": "import" } })
17+
> : ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
18+
19+
type _3 = typeof import("./module.mts", { with: { "resolution-mode": "require" } });
20+
>_3 : typeof import("module", { with: { "resolution-mode": "import" } })
21+
> : ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
22+

0 commit comments

Comments
 (0)