Skip to content

Commit

Permalink
Support resolution-mode overrides in type-only constructs in all modu…
Browse files Browse the repository at this point in the history
…leResolution modes (#55725)
  • Loading branch information
andrewbranch authored Sep 26, 2023
1 parent 2e9e9a2 commit 1b70ac3
Show file tree
Hide file tree
Showing 41 changed files with 1,708 additions and 121 deletions.
19 changes: 1 addition & 18 deletions src/compiler/checker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39766,16 +39766,7 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
checkSourceElement(node.argument);

if (node.attributes) {
const override = getResolutionModeOverride(node.attributes, grammarErrorOnNode);
const errorNode = node.attributes;
if (override && errorNode && getEmitModuleResolutionKind(compilerOptions) !== ModuleResolutionKind.Node16 && getEmitModuleResolutionKind(compilerOptions) !== ModuleResolutionKind.NodeNext) {
grammarErrorOnNode(
errorNode,
node.attributes.token === SyntaxKind.WithKeyword
? Diagnostics.The_resolution_mode_attribute_is_only_supported_when_moduleResolution_is_node16_or_nodenext
: Diagnostics.resolution_mode_assertions_are_only_supported_when_moduleResolution_is_node16_or_nodenext,
);
}
getResolutionModeOverride(node.attributes, grammarErrorOnNode);
}
checkTypeReferenceOrImport(node);
}
Expand Down Expand Up @@ -45212,14 +45203,6 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
const override = getResolutionModeOverride(node, validForTypeAttributes ? grammarErrorOnNode : undefined);
const isImportAttributes = declaration.attributes.token === SyntaxKind.WithKeyword;
if (validForTypeAttributes && override) {
if (getEmitModuleResolutionKind(compilerOptions) !== ModuleResolutionKind.Node16 && getEmitModuleResolutionKind(compilerOptions) !== ModuleResolutionKind.NodeNext) {
return grammarErrorOnNode(
node,
isImportAttributes
? Diagnostics.The_resolution_mode_attribute_is_only_supported_when_moduleResolution_is_node16_or_nodenext
: Diagnostics.resolution_mode_assertions_are_only_supported_when_moduleResolution_is_node16_or_nodenext,
);
}
return; // Other grammar checks do not apply to type-only imports with resolution mode assertions
}

Expand Down
8 changes: 0 additions & 8 deletions src/compiler/diagnosticMessages.json
Original file line number Diff line number Diff line change
Expand Up @@ -1480,10 +1480,6 @@
"category": "Error",
"code": 1451
},
"'resolution-mode' assertions are only supported when `moduleResolution` is `node16` or `nodenext`.": {
"category": "Error",
"code": 1452
},
"`resolution-mode` should be either `require` or `import`.": {
"category": "Error",
"code": 1453
Expand Down Expand Up @@ -1520,10 +1516,6 @@
"category": "Message",
"code": 1461
},
"The 'resolution-mode' attribute is only supported when 'moduleResolution' is 'node16' or 'nodenext'.": {
"category": "Error",
"code": 1462
},
"'resolution-mode' is the only valid key for type import attributes.": {
"category": "Error",
"code": 1463
Expand Down
103 changes: 71 additions & 32 deletions src/compiler/moduleNameResolver.ts

Large diffs are not rendered by default.

11 changes: 1 addition & 10 deletions src/compiler/program.ts
Original file line number Diff line number Diff line change
Expand Up @@ -865,7 +865,6 @@ export function getModeForResolutionAtIndex(file: SourceFile, index: number): Re
// eslint-disable-next-line @typescript-eslint/unified-signatures
export function getModeForResolutionAtIndex(file: SourceFileImportsList, index: number): ResolutionMode;
export function getModeForResolutionAtIndex(file: SourceFileImportsList, index: number): ResolutionMode {
if (file.impliedNodeFormat === undefined) return undefined;
// we ensure all elements of file.imports and file.moduleAugmentations have the relevant parent pointers set during program setup,
// so it's safe to use them even pre-bind
return getModeForUsageLocation(file, getModuleNameStringLiteralAt(file, index));
Expand All @@ -892,7 +891,6 @@ export function isExclusivelyTypeOnlyImportOrExport(decl: ImportDeclaration | Ex
* @returns The final resolution mode of the import
*/
export function getModeForUsageLocation(file: { impliedNodeFormat?: ResolutionMode; }, usage: StringLiteralLike) {
if (file.impliedNodeFormat === undefined) return undefined;
if ((isImportDeclaration(usage.parent) || isExportDeclaration(usage.parent))) {
const isTypeOnly = isExclusivelyTypeOnlyImportOrExport(usage.parent);
if (isTypeOnly) {
Expand All @@ -908,6 +906,7 @@ export function getModeForUsageLocation(file: { impliedNodeFormat?: ResolutionMo
return override;
}
}
if (file.impliedNodeFormat === undefined) return undefined;
if (file.impliedNodeFormat !== ModuleKind.ESNext) {
// in cjs files, import call expressions are esm format, otherwise everything is cjs
return isImportCall(walkUpParenthesizedExpressions(usage.parent)) ? ModuleKind.ESNext : ModuleKind.CommonJS;
Expand Down Expand Up @@ -3863,14 +3862,6 @@ export function createProgram(rootNamesOrOptions: readonly string[] | CreateProg
const fileName = toFileNameLowerCase(ref.fileName);
resolutionsInFile.set(fileName, getModeForFileReference(ref, file.impliedNodeFormat), resolvedTypeReferenceDirective);
const mode = ref.resolutionMode || file.impliedNodeFormat;
if (mode && getEmitModuleResolutionKind(options) !== ModuleResolutionKind.Node16 && getEmitModuleResolutionKind(options) !== ModuleResolutionKind.NodeNext) {
(fileProcessingDiagnostics ??= []).push({
kind: FilePreprocessingDiagnosticsKind.ResolutionDiagnostics,
diagnostics: [
createDiagnosticForRange(file, ref, Diagnostics.resolution_mode_assertions_are_only_supported_when_moduleResolution_is_node16_or_nodenext),
],
});
}
processTypeReferenceDirective(fileName, mode, resolvedTypeReferenceDirective, { kind: FileIncludeKind.TypeReferenceDirective, file: file.path, index });
}
}
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ declare global {
//// [index.ts]
/// <reference types="pkg" resolution-mode="require" />
/// <reference types="pkg" resolution-mode="import" />
foo; // `resolution-mode` is an error in old resolution settings, which resolves is arbitrary
foo;
bar;
export {};

Expand All @@ -31,5 +31,5 @@ export {};
Object.defineProperty(exports, "__esModule", { value: true });
/// <reference types="pkg" resolution-mode="require" />
/// <reference types="pkg" resolution-mode="import" />
foo; // `resolution-mode` is an error in old resolution settings, which resolves is arbitrary
foo;
bar;
Original file line number Diff line number Diff line change
@@ -1,9 +1,28 @@
//// [tests/cases/conformance/node/nodeModulesTripleSlashReferenceModeOverrideOldResolutionError.ts] ////

=== /index.ts ===

/// <reference types="pkg" resolution-mode="require" />
/// <reference types="pkg" resolution-mode="import" />
foo; // `resolution-mode` is an error in old resolution settings, which resolves is arbitrary
foo;
>foo : Symbol(foo, Decl(import.d.ts, 2, 7))

bar;
>bar : Symbol(bar, Decl(require.d.ts, 2, 7))

export {};
=== /node_modules/pkg/import.d.ts ===
export {};
declare global {
>global : Symbol(global, Decl(import.d.ts, 0, 10))

var foo: number;
>foo : Symbol(foo, Decl(import.d.ts, 2, 7))
}
=== /node_modules/pkg/require.d.ts ===
export {};
declare global {
>global : Symbol(global, Decl(require.d.ts, 0, 10))

var bar: number;
>bar : Symbol(bar, Decl(require.d.ts, 2, 7))
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,26 @@
=== /index.ts ===
/// <reference types="pkg" resolution-mode="require" />
/// <reference types="pkg" resolution-mode="import" />
foo; // `resolution-mode` is an error in old resolution settings, which resolves is arbitrary
>foo : any
foo;
>foo : number

bar;
>bar : any
>bar : number

export {};
=== /node_modules/pkg/import.d.ts ===
export {};
declare global {
>global : typeof global

var foo: number;
>foo : number
}
=== /node_modules/pkg/require.d.ts ===
export {};
declare global {
>global : typeof global

var bar: number;
>bar : number
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
//// [tests/cases/conformance/moduleResolution/resolutionModeImportType1.ts] ////

=== /node_modules/@types/foo/index.d.mts ===
export declare const x: "module";
>x : Symbol(x, Decl(index.d.mts, 0, 20))

=== /node_modules/@types/foo/index.d.cts ===
export declare const x: "script";
>x : Symbol(x, Decl(index.d.cts, 0, 20))

=== /app.ts ===
type Default = typeof import("foo").x;
>Default : Symbol(Default, Decl(app.ts, 0, 0))
>x : Symbol(x, Decl(index.d.mts, 0, 20))

type Import = typeof import("foo", { assert: { "resolution-mode": "import" } }).x;
>Import : Symbol(Import, Decl(app.ts, 0, 38))
>x : Symbol(x, Decl(index.d.mts, 0, 20))

type Require = typeof import("foo", { assert: { "resolution-mode": "require" } }).x;
>Require : Symbol(Require, Decl(app.ts, 1, 82))
>x : Symbol(x, Decl(index.d.cts, 0, 20))

// resolution-mode does not enforce file extension in `bundler`, just sets conditions
type ImportRelative = typeof import("./other", { assert: { "resolution-mode": "import" } }).x;
>ImportRelative : Symbol(ImportRelative, Decl(app.ts, 2, 84))
>x : Symbol(x, Decl(other.ts, 0, 12))

type RequireRelative = typeof import("./other", { assert: { "resolution-mode": "require" } }).x;
>RequireRelative : Symbol(RequireRelative, Decl(app.ts, 4, 94))
>x : Symbol(x, Decl(other.ts, 0, 12))

=== /other.ts ===
export const x = "other";
>x : Symbol(x, Decl(other.ts, 0, 12))

Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
//// [tests/cases/conformance/moduleResolution/resolutionModeImportType1.ts] ////

=== /node_modules/@types/foo/index.d.mts ===
export declare const x: "module";
>x : "module"

=== /node_modules/@types/foo/index.d.cts ===
export declare const x: "script";
>x : "script"

=== /app.ts ===
type Default = typeof import("foo").x;
>Default : "module"
>x : error

type Import = typeof import("foo", { assert: { "resolution-mode": "import" } }).x;
>Import : "module"
>x : error

type Require = typeof import("foo", { assert: { "resolution-mode": "require" } }).x;
>Require : "script"
>x : error

// resolution-mode does not enforce file extension in `bundler`, just sets conditions
type ImportRelative = typeof import("./other", { assert: { "resolution-mode": "import" } }).x;
>ImportRelative : "other"
>x : error

type RequireRelative = typeof import("./other", { assert: { "resolution-mode": "require" } }).x;
>RequireRelative : "other"
>x : error

=== /other.ts ===
export const x = "other";
>x : "other"
>"other" : "other"

Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
error TS2688: Cannot find type definition file for 'foo'.
The file is in the program because:
Entry point for implicit type library 'foo'
/app.ts(1,30): error TS2792: Cannot find module 'foo'. Did you mean to set the 'moduleResolution' option to 'nodenext', or to add aliases to the 'paths' option?
/app.ts(2,29): error TS2792: Cannot find module 'foo'. Did you mean to set the 'moduleResolution' option to 'nodenext', or to add aliases to the 'paths' option?
/app.ts(3,30): error TS2792: Cannot find module 'foo'. Did you mean to set the 'moduleResolution' option to 'nodenext', or to add aliases to the 'paths' option?


!!! error TS2688: Cannot find type definition file for 'foo'.
!!! error TS2688: The file is in the program because:
!!! error TS2688: Entry point for implicit type library 'foo'
==== /node_modules/@types/foo/package.json (0 errors) ====
{
"name": "@types/foo",
"version": "1.0.0",
"exports": {
".": {
"import": "./index.d.mts",
"require": "./index.d.cts"
}
}
}

==== /node_modules/@types/foo/index.d.mts (0 errors) ====
export declare const x: "module";

==== /node_modules/@types/foo/index.d.cts (0 errors) ====
export declare const x: "script";

==== /app.ts (3 errors) ====
type Default = typeof import("foo").x;
~~~~~
!!! error TS2792: Cannot find module 'foo'. Did you mean to set the 'moduleResolution' option to 'nodenext', or to add aliases to the 'paths' option?
type Import = typeof import("foo", { assert: { "resolution-mode": "import" } }).x;
~~~~~
!!! error TS2792: Cannot find module 'foo'. Did you mean to set the 'moduleResolution' option to 'nodenext', or to add aliases to the 'paths' option?
type Require = typeof import("foo", { assert: { "resolution-mode": "require" } }).x;
~~~~~
!!! error TS2792: Cannot find module 'foo'. Did you mean to set the 'moduleResolution' option to 'nodenext', or to add aliases to the 'paths' option?
// resolution-mode does not enforce file extension in `bundler`, just sets conditions
type ImportRelative = typeof import("./other", { assert: { "resolution-mode": "import" } }).x;
type RequireRelative = typeof import("./other", { assert: { "resolution-mode": "require" } }).x;

==== /other.ts (0 errors) ====
export const x = "other";

Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
//// [tests/cases/conformance/moduleResolution/resolutionModeImportType1.ts] ////

=== /node_modules/@types/foo/index.d.mts ===
export declare const x: "module";
>x : Symbol(x, Decl(index.d.mts, 0, 20))

=== /node_modules/@types/foo/index.d.cts ===
export declare const x: "script";
>x : Symbol(x, Decl(index.d.cts, 0, 20))

=== /app.ts ===
type Default = typeof import("foo").x;
>Default : Symbol(Default, Decl(app.ts, 0, 0))

type Import = typeof import("foo", { assert: { "resolution-mode": "import" } }).x;
>Import : Symbol(Import, Decl(app.ts, 0, 38))

type Require = typeof import("foo", { assert: { "resolution-mode": "require" } }).x;
>Require : Symbol(Require, Decl(app.ts, 1, 82))

// resolution-mode does not enforce file extension in `bundler`, just sets conditions
type ImportRelative = typeof import("./other", { assert: { "resolution-mode": "import" } }).x;
>ImportRelative : Symbol(ImportRelative, Decl(app.ts, 2, 84))
>x : Symbol(x, Decl(other.ts, 0, 12))

type RequireRelative = typeof import("./other", { assert: { "resolution-mode": "require" } }).x;
>RequireRelative : Symbol(RequireRelative, Decl(app.ts, 4, 94))
>x : Symbol(x, Decl(other.ts, 0, 12))

=== /other.ts ===
export const x = "other";
>x : Symbol(x, Decl(other.ts, 0, 12))

Loading

0 comments on commit 1b70ac3

Please sign in to comment.