Skip to content

Commit

Permalink
Update error messages for CJS imports resolving to ES modules
Browse files Browse the repository at this point in the history
  • Loading branch information
andrewbranch committed Jul 28, 2022
1 parent b7355e3 commit 07d1922
Show file tree
Hide file tree
Showing 57 changed files with 758 additions and 418 deletions.
27 changes: 26 additions & 1 deletion src/compiler/checker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3581,7 +3581,32 @@ namespace ts {
// An override clause will take effect for type-only imports and import types, and allows importing the types across formats, regardless of
// normal mode restrictions
if (isSyncImport && sourceFile.impliedNodeFormat === ModuleKind.ESNext && !getResolutionModeOverrideForClause(overrideClause)) {
error(errorNode, Diagnostics.Module_0_cannot_be_imported_using_this_construct_The_specifier_only_resolves_to_an_ES_module_which_cannot_be_imported_synchronously_Use_dynamic_import_instead, moduleReference);
if (findAncestor(location, isImportEqualsDeclaration)) {
// ImportEquals in a ESM file resolving to another ESM file
error(errorNode, Diagnostics.Module_0_cannot_be_imported_using_this_construct_The_specifier_only_resolves_to_an_ES_module_which_cannot_be_imported_with_require_Use_an_ECMAScript_import_instead, moduleReference);
}
else {
// CJS file resolving to an ESM file
const diag = error(errorNode, 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);
const ext = tryGetExtensionFromPath(currentSourceFile.fileName);
if (ext === Extension.Ts || ext === Extension.Js) {
const scope = host.getPackageScopeForPath(currentSourceFile.path);
const targetExt = ext === Extension.Ts ? Extension.Mts : Extension.Mjs;
if (scope && !scope.packageJsonContent.type) {
addRelatedInfo(diag, createDiagnosticForNode(
errorNode,
Diagnostics.To_convert_this_file_to_an_ECMAScript_module_change_its_file_extension_to_0_or_add_type_Colon_module_to_its_package_json_file_with_path_1,
targetExt,
combinePaths(scope.packageDirectory, "package.json")));
}
else {
addRelatedInfo(diag, createDiagnosticForNode(
errorNode,
Diagnostics.To_convert_this_file_to_an_ECMAScript_module_change_its_file_extension_to_0,
targetExt));
}
}
}
}
}
// merged symbol is module declaration symbol combined with all augmentations
Expand Down
14 changes: 13 additions & 1 deletion src/compiler/diagnosticMessages.json
Original file line number Diff line number Diff line change
Expand Up @@ -1469,7 +1469,7 @@
"category": "Error",
"code": 1470
},
"Module '{0}' cannot be imported using this construct. The specifier only resolves to an ES module, which cannot be imported synchronously. Use dynamic import instead.": {
"Module '{0}' cannot be imported using this construct. The specifier only resolves to an ES module, which cannot be imported with 'require'. Use an ECMAScript import instead.": {
"category": "Error",
"code": 1471
},
Expand Down Expand Up @@ -1501,6 +1501,18 @@
"category": "Error",
"code": 1478
},
"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.": {
"category": "Error",
"code": 1479
},
"To convert this file to an ECMAScript module, change its file extension to '{0}'.": {
"category": "Message",
"code": 1480
},
"To convert this file to an ECMAScript module, change its file extension to '{0}', or add `\"type\": \"module\"` to its package.json file with path '{1}'.": {
"category": "Message",
"code": 1481
},

"The types of '{0}' are incompatible between these types.": {
"category": "Error",
Expand Down
2 changes: 1 addition & 1 deletion src/compiler/moduleNameResolver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1790,7 +1790,7 @@ namespace ts {
}

/*@internal*/
interface PackageJsonInfo {
export interface PackageJsonInfo {
packageDirectory: string;
packageJsonContent: PackageJsonPathFields;
versionPaths: VersionPaths | undefined;
Expand Down
1 change: 1 addition & 0 deletions src/compiler/program.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1325,6 +1325,7 @@ namespace ts {
directoryExists,
getSymlinkCache,
realpath: host.realpath?.bind(host),
getPackageScopeForPath: path => getPackageScopeForPath(path, moduleResolutionCache?.getPackageJsonInfoCache(), host, options),
useCaseSensitiveFileNames: () => host.useCaseSensitiveFileNames(),
getFileIncludeReasons: () => fileReasons,
structureIsReused,
Expand Down
1 change: 1 addition & 0 deletions src/compiler/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4353,6 +4353,7 @@ namespace ts {
getResolvedTypeReferenceDirectives(): ModeAwareCache<ResolvedTypeReferenceDirective | undefined>;
getProjectReferenceRedirect(fileName: string): string | undefined;
isSourceOfProjectReferenceRedirect(fileName: string): boolean;
getPackageScopeForPath(path: Path): PackageJsonInfo | undefined;

readonly redirectTargetsMap: RedirectTargetsMap;
}
Expand Down
4 changes: 2 additions & 2 deletions src/server/editorServices.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4185,11 +4185,11 @@ namespace ts.server {
}

/*@internal*/
getPackageJsonsVisibleToFile(fileName: string, rootDir?: string): readonly PackageJsonInfo[] {
getPackageJsonsVisibleToFile(fileName: string, rootDir?: string): readonly ProjectPackageJsonInfo[] {
const packageJsonCache = this.packageJsonCache;
const rootPath = rootDir && this.toPath(rootDir);
const filePath = this.toPath(fileName);
const result: PackageJsonInfo[] = [];
const result: ProjectPackageJsonInfo[] = [];
const processDirectory = (directory: Path): boolean | undefined => {
switch (packageJsonCache.directoryHasPackageJson(directory)) {
// Sync and check same directory again
Expand Down
8 changes: 4 additions & 4 deletions src/server/packageJsonCache.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,16 +2,16 @@
namespace ts.server {
export interface PackageJsonCache {
addOrUpdate(fileName: Path): void;
forEach(action: (info: PackageJsonInfo, fileName: Path) => void): void;
forEach(action: (info: ProjectPackageJsonInfo, fileName: Path) => void): void;
delete(fileName: Path): void;
get(fileName: Path): PackageJsonInfo | false | undefined;
getInDirectory(directory: Path): PackageJsonInfo | undefined;
get(fileName: Path): ProjectPackageJsonInfo | false | undefined;
getInDirectory(directory: Path): ProjectPackageJsonInfo | undefined;
directoryHasPackageJson(directory: Path): Ternary;
searchDirectoryAndAncestors(directory: Path): void;
}

export function createPackageJsonCache(host: ProjectService): PackageJsonCache {
const packageJsons = new Map<string, PackageJsonInfo>();
const packageJsons = new Map<string, ProjectPackageJsonInfo>();
const directoriesWithoutPackageJson = new Map<string, true>();
return {
addOrUpdate,
Expand Down
4 changes: 2 additions & 2 deletions src/server/project.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1751,7 +1751,7 @@ namespace ts.server {
}

/*@internal*/
getPackageJsonsVisibleToFile(fileName: string, rootDir?: string): readonly PackageJsonInfo[] {
getPackageJsonsVisibleToFile(fileName: string, rootDir?: string): readonly ProjectPackageJsonInfo[] {
if (this.projectService.serverMode !== LanguageServiceMode.Semantic) return emptyArray;
return this.projectService.getPackageJsonsVisibleToFile(fileName, rootDir);
}
Expand All @@ -1762,7 +1762,7 @@ namespace ts.server {
}

/*@internal*/
getPackageJsonsForAutoImport(rootDir?: string): readonly PackageJsonInfo[] {
getPackageJsonsForAutoImport(rootDir?: string): readonly ProjectPackageJsonInfo[] {
const packageJsons = this.getPackageJsonsVisibleToFile(combinePaths(this.currentDirectory, inferredTypesContainingFile), rootDir);
this.packageJsonsForAutoImport = new Set(packageJsons.map(p => p.fileName));
return packageJsons;
Expand Down
6 changes: 3 additions & 3 deletions src/services/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,7 @@ namespace ts {
}

/* @internal */
export interface PackageJsonInfo {
export interface ProjectPackageJsonInfo {
fileName: string;
parseable: boolean;
dependencies?: ESMap<string, string>;
Expand Down Expand Up @@ -311,9 +311,9 @@ namespace ts {

/* @internal */ getDocumentPositionMapper?(generatedFileName: string, sourceFileName?: string): DocumentPositionMapper | undefined;
/* @internal */ getSourceFileLike?(fileName: string): SourceFileLike | undefined;
/* @internal */ getPackageJsonsVisibleToFile?(fileName: string, rootDir?: string): readonly PackageJsonInfo[];
/* @internal */ getPackageJsonsVisibleToFile?(fileName: string, rootDir?: string): readonly ProjectPackageJsonInfo[];
/* @internal */ getNearestAncestorDirectoryWithPackageJson?(fileName: string): string | undefined;
/* @internal */ getPackageJsonsForAutoImport?(rootDir?: string): readonly PackageJsonInfo[];
/* @internal */ getPackageJsonsForAutoImport?(rootDir?: string): readonly ProjectPackageJsonInfo[];
/* @internal */ getCachedExportInfoMap?(): ExportInfoMap;
/* @internal */ getModuleSpecifierCache?(): ModuleSpecifierCache;
/* @internal */ setCompilerHost?(host: CompilerHost): void;
Expand Down
8 changes: 4 additions & 4 deletions src/services/utilities.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3004,12 +3004,12 @@ namespace ts {
return packageJson;
}

export function getPackageJsonsVisibleToFile(fileName: string, host: LanguageServiceHost): readonly PackageJsonInfo[] {
export function getPackageJsonsVisibleToFile(fileName: string, host: LanguageServiceHost): readonly ProjectPackageJsonInfo[] {
if (!host.fileExists) {
return [];
}

const packageJsons: PackageJsonInfo[] = [];
const packageJsons: ProjectPackageJsonInfo[] = [];
forEachAncestorDirectory(getDirectoryPath(fileName), ancestor => {
const packageJsonFileName = combinePaths(ancestor, "package.json");
if (host.fileExists(packageJsonFileName)) {
Expand All @@ -3023,7 +3023,7 @@ namespace ts {
return packageJsons;
}

export function createPackageJsonInfo(fileName: string, host: { readFile?(fileName: string): string | undefined }): PackageJsonInfo | undefined {
export function createPackageJsonInfo(fileName: string, host: { readFile?(fileName: string): string | undefined }): ProjectPackageJsonInfo | undefined {
if (!host.readFile) {
return undefined;
}
Expand All @@ -3032,7 +3032,7 @@ namespace ts {
const dependencyKeys = ["dependencies", "devDependencies", "optionalDependencies", "peerDependencies"] as const;
const stringContent = host.readFile(fileName) || "";
const content = tryParseJson(stringContent) as PackageJsonRaw | undefined;
const info: Pick<PackageJsonInfo, typeof dependencyKeys[number]> = {};
const info: Pick<ProjectPackageJsonInfo, typeof dependencyKeys[number]> = {};
if (content) {
for (const key of dependencyKeys) {
const dependencies = content[key];
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
error TS2209: The project root is ambiguous, but is required to resolve export map entry '.' in file 'tests/cases/conformance/node/allowJs/package.json'. Supply the `rootDir` compiler option to disambiguate.
tests/cases/conformance/node/allowJs/index.cjs(2,23): error TS1471: Module 'package' cannot be imported using this construct. The specifier only resolves to an ES module, which cannot be imported synchronously. Use dynamic import instead.
tests/cases/conformance/node/allowJs/index.cjs(2,23): 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("package")' call instead.


!!! error TS2209: The project root is ambiguous, but is required to resolve export map entry '.' in file 'tests/cases/conformance/node/allowJs/package.json'. Supply the `rootDir` compiler option to disambiguate.
Expand All @@ -15,7 +15,7 @@ tests/cases/conformance/node/allowJs/index.cjs(2,23): error TS1471: Module 'pack
// esm format file
import * as self from "package";
~~~~~~~~~
!!! error TS1471: Module 'package' cannot be imported using this construct. The specifier only resolves to an ES module, which cannot be imported synchronously. Use dynamic import instead.
!!! 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("package")' call instead.
self;
==== tests/cases/conformance/node/allowJs/package.json (0 errors) ====
{
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
error TS2209: The project root is ambiguous, but is required to resolve export map entry '.' in file 'tests/cases/conformance/node/allowJs/package.json'. Supply the `rootDir` compiler option to disambiguate.
tests/cases/conformance/node/allowJs/index.cjs(2,23): error TS1471: Module 'package' cannot be imported using this construct. The specifier only resolves to an ES module, which cannot be imported synchronously. Use dynamic import instead.
tests/cases/conformance/node/allowJs/index.cjs(2,23): 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("package")' call instead.


!!! error TS2209: The project root is ambiguous, but is required to resolve export map entry '.' in file 'tests/cases/conformance/node/allowJs/package.json'. Supply the `rootDir` compiler option to disambiguate.
Expand All @@ -15,7 +15,7 @@ tests/cases/conformance/node/allowJs/index.cjs(2,23): error TS1471: Module 'pack
// esm format file
import * as self from "package";
~~~~~~~~~
!!! error TS1471: Module 'package' cannot be imported using this construct. The specifier only resolves to an ES module, which cannot be imported synchronously. Use dynamic import instead.
!!! 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("package")' call instead.
self;
==== tests/cases/conformance/node/allowJs/package.json (0 errors) ====
{
Expand Down
Loading

0 comments on commit 07d1922

Please sign in to comment.