Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix error message for type-only import of ES module from CJS #59711

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
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.
packages/pkg1/index.ts:1:29 - error TS1479: 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("pkg2")' call instead.
packages/pkg1/index.ts:1:29 - error TS1541: Type-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.
packages/pkg1/index.ts:1:29 - error TS1479: 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("pkg2")' call instead.
packages/pkg1/index.ts:1:29 - error TS1541: Type-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" } });`,
});