Skip to content

Commit

Permalink
Improve errors on module: node12 and extensionless relative imports (#…
Browse files Browse the repository at this point in the history
…46486)

* add new error + suggestions

* push down assert defined

* remove comment

* fix esm module import detection, update baselines

* add and update new tests

* fix review comments

* remove renamed baseline references

* update node modules test baselines

* fix error message, add new tests

* update old tests with new error message
  • Loading branch information
gabritto authored Oct 29, 2021
1 parent f1a69ec commit 9cdbb72
Show file tree
Hide file tree
Showing 47 changed files with 854 additions and 404 deletions.
38 changes: 35 additions & 3 deletions src/compiler/checker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1016,6 +1016,22 @@ namespace ts {
const builtinGlobals = createSymbolTable();
builtinGlobals.set(undefinedSymbol.escapedName, undefinedSymbol);

// Extensions suggested for path imports when module resolution is node12 or higher.
// The first element of each tuple is the extension a file has.
// The second element of each tuple is the extension that should be used in a path import.
// e.g. if we want to import file `foo.mts`, we should write `import {} from "./foo.mjs".
const suggestedExtensions: [string, string][] = [
[".mts", ".mjs"],
[".ts", ".js"],
[".cts", ".cjs"],
[".mjs", ".mjs"],
[".js", ".js"],
[".cjs", ".cjs"],
[".tsx", compilerOptions.jsx === JsxEmit.Preserve ? ".jsx" : ".js"],
[".jsx", ".jsx"],
[".json", ".json"],
];

initializeTypeChecker();

return checker;
Expand Down Expand Up @@ -3443,7 +3459,7 @@ namespace ts {
(isModuleDeclaration(location) ? location : location.parent && isModuleDeclaration(location.parent) && location.parent.name === location ? location.parent : undefined)?.name ||
(isLiteralImportTypeNode(location) ? location : undefined)?.argument.literal;
const mode = contextSpecifier && isStringLiteralLike(contextSpecifier) ? getModeForUsageLocation(currentSourceFile, contextSpecifier) : currentSourceFile.impliedNodeFormat;
const resolvedModule = getResolvedModule(currentSourceFile, moduleReference, mode)!; // TODO: GH#18217
const resolvedModule = getResolvedModule(currentSourceFile, moduleReference, mode);
const resolutionDiagnostic = resolvedModule && getResolutionDiagnostic(compilerOptions, resolvedModule);
const sourceFile = resolvedModule && !resolutionDiagnostic && host.getSourceFile(resolvedModule.resolvedFileName);
if (sourceFile) {
Expand Down Expand Up @@ -3489,10 +3505,10 @@ namespace ts {
if (resolvedModule && !resolutionExtensionIsTSOrJson(resolvedModule.extension) && resolutionDiagnostic === undefined || resolutionDiagnostic === Diagnostics.Could_not_find_a_declaration_file_for_module_0_1_implicitly_has_an_any_type) {
if (isForAugmentation) {
const diag = Diagnostics.Invalid_module_name_in_augmentation_Module_0_resolves_to_an_untyped_module_at_1_which_cannot_be_augmented;
error(errorNode, diag, moduleReference, resolvedModule.resolvedFileName);
error(errorNode, diag, moduleReference, resolvedModule!.resolvedFileName);
}
else {
errorOnImplicitAnyModule(/*isError*/ noImplicitAny && !!moduleNotFoundError, errorNode, resolvedModule, moduleReference);
errorOnImplicitAnyModule(/*isError*/ noImplicitAny && !!moduleNotFoundError, errorNode, resolvedModule!, moduleReference);
}
// Failed imports and untyped modules are both treated in an untyped manner; only difference is whether we give a diagnostic first.
return undefined;
Expand All @@ -3513,6 +3529,10 @@ namespace ts {
}
else {
const tsExtension = tryExtractTSExtension(moduleReference);
const isExtensionlessRelativePathImport = pathIsRelative(moduleReference) && !hasExtension(moduleReference);
const moduleResolutionKind = getEmitModuleResolutionKind(compilerOptions);
const resolutionIsNode12OrNext = moduleResolutionKind === ModuleResolutionKind.Node12 ||
moduleResolutionKind === ModuleResolutionKind.NodeNext;
if (tsExtension) {
const diag = Diagnostics.An_import_path_cannot_end_with_a_0_extension_Consider_importing_1_instead;
const importSourceWithoutExtension = removeExtension(moduleReference, tsExtension);
Expand All @@ -3532,6 +3552,18 @@ namespace ts {
hasJsonModuleEmitEnabled(compilerOptions)) {
error(errorNode, Diagnostics.Cannot_find_module_0_Consider_using_resolveJsonModule_to_import_module_with_json_extension, moduleReference);
}
else if (mode === ModuleKind.ESNext && resolutionIsNode12OrNext && isExtensionlessRelativePathImport) {
const absoluteRef = getNormalizedAbsolutePath(moduleReference, getDirectoryPath(currentSourceFile.path));
const suggestedExt = suggestedExtensions.find(([actualExt, _importExt]) => host.fileExists(absoluteRef + actualExt))?.[1];
if (suggestedExt) {
error(errorNode,
Diagnostics.Relative_import_paths_need_explicit_file_extensions_in_EcmaScript_imports_when_moduleResolution_is_node12_or_nodenext_Did_you_mean_0,
moduleReference + suggestedExt);
}
else {
error(errorNode, Diagnostics.Relative_import_paths_need_explicit_file_extensions_in_EcmaScript_imports_when_moduleResolution_is_node12_or_nodenext_Consider_adding_an_extension_to_the_import_path);
}
}
else {
error(errorNode, moduleNotFoundError, moduleReference);
}
Expand Down
8 changes: 8 additions & 0 deletions src/compiler/diagnosticMessages.json
Original file line number Diff line number Diff line change
Expand Up @@ -3345,6 +3345,14 @@
"category": "Error",
"code": 2833
},
"Relative import paths need explicit file extensions in EcmaScript imports when '--moduleResolution' is 'node12' or 'nodenext'. Consider adding an extension to the import path.": {
"category": "Error",
"code": 2834
},
"Relative import paths need explicit file extensions in EcmaScript imports when '--moduleResolution' is 'node12' or 'nodenext'. Did you mean '{0}'?": {
"category": "Error",
"code": 2835
},

"Import declaration '{0}' is using private name '{1}'.": {
"category": "Error",
Expand Down
2 changes: 1 addition & 1 deletion src/compiler/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5992,7 +5992,7 @@ namespace ts {
export enum ModuleResolutionKind {
Classic = 1,
NodeJs = 2,
// Starting with node12, node's module resolver has significant departures from tranditional cjs resolution
// Starting with node12, node's module resolver has significant departures from traditional cjs resolution
// to better support ecmascript modules and their use within node - more features are still being added, so
// we can expect it to change over time, and as such, offer both a `NodeNext` moving resolution target, and a `Node12`
// version-anchored resolution target
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
/src/bar.mts(2,21): error TS2835: Relative import paths need explicit file extensions in EcmaScript imports when '--moduleResolution' is 'node12' or 'nodenext'. Did you mean './foo.mjs'?
/src/bar.mts(3,21): error TS2834: Relative import paths need explicit file extensions in EcmaScript imports when '--moduleResolution' is 'node12' or 'nodenext'. Consider adding an extension to the import path.


==== /src/foo.mts (0 errors) ====
export function foo() {
return "";
}

==== /src/bar.mts (2 errors) ====
// Extensionless relative path ES import in an ES module
import { foo } from "./foo"; // should error, suggest adding ".mjs"
~~~~~~~
!!! error TS2835: Relative import paths need explicit file extensions in EcmaScript imports when '--moduleResolution' is 'node12' or 'nodenext'. Did you mean './foo.mjs'?
import { baz } from "./baz"; // should error, ask for extension, no extension suggestion
~~~~~~~
!!! error TS2834: Relative import paths need explicit file extensions in EcmaScript imports when '--moduleResolution' is 'node12' or 'nodenext'. Consider adding an extension to the import path.

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

//// [foo.mts]
export function foo() {
return "";
}

//// [bar.mts]
// Extensionless relative path ES import in an ES module
import { foo } from "./foo"; // should error, suggest adding ".mjs"
import { baz } from "./baz"; // should error, ask for extension, no extension suggestion


//// [foo.mjs]
export function foo() {
return "";
}
//// [bar.mjs]
export {};
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
=== /src/foo.mts ===
export function foo() {
>foo : Symbol(foo, Decl(foo.mts, 0, 0))

return "";
}

=== /src/bar.mts ===
// Extensionless relative path ES import in an ES module
import { foo } from "./foo"; // should error, suggest adding ".mjs"
>foo : Symbol(foo, Decl(bar.mts, 1, 8))

import { baz } from "./baz"; // should error, ask for extension, no extension suggestion
>baz : Symbol(baz, Decl(bar.mts, 2, 8))

16 changes: 16 additions & 0 deletions tests/baselines/reference/moduleResolutionWithoutExtension1.types
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
=== /src/foo.mts ===
export function foo() {
>foo : () => string

return "";
>"" : ""
}

=== /src/bar.mts ===
// Extensionless relative path ES import in an ES module
import { foo } from "./foo"; // should error, suggest adding ".mjs"
>foo : any

import { baz } from "./baz"; // should error, ask for extension, no extension suggestion
>baz : any

Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
/src/buzz.mts(2,22): error TS2307: Cannot find module './foo' or its corresponding type declarations.


==== /src/buzz.mts (1 errors) ====
// Extensionless relative path cjs import in an ES module
import foo = require("./foo"); // should error, should not ask for extension
~~~~~~~
!!! error TS2307: Cannot find module './foo' or its corresponding type declarations.
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
//// [buzz.mts]
// Extensionless relative path cjs import in an ES module
import foo = require("./foo"); // should error, should not ask for extension

//// [buzz.mjs]
export {};
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
=== /src/buzz.mts ===
// Extensionless relative path cjs import in an ES module
import foo = require("./foo"); // should error, should not ask for extension
>foo : Symbol(foo, Decl(buzz.mts, 0, 0))

Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
=== /src/buzz.mts ===
// Extensionless relative path cjs import in an ES module
import foo = require("./foo"); // should error, should not ask for extension
>foo : any

Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
/src/bar.mts(2,21): error TS2835: Relative import paths need explicit file extensions in EcmaScript imports when '--moduleResolution' is 'node12' or 'nodenext'. Did you mean './foo.jsx'?


==== /src/foo.tsx (0 errors) ====
export function foo() {
return "";
}

==== /src/bar.mts (1 errors) ====
// Extensionless relative path ES import in an ES module
import { foo } from "./foo"; // should error, suggest adding ".jsx"
~~~~~~~
!!! error TS2835: Relative import paths need explicit file extensions in EcmaScript imports when '--moduleResolution' is 'node12' or 'nodenext'. Did you mean './foo.jsx'?

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

//// [foo.tsx]
export function foo() {
return "";
}

//// [bar.mts]
// Extensionless relative path ES import in an ES module
import { foo } from "./foo"; // should error, suggest adding ".jsx"


//// [foo.jsx]
"use strict";
Object.defineProperty(exports, "__esModule", { value: true });
exports.foo = void 0;
function foo() {
return "";
}
exports.foo = foo;
//// [bar.mjs]
export {};
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
=== /src/foo.tsx ===
export function foo() {
>foo : Symbol(foo, Decl(foo.tsx, 0, 0))

return "";
}

=== /src/bar.mts ===
// Extensionless relative path ES import in an ES module
import { foo } from "./foo"; // should error, suggest adding ".jsx"
>foo : Symbol(foo, Decl(bar.mts, 1, 8))

13 changes: 13 additions & 0 deletions tests/baselines/reference/moduleResolutionWithoutExtension3.types
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
=== /src/foo.tsx ===
export function foo() {
>foo : () => string

return "";
>"" : ""
}

=== /src/bar.mts ===
// Extensionless relative path ES import in an ES module
import { foo } from "./foo"; // should error, suggest adding ".jsx"
>foo : any

Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
/src/bar.mts(2,21): error TS2835: Relative import paths need explicit file extensions in EcmaScript imports when '--moduleResolution' is 'node12' or 'nodenext'. Did you mean './foo.js'?


==== /src/foo.tsx (0 errors) ====
export function foo() {
return "";
}

==== /src/bar.mts (1 errors) ====
// Extensionless relative path ES import in an ES module
import { foo } from "./foo"; // should error, suggest adding ".js"
~~~~~~~
!!! error TS2835: Relative import paths need explicit file extensions in EcmaScript imports when '--moduleResolution' is 'node12' or 'nodenext'. Did you mean './foo.js'?

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

//// [foo.tsx]
export function foo() {
return "";
}

//// [bar.mts]
// Extensionless relative path ES import in an ES module
import { foo } from "./foo"; // should error, suggest adding ".js"


//// [foo.js]
"use strict";
Object.defineProperty(exports, "__esModule", { value: true });
exports.foo = void 0;
function foo() {
return "";
}
exports.foo = foo;
//// [bar.mjs]
export {};
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
=== /src/foo.tsx ===
export function foo() {
>foo : Symbol(foo, Decl(foo.tsx, 0, 0))

return "";
}

=== /src/bar.mts ===
// Extensionless relative path ES import in an ES module
import { foo } from "./foo"; // should error, suggest adding ".js"
>foo : Symbol(foo, Decl(bar.mts, 1, 8))

13 changes: 13 additions & 0 deletions tests/baselines/reference/moduleResolutionWithoutExtension4.types
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
=== /src/foo.tsx ===
export function foo() {
>foo : () => string

return "";
>"" : ""
}

=== /src/bar.mts ===
// Extensionless relative path ES import in an ES module
import { foo } from "./foo"; // should error, suggest adding ".js"
>foo : any

Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
/src/buzz.mts(2,8): error TS2834: Relative import paths need explicit file extensions in EcmaScript imports when '--moduleResolution' is 'node12' or 'nodenext'. Consider adding an extension to the import path.


==== /src/buzz.mts (1 errors) ====
// Extensionless relative path dynamic import in an ES module
import("./foo").then(x => x); // should error, ask for extension
~~~~~~~
!!! error TS2834: Relative import paths need explicit file extensions in EcmaScript imports when '--moduleResolution' is 'node12' or 'nodenext'. Consider adding an extension to the import path.
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
//// [buzz.mts]
// Extensionless relative path dynamic import in an ES module
import("./foo").then(x => x); // should error, ask for extension

//// [buzz.mjs]
// Extensionless relative path dynamic import in an ES module
import("./foo").then(x => x); // should error, ask for extension
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
=== /src/buzz.mts ===
// Extensionless relative path dynamic import in an ES module
import("./foo").then(x => x); // should error, ask for extension
>import("./foo").then : Symbol(Promise.then, Decl(lib.es5.d.ts, --, --))
>then : Symbol(Promise.then, Decl(lib.es5.d.ts, --, --))
>x : Symbol(x, Decl(buzz.mts, 1, 21))
>x : Symbol(x, Decl(buzz.mts, 1, 21))

12 changes: 12 additions & 0 deletions tests/baselines/reference/moduleResolutionWithoutExtension5.types
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
=== /src/buzz.mts ===
// Extensionless relative path dynamic import in an ES module
import("./foo").then(x => x); // should error, ask for extension
>import("./foo").then(x => x) : Promise<any>
>import("./foo").then : <TResult1 = any, TResult2 = never>(onfulfilled?: (value: any) => TResult1 | PromiseLike<TResult1>, onrejected?: (reason: any) => TResult2 | PromiseLike<TResult2>) => Promise<TResult1 | TResult2>
>import("./foo") : Promise<any>
>"./foo" : "./foo"
>then : <TResult1 = any, TResult2 = never>(onfulfilled?: (value: any) => TResult1 | PromiseLike<TResult1>, onrejected?: (reason: any) => TResult2 | PromiseLike<TResult2>) => Promise<TResult1 | TResult2>
>x => x : (x: any) => any
>x : any
>x : any

Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
/src/bar.cts(4,21): error TS2307: Cannot find module './foo' or its corresponding type declarations.


==== /src/bar.cts (1 errors) ====
// Extensionless relative path import statement in a cjs module
// Import statements are not allowed in cjs files,
// but other errors should not assume that they are allowed
import { foo } from "./foo"; // should error, should not ask for extension
~~~~~~~
!!! error TS2307: Cannot find module './foo' or its corresponding type declarations.
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
//// [bar.cts]
// Extensionless relative path import statement in a cjs module
// Import statements are not allowed in cjs files,
// but other errors should not assume that they are allowed
import { foo } from "./foo"; // should error, should not ask for extension

//// [bar.cjs]
"use strict";
Object.defineProperty(exports, "__esModule", { value: true });
Loading

0 comments on commit 9cdbb72

Please sign in to comment.