Skip to content

Commit bf7ca71

Browse files
rbucktonrbuckton
committed
Incorporate PR feedback, fix module resolution for import()
1 parent fc51c42 commit bf7ca71

29 files changed

+127
-91
lines changed

‎src/compiler/checker.ts

Lines changed: 20 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -933,6 +933,7 @@ namespace ts {
933933
let deferredGlobalTemplateStringsArrayType: ObjectType | undefined;
934934
let deferredGlobalImportMetaType: ObjectType;
935935
let deferredGlobalImportMetaExpressionType: ObjectType;
936+
let deferredGlobalImportCallOptionsType: ObjectType | undefined;
936937
let deferredGlobalExtractSymbol: Symbol | undefined;
937938
let deferredGlobalOmitSymbol: Symbol | undefined;
938939
let deferredGlobalAwaitedSymbol: Symbol | undefined;
@@ -13460,6 +13461,11 @@ namespace ts {
1346013461
return deferredGlobalImportMetaExpressionType;
1346113462
}
1346213463

13464+
function getGlobalImportCallOptionsType(reportErrors: boolean) {
13465+
// We always report an error, so store a result in the event we could not resolve the symbol to prevent reporting it multiple times
13466+
return (deferredGlobalImportCallOptionsType ||= getGlobalType("ImportCallOptions" as __String, /*arity*/ 0, reportErrors)) || emptyObjectType;
13467+
}
13468+
1346313469
function getGlobalESSymbolConstructorSymbol(reportErrors: boolean): Symbol | undefined {
1346413470
return deferredGlobalESSymbolConstructorSymbol ||= getGlobalValueSymbol("Symbol" as __String, reportErrors);
1346513471
}
@@ -30587,15 +30593,23 @@ namespace ts {
3058730593
}
3058830594
const specifier = node.arguments[0];
3058930595
const specifierType = checkExpressionCached(specifier);
30596+
const optionsType = node.arguments.length > 1 ? checkExpressionCached(node.arguments[1]) : undefined;
3059030597
// Even though multiple arguments is grammatically incorrect, type-check extra arguments for completion
30591-
for (let i = 1; i < node.arguments.length; ++i) {
30598+
for (let i = 2; i < node.arguments.length; ++i) {
3059230599
checkExpressionCached(node.arguments[i]);
3059330600
}
3059430601

3059530602
if (specifierType.flags & TypeFlags.Undefined || specifierType.flags & TypeFlags.Null || !isTypeAssignableTo(specifierType, stringType)) {
3059630603
error(specifier, Diagnostics.Dynamic_import_s_specifier_must_be_of_type_string_but_here_has_type_0, typeToString(specifierType));
3059730604
}
3059830605

30606+
if (optionsType) {
30607+
const importCallOptionsType = getGlobalImportCallOptionsType(/*reportErrors*/ true);
30608+
if (importCallOptionsType !== emptyObjectType) {
30609+
checkTypeAssignableTo(optionsType, getNullableType(importCallOptionsType, TypeFlags.Undefined), node.arguments[1]);
30610+
}
30611+
}
30612+
3059930613
// resolveExternalModuleName will return undefined if the moduleReferenceExpression is not a string literal
3060030614
const moduleSymbol = resolveExternalModuleName(node, specifier);
3060130615
if (moduleSymbol) {
@@ -43178,19 +43192,19 @@ namespace ts {
4317843192

4317943193
if (nodeArguments.length > 1) {
4318043194
const assertionArgument = nodeArguments[1];
43181-
return grammarErrorOnNode(assertionArgument, Diagnostics.Dynamic_import_only_supports_a_second_argument_when_the_module_option_is_set_to_esnext);
43195+
return grammarErrorOnNode(assertionArgument, Diagnostics.Dynamic_imports_only_support_a_second_argument_when_the_module_option_is_set_to_esnext);
4318243196
}
4318343197
}
4318443198

43185-
if (nodeArguments.length === 0 || nodeArguments.length > 2) {
43186-
return grammarErrorOnNode(node, Diagnostics.Dynamic_import_must_only_have_a_specifier_and_an_optional_assertion_as_arguments);
43199+
if (nodeArguments.length === 0 || nodeArguments.length > 2) {
43200+
return grammarErrorOnNode(node, Diagnostics.Dynamic_imports_can_only_accept_a_module_specifier_and_an_optional_assertion_as_arguments);
4318743201
}
4318843202

4318943203
// see: parseArgumentOrArrayLiteralElement...we use this function which parse arguments of callExpression to parse specifier for dynamic import.
4319043204
// parseArgumentOrArrayLiteralElement allows spread element to be in an argument list which is not allowed as specifier in dynamic import.
43191-
const spreadElement = forEach(nodeArguments, isSpreadElement);
43205+
const spreadElement = find(nodeArguments, isSpreadElement);
4319243206
if (spreadElement) {
43193-
return grammarErrorOnNode(nodeArguments[0], Diagnostics.Specifier_of_dynamic_import_cannot_be_spread_element);
43207+
return grammarErrorOnNode(spreadElement, Diagnostics.Argument_of_dynamic_import_cannot_be_spread_element);
4319443208
}
4319543209
return false;
4319643210
}

‎src/compiler/diagnosticMessages.json

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -924,11 +924,11 @@
924924
"category": "Error",
925925
"code": 1323
926926
},
927-
"Dynamic import only supports a second argument when the '--module' option is set to 'esnext'.": {
927+
"Dynamic imports only support a second argument when the '--module' option is set to 'esnext'.": {
928928
"category": "Error",
929929
"code": 1324
930930
},
931-
"Specifier of dynamic import cannot be spread element.": {
931+
"Argument of dynamic import cannot be spread element.": {
932932
"category": "Error",
933933
"code": 1325
934934
},
@@ -1388,7 +1388,7 @@
13881388
"category": "Message",
13891389
"code": 1449
13901390
},
1391-
"Dynamic import must only have a specifier and an optional assertion as arguments": {
1391+
"Dynamic imports can only accept a module specifier and an optional assertion as arguments": {
13921392
"category": "Message",
13931393
"code": 1450
13941394
},

‎src/compiler/program.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2374,8 +2374,8 @@ namespace ts {
23742374
if (isJavaScriptFile && isRequireCall(node, /*checkArgumentIsStringLiteralLike*/ true)) {
23752375
imports = append(imports, node.arguments[0]);
23762376
}
2377-
// we have to check the argument list has length of 1. We will still have to process these even though we have parsing error.
2378-
else if (isImportCall(node) && node.arguments.length === 1 && isStringLiteralLike(node.arguments[0])) {
2377+
// we have to check the argument list has length of at least 1. We will still have to process these even though we have parsing error.
2378+
else if (isImportCall(node) && node.arguments.length >= 1 && isStringLiteralLike(node.arguments[0])) {
23792379
imports = append(imports, node.arguments[0]);
23802380
}
23812381
else if (isLiteralImportTypeNode(node)) {

‎src/compiler/types.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7342,7 +7342,7 @@ namespace ts {
73427342
createImportEqualsDeclaration(decorators: readonly Decorator[] | undefined, modifiers: readonly Modifier[] | undefined, isTypeOnly: boolean, name: string | Identifier, moduleReference: ModuleReference): ImportEqualsDeclaration;
73437343
updateImportEqualsDeclaration(node: ImportEqualsDeclaration, decorators: readonly Decorator[] | undefined, modifiers: readonly Modifier[] | undefined, isTypeOnly: boolean, name: Identifier, moduleReference: ModuleReference): ImportEqualsDeclaration;
73447344
createImportDeclaration(decorators: readonly Decorator[] | undefined, modifiers: readonly Modifier[] | undefined, importClause: ImportClause | undefined, moduleSpecifier: Expression, assertClause?: AssertClause): ImportDeclaration;
7345-
updateImportDeclaration(node: ImportDeclaration, decorators: readonly Decorator[] | undefined, modifiers: readonly Modifier[] | undefined, importClause: ImportClause | undefined, moduleSpecifier: Expression, assertClause?: AssertClause): ImportDeclaration;
7345+
updateImportDeclaration(node: ImportDeclaration, decorators: readonly Decorator[] | undefined, modifiers: readonly Modifier[] | undefined, importClause: ImportClause | undefined, moduleSpecifier: Expression, assertClause: AssertClause | undefined): ImportDeclaration;
73467346
createImportClause(isTypeOnly: boolean, name: Identifier | undefined, namedBindings: NamedImportBindings | undefined): ImportClause;
73477347
updateImportClause(node: ImportClause, isTypeOnly: boolean, name: Identifier | undefined, namedBindings: NamedImportBindings | undefined): ImportClause;
73487348
createAssertClause(elements: NodeArray<AssertEntry>, multiLine?: boolean): AssertClause;
@@ -7360,7 +7360,7 @@ namespace ts {
73607360
createExportAssignment(decorators: readonly Decorator[] | undefined, modifiers: readonly Modifier[] | undefined, isExportEquals: boolean | undefined, expression: Expression): ExportAssignment;
73617361
updateExportAssignment(node: ExportAssignment, decorators: readonly Decorator[] | undefined, modifiers: readonly Modifier[] | undefined, expression: Expression): ExportAssignment;
73627362
createExportDeclaration(decorators: readonly Decorator[] | undefined, modifiers: readonly Modifier[] | undefined, isTypeOnly: boolean, exportClause: NamedExportBindings | undefined, moduleSpecifier?: Expression, assertClause?: AssertClause): ExportDeclaration;
7363-
updateExportDeclaration(node: ExportDeclaration, decorators: readonly Decorator[] | undefined, modifiers: readonly Modifier[] | undefined, isTypeOnly: boolean, exportClause: NamedExportBindings | undefined, moduleSpecifier: Expression | undefined, assertClause?: AssertClause): ExportDeclaration;
7363+
updateExportDeclaration(node: ExportDeclaration, decorators: readonly Decorator[] | undefined, modifiers: readonly Modifier[] | undefined, isTypeOnly: boolean, exportClause: NamedExportBindings | undefined, moduleSpecifier: Expression | undefined, assertClause: AssertClause | undefined): ExportDeclaration;
73647364
createNamedExports(elements: readonly ExportSpecifier[]): NamedExports;
73657365
updateNamedExports(node: NamedExports, elements: readonly ExportSpecifier[]): NamedExports;
73667366
createExportSpecifier(propertyName: string | Identifier | undefined, name: string | Identifier): ExportSpecifier;

‎src/harness/fourslashInterfaceImpl.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1061,6 +1061,8 @@ namespace FourSlashInterface {
10611061
interfaceEntry("NumberConstructor"),
10621062
interfaceEntry("TemplateStringsArray"),
10631063
interfaceEntry("ImportMeta"),
1064+
interfaceEntry("ImportCallOptions"),
1065+
interfaceEntry("ImportAssertions"),
10641066
varEntry("Math"),
10651067
varEntry("Date"),
10661068
interfaceEntry("DateConstructor"),

‎src/lib/es5.d.ts

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -597,6 +597,23 @@ interface TemplateStringsArray extends ReadonlyArray<string> {
597597
interface ImportMeta {
598598
}
599599

600+
/**
601+
* The type for the optional second argument to `import()`.
602+
*
603+
* If your host environment supports additional options, this type may be
604+
* augmented via interface merging.
605+
*/
606+
interface ImportCallOptions {
607+
assert?: ImportAssertions;
608+
}
609+
610+
/**
611+
* The type for the `assert` property of the optional second argument to `import()`.
612+
*/
613+
interface ImportAssertions {
614+
[key: string]: string;
615+
}
616+
600617
interface Math {
601618
/** The mathematical constant e. This is Euler's number, the base of natural logarithms. */
602619
readonly E: number;

‎tests/baselines/reference/api/tsserverlibrary.d.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3538,7 +3538,7 @@ declare namespace ts {
35383538
updateNamespaceExportDeclaration(node: NamespaceExportDeclaration, name: Identifier): NamespaceExportDeclaration;
35393539
createImportEqualsDeclaration(decorators: readonly Decorator[] | undefined, modifiers: readonly Modifier[] | undefined, isTypeOnly: boolean, name: string | Identifier, moduleReference: ModuleReference): ImportEqualsDeclaration;
35403540
updateImportEqualsDeclaration(node: ImportEqualsDeclaration, decorators: readonly Decorator[] | undefined, modifiers: readonly Modifier[] | undefined, isTypeOnly: boolean, name: Identifier, moduleReference: ModuleReference): ImportEqualsDeclaration;
3541-
createImportDeclaration(decorators: readonly Decorator[] | undefined, modifiers: readonly Modifier[] | undefined, importClause: ImportClause | undefined, moduleSpecifier: Expression, assertClause: AssertClause | undefined): ImportDeclaration;
3541+
createImportDeclaration(decorators: readonly Decorator[] | undefined, modifiers: readonly Modifier[] | undefined, importClause: ImportClause | undefined, moduleSpecifier: Expression, assertClause?: AssertClause): ImportDeclaration;
35423542
updateImportDeclaration(node: ImportDeclaration, decorators: readonly Decorator[] | undefined, modifiers: readonly Modifier[] | undefined, importClause: ImportClause | undefined, moduleSpecifier: Expression, assertClause: AssertClause | undefined): ImportDeclaration;
35433543
createImportClause(isTypeOnly: boolean, name: Identifier | undefined, namedBindings: NamedImportBindings | undefined): ImportClause;
35443544
updateImportClause(node: ImportClause, isTypeOnly: boolean, name: Identifier | undefined, namedBindings: NamedImportBindings | undefined): ImportClause;
@@ -10988,7 +10988,7 @@ declare namespace ts {
1098810988
/** @deprecated Use `factory.updateImportEqualsDeclaration` or the factory supplied by your transformation context instead. */
1098910989
const updateImportEqualsDeclaration: (node: ImportEqualsDeclaration, decorators: readonly Decorator[] | undefined, modifiers: readonly Modifier[] | undefined, isTypeOnly: boolean, name: Identifier, moduleReference: ModuleReference) => ImportEqualsDeclaration;
1099010990
/** @deprecated Use `factory.createImportDeclaration` or the factory supplied by your transformation context instead. */
10991-
const createImportDeclaration: (decorators: readonly Decorator[] | undefined, modifiers: readonly Modifier[] | undefined, importClause: ImportClause | undefined, moduleSpecifier: Expression, assertClause: AssertClause | undefined) => ImportDeclaration;
10991+
const createImportDeclaration: (decorators: readonly Decorator[] | undefined, modifiers: readonly Modifier[] | undefined, importClause: ImportClause | undefined, moduleSpecifier: Expression, assertClause?: AssertClause | undefined) => ImportDeclaration;
1099210992
/** @deprecated Use `factory.updateImportDeclaration` or the factory supplied by your transformation context instead. */
1099310993
const updateImportDeclaration: (node: ImportDeclaration, decorators: readonly Decorator[] | undefined, modifiers: readonly Modifier[] | undefined, importClause: ImportClause | undefined, moduleSpecifier: Expression, assertClause: AssertClause | undefined) => ImportDeclaration;
1099410994
/** @deprecated Use `factory.createNamespaceImport` or the factory supplied by your transformation context instead. */

‎tests/baselines/reference/api/typescript.d.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3538,7 +3538,7 @@ declare namespace ts {
35383538
updateNamespaceExportDeclaration(node: NamespaceExportDeclaration, name: Identifier): NamespaceExportDeclaration;
35393539
createImportEqualsDeclaration(decorators: readonly Decorator[] | undefined, modifiers: readonly Modifier[] | undefined, isTypeOnly: boolean, name: string | Identifier, moduleReference: ModuleReference): ImportEqualsDeclaration;
35403540
updateImportEqualsDeclaration(node: ImportEqualsDeclaration, decorators: readonly Decorator[] | undefined, modifiers: readonly Modifier[] | undefined, isTypeOnly: boolean, name: Identifier, moduleReference: ModuleReference): ImportEqualsDeclaration;
3541-
createImportDeclaration(decorators: readonly Decorator[] | undefined, modifiers: readonly Modifier[] | undefined, importClause: ImportClause | undefined, moduleSpecifier: Expression, assertClause: AssertClause | undefined): ImportDeclaration;
3541+
createImportDeclaration(decorators: readonly Decorator[] | undefined, modifiers: readonly Modifier[] | undefined, importClause: ImportClause | undefined, moduleSpecifier: Expression, assertClause?: AssertClause): ImportDeclaration;
35423542
updateImportDeclaration(node: ImportDeclaration, decorators: readonly Decorator[] | undefined, modifiers: readonly Modifier[] | undefined, importClause: ImportClause | undefined, moduleSpecifier: Expression, assertClause: AssertClause | undefined): ImportDeclaration;
35433543
createImportClause(isTypeOnly: boolean, name: Identifier | undefined, namedBindings: NamedImportBindings | undefined): ImportClause;
35443544
updateImportClause(node: ImportClause, isTypeOnly: boolean, name: Identifier | undefined, namedBindings: NamedImportBindings | undefined): ImportClause;
@@ -7188,7 +7188,7 @@ declare namespace ts {
71887188
/** @deprecated Use `factory.updateImportEqualsDeclaration` or the factory supplied by your transformation context instead. */
71897189
const updateImportEqualsDeclaration: (node: ImportEqualsDeclaration, decorators: readonly Decorator[] | undefined, modifiers: readonly Modifier[] | undefined, isTypeOnly: boolean, name: Identifier, moduleReference: ModuleReference) => ImportEqualsDeclaration;
71907190
/** @deprecated Use `factory.createImportDeclaration` or the factory supplied by your transformation context instead. */
7191-
const createImportDeclaration: (decorators: readonly Decorator[] | undefined, modifiers: readonly Modifier[] | undefined, importClause: ImportClause | undefined, moduleSpecifier: Expression, assertClause: AssertClause | undefined) => ImportDeclaration;
7191+
const createImportDeclaration: (decorators: readonly Decorator[] | undefined, modifiers: readonly Modifier[] | undefined, importClause: ImportClause | undefined, moduleSpecifier: Expression, assertClause?: AssertClause | undefined) => ImportDeclaration;
71927192
/** @deprecated Use `factory.updateImportDeclaration` or the factory supplied by your transformation context instead. */
71937193
const updateImportDeclaration: (node: ImportDeclaration, decorators: readonly Decorator[] | undefined, modifiers: readonly Modifier[] | undefined, importClause: ImportClause | undefined, moduleSpecifier: Expression, assertClause: AssertClause | undefined) => ImportDeclaration;
71947194
/** @deprecated Use `factory.createNamespaceImport` or the factory supplied by your transformation context instead. */

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ tests/cases/conformance/es6/destructuring/destructuringParameterDeclaration4.ts(
4141
a1(...array2); // Error parameter type is (number|string)[]
4242
~~~~~~
4343
!!! error TS2552: Cannot find name 'array2'. Did you mean 'Array'?
44-
!!! related TS2728 /.ts/lib.es5.d.ts:1447:13: 'Array' is declared here.
44+
!!! related TS2728 /.ts/lib.es5.d.ts:1464:13: 'Array' is declared here.
4545
a5([1, 2, "string", false, true]); // Error, parameter type is [any, any, [[any]]]
4646
~~~~~~~~
4747
!!! error TS2322: Type 'string' is not assignable to type '[[any]]'.

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ tests/cases/conformance/types/members/duplicateNumericIndexers.ts(25,5): error T
1111
tests/cases/conformance/types/members/duplicateNumericIndexers.ts(29,5): error TS2374: Duplicate index signature for type 'number'.
1212
tests/cases/conformance/types/members/duplicateNumericIndexers.ts(30,5): error TS2374: Duplicate index signature for type 'number'.
1313
lib.es5.d.ts(517,5): error TS2374: Duplicate index signature for type 'number'.
14-
lib.es5.d.ts(1433,5): error TS2374: Duplicate index signature for type 'number'.
14+
lib.es5.d.ts(1450,5): error TS2374: Duplicate index signature for type 'number'.
1515

1616

1717
==== tests/cases/conformance/types/members/duplicateNumericIndexers.ts (12 errors) ====

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

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -66,20 +66,20 @@ tests/cases/compiler/externModule.ts(37,3): error TS2552: Cannot find name 'XDat
6666
var d=new XDate();
6767
~~~~~
6868
!!! error TS2552: Cannot find name 'XDate'. Did you mean 'Date'?
69-
!!! related TS2728 /.ts/lib.es5.d.ts:910:13: 'Date' is declared here.
69+
!!! related TS2728 /.ts/lib.es5.d.ts:927:13: 'Date' is declared here.
7070
d.getDay();
7171
d=new XDate(1978,2);
7272
~~~~~
7373
!!! error TS2552: Cannot find name 'XDate'. Did you mean 'Date'?
74-
!!! related TS2728 /.ts/lib.es5.d.ts:910:13: 'Date' is declared here.
74+
!!! related TS2728 /.ts/lib.es5.d.ts:927:13: 'Date' is declared here.
7575
d.getXDate();
7676
var n=XDate.parse("3/2/2004");
7777
~~~~~
7878
!!! error TS2552: Cannot find name 'XDate'. Did you mean 'Date'?
79-
!!! related TS2728 /.ts/lib.es5.d.ts:910:13: 'Date' is declared here.
79+
!!! related TS2728 /.ts/lib.es5.d.ts:927:13: 'Date' is declared here.
8080
n=XDate.UTC(1964,2,1);
8181
~~~~~
8282
!!! error TS2552: Cannot find name 'XDate'. Did you mean 'Date'?
83-
!!! related TS2728 /.ts/lib.es5.d.ts:910:13: 'Date' is declared here.
83+
!!! related TS2728 /.ts/lib.es5.d.ts:927:13: 'Date' is declared here.
8484

8585

0 commit comments

Comments
 (0)