Skip to content

Remove missing type from tuple type arguments under exactOptionalPropertyTypes #54718

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.

Sign up for GitHub

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

Open
Andarist wants to merge 8 commits into
base: main
Choose a base branch
from
+1,090 −66
Open

Remove missing type from tuple type arguments under exactOptionalPropertyTypes #54718

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
1f43c8d
Remove missing type from tuple type arguments under `exactOptionalPro…
Andarist Jun 20, 2023
64843f4
Merge remote-tracking branch 'origin/main' into fix/for-of-iteration-…
Andarist Aug 31, 2023
cd8f59c
avoid adding a missing type instead of removing it afterward
Andarist Aug 31, 2023
31d5bf7
Fixed more cases
Andarist Sep 1, 2023
3407b6b
Remove redundant `removeMissingType` calls
Andarist Sep 1, 2023
3c846b4
Revert part of the `removeMissingType` and add a test for it
Andarist Sep 3, 2023
0682323
Add extra test case
Andarist Sep 4, 2023
486c327
Merge remote-tracking branch 'origin/main' into fix/for-of-iteration-…
Andarist Dec 5, 2025
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
2 changes: 1 addition & 1 deletion src/compiler/checker.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15217,7 +15217,7 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
const typeArguments = !node ? emptyArray :
node.kind === SyntaxKind.TypeReference ? concatenate(type.target.outerTypeParameters, getEffectiveTypeArguments(node, type.target.localTypeParameters!)) :
node.kind === SyntaxKind.ArrayType ? [getTypeFromTypeNode(node.elementType)] :
map(node.elements, getTypeFromTypeNode);
map(node.elements, element => removeMissingType(getTypeFromTypeNode(element), element.kind === SyntaxKind.OptionalType));
Copy link
Member

@jakebailey jakebailey Aug 29, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems strange to me that this is being done inside of getTypeArguments; previously this function effectively just pulled info from the node and that's it, no processing.

Is there no other place this makes sense?

Copy link
Contributor Author

@Andarist Andarist Aug 31, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would go even further and say that the optionality should always be removed from those type arguments (of optional elements).

When it comes to object properties the optionality is added to the declared type based on the optionality marker on the property, see here. I see a strong parallel between this and the element types. The type argument is like a property's declared type - the optionality is a separate piece of information.

In a somewhat similar manner, in the case of a rest element - the type argument is not an array type! It's an element type of that array and the final array type~ gets created when the type argument meets the element flag.

I think removing this from the type arguments is best because the iterated type of an array comes from the Symbol.iterator method. Its signature is defined as (): IterableIterator<T> and T gets instantiated to the union that contains all type arguments. So at the end, we get something like (): IterableIterator<string | number | undefined>.

We might notice here that the rest type fits into this perfectly because its type is not an array type - so if we just push that to the union we end up with its element type and not an array, its concatenable~ with required elements, and the Symbol.iterator method that gets instantiated this way is correct.

It doesn't work well with optional elements in combination with exactOptionalPropertyTypes though if the undefined is included in the type argument right from the start. After all, at runtime - we won't actually iterate over that undefined since undefined isn't assignable to an optional element under EOPT.

So, I run an experiment to remove this at all times and "add back" that optionality at other places (see the commit here). It turned out to be... semi-successful. All the tests pass except the one that I'm trying to fix here.

The problem with this is that the easiest way to add back the optionality is through the created mapper (here). But then the T -> UnionOfTypeArguments uses that as well when instantiating the Symbol.iterator method. I don't see a clean way to branch this somehow or to remove the missing type afterward. After all, it's not only about what we get internally. The Symbol.iterator's method should get instantiated to, let's say, (): IterableIterator<string | number> and not to (): IterableIterator<string | number | undefined>. That's publicly~ visible information that can be obtained through types in user programs.

So, for better or worse - I think that it's best to leave things as-is but avoid adding the missingType to optional type arguments under EOPT. Perhaps a cleaner way of doing this would be something along those lines:

diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts
index 512e191cce..874318fb3b 100644
--- a/src/compiler/checker.ts
+++ b/src/compiler/checker.ts
@@ -15563,7 +15563,7 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
             const typeArguments = !node ? emptyArray :
                 node.kind === SyntaxKind.TypeReference ? concatenate(type.target.outerTypeParameters, getEffectiveTypeArguments(node, type.target.localTypeParameters!)) :
                 node.kind === SyntaxKind.ArrayType ? [getTypeFromTypeNode(node.elementType)] :
-                map(node.elements, element => removeMissingType(getTypeFromTypeNode(element), element.kind === SyntaxKind.OptionalType));
+                map(node.elements, element => getTypeFromTypeNode(element.kind === SyntaxKind.OptionalType && exactOptionalPropertyTypes ? (element as OptionalTypeNode).type : element));
             if (popTypeResolution()) {
                 type.resolvedTypeArguments = type.mapper ? instantiateTypes(typeArguments, type.mapper) : typeArguments;
             }

I think I like this one better and I am going to push it out in a second.

Copy link
Contributor Author

@Andarist Andarist Aug 31, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is also a non-zero chance that this actually makes some of the existing removeMissingType calls redundant. I will investigate this.

if (popTypeResolution()) {
type.resolvedTypeArguments = type.mapper ? instantiateTypes(typeArguments, type.mapper) : typeArguments;
}
Expand Down
20 changes: 20 additions & 0 deletions ...eference/forOfOptionalTupleMember(exactoptionalpropertytypes=false,target=es5).errors.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
forOfOptionalTupleMember.ts(12,3): error TS18048: 'item' is possibly 'undefined'.


==== forOfOptionalTupleMember.ts (1 errors) ====
// repro from https://github.com/microsoft/TypeScript/issues/54302

type Item = {
value: string;
};

type Foo = [Item?];

declare const foo: Foo;

for (let item of foo) {
item.value;
~~~~
!!! error TS18048: 'item' is possibly 'undefined'.
}

31 changes: 31 additions & 0 deletions ...s/reference/forOfOptionalTupleMember(exactoptionalpropertytypes=false,target=es5).symbols
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
//// [tests/cases/compiler/forOfOptionalTupleMember.ts] ////

=== forOfOptionalTupleMember.ts ===
// repro from https://github.com/microsoft/TypeScript/issues/54302

type Item = {
>Item : Symbol(Item, Decl(forOfOptionalTupleMember.ts, 0, 0))

value: string;
>value : Symbol(value, Decl(forOfOptionalTupleMember.ts, 2, 13))

};

type Foo = [Item?];
>Foo : Symbol(Foo, Decl(forOfOptionalTupleMember.ts, 4, 2))
>Item : Symbol(Item, Decl(forOfOptionalTupleMember.ts, 0, 0))

declare const foo: Foo;
>foo : Symbol(foo, Decl(forOfOptionalTupleMember.ts, 8, 13))
>Foo : Symbol(Foo, Decl(forOfOptionalTupleMember.ts, 4, 2))

for (let item of foo) {
>item : Symbol(item, Decl(forOfOptionalTupleMember.ts, 10, 8))
>foo : Symbol(foo, Decl(forOfOptionalTupleMember.ts, 8, 13))

item.value;
>item.value : Symbol(value, Decl(forOfOptionalTupleMember.ts, 2, 13))
>item : Symbol(item, Decl(forOfOptionalTupleMember.ts, 10, 8))
>value : Symbol(value, Decl(forOfOptionalTupleMember.ts, 2, 13))
}

29 changes: 29 additions & 0 deletions ...nes/reference/forOfOptionalTupleMember(exactoptionalpropertytypes=false,target=es5).types
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
//// [tests/cases/compiler/forOfOptionalTupleMember.ts] ////

=== forOfOptionalTupleMember.ts ===
// repro from https://github.com/microsoft/TypeScript/issues/54302

type Item = {
>Item : { value: string; }

value: string;
>value : string

};

type Foo = [Item?];
>Foo : [(Item | undefined)?]

declare const foo: Foo;
>foo : Foo

for (let item of foo) {
>item : Item | undefined
>foo : Foo

item.value;
>item.value : string
>item : Item | undefined
>value : string
}

20 changes: 20 additions & 0 deletions ...rence/forOfOptionalTupleMember(exactoptionalpropertytypes=false,target=esnext).errors.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
forOfOptionalTupleMember.ts(12,3): error TS18048: 'item' is possibly 'undefined'.


==== forOfOptionalTupleMember.ts (1 errors) ====
// repro from https://github.com/microsoft/TypeScript/issues/54302

type Item = {
value: string;
};

type Foo = [Item?];

declare const foo: Foo;

for (let item of foo) {
item.value;
~~~~
!!! error TS18048: 'item' is possibly 'undefined'.
}

31 changes: 31 additions & 0 deletions ...eference/forOfOptionalTupleMember(exactoptionalpropertytypes=false,target=esnext).symbols
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
//// [tests/cases/compiler/forOfOptionalTupleMember.ts] ////

=== forOfOptionalTupleMember.ts ===
// repro from https://github.com/microsoft/TypeScript/issues/54302

type Item = {
>Item : Symbol(Item, Decl(forOfOptionalTupleMember.ts, 0, 0))

value: string;
>value : Symbol(value, Decl(forOfOptionalTupleMember.ts, 2, 13))

};

type Foo = [Item?];
>Foo : Symbol(Foo, Decl(forOfOptionalTupleMember.ts, 4, 2))
>Item : Symbol(Item, Decl(forOfOptionalTupleMember.ts, 0, 0))

declare const foo: Foo;
>foo : Symbol(foo, Decl(forOfOptionalTupleMember.ts, 8, 13))
>Foo : Symbol(Foo, Decl(forOfOptionalTupleMember.ts, 4, 2))

for (let item of foo) {
>item : Symbol(item, Decl(forOfOptionalTupleMember.ts, 10, 8))
>foo : Symbol(foo, Decl(forOfOptionalTupleMember.ts, 8, 13))

item.value;
>item.value : Symbol(value, Decl(forOfOptionalTupleMember.ts, 2, 13))
>item : Symbol(item, Decl(forOfOptionalTupleMember.ts, 10, 8))
>value : Symbol(value, Decl(forOfOptionalTupleMember.ts, 2, 13))
}

29 changes: 29 additions & 0 deletions .../reference/forOfOptionalTupleMember(exactoptionalpropertytypes=false,target=esnext).types
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
//// [tests/cases/compiler/forOfOptionalTupleMember.ts] ////

=== forOfOptionalTupleMember.ts ===
// repro from https://github.com/microsoft/TypeScript/issues/54302

type Item = {
>Item : { value: string; }

value: string;
>value : string

};

type Foo = [Item?];
>Foo : [(Item | undefined)?]

declare const foo: Foo;
>foo : Foo

for (let item of foo) {
>item : Item | undefined
>foo : Foo

item.value;
>item.value : string
>item : Item | undefined
>value : string
}

31 changes: 31 additions & 0 deletions ...es/reference/forOfOptionalTupleMember(exactoptionalpropertytypes=true,target=es5).symbols
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
//// [tests/cases/compiler/forOfOptionalTupleMember.ts] ////

=== forOfOptionalTupleMember.ts ===
// repro from https://github.com/microsoft/TypeScript/issues/54302

type Item = {
>Item : Symbol(Item, Decl(forOfOptionalTupleMember.ts, 0, 0))

value: string;
>value : Symbol(value, Decl(forOfOptionalTupleMember.ts, 2, 13))

};

type Foo = [Item?];
>Foo : Symbol(Foo, Decl(forOfOptionalTupleMember.ts, 4, 2))
>Item : Symbol(Item, Decl(forOfOptionalTupleMember.ts, 0, 0))

declare const foo: Foo;
>foo : Symbol(foo, Decl(forOfOptionalTupleMember.ts, 8, 13))
>Foo : Symbol(Foo, Decl(forOfOptionalTupleMember.ts, 4, 2))

for (let item of foo) {
>item : Symbol(item, Decl(forOfOptionalTupleMember.ts, 10, 8))
>foo : Symbol(foo, Decl(forOfOptionalTupleMember.ts, 8, 13))

item.value;
>item.value : Symbol(value, Decl(forOfOptionalTupleMember.ts, 2, 13))
>item : Symbol(item, Decl(forOfOptionalTupleMember.ts, 10, 8))
>value : Symbol(value, Decl(forOfOptionalTupleMember.ts, 2, 13))
}

29 changes: 29 additions & 0 deletions ...ines/reference/forOfOptionalTupleMember(exactoptionalpropertytypes=true,target=es5).types
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
//// [tests/cases/compiler/forOfOptionalTupleMember.ts] ////

=== forOfOptionalTupleMember.ts ===
// repro from https://github.com/microsoft/TypeScript/issues/54302

type Item = {
>Item : { value: string; }

value: string;
>value : string

};

type Foo = [Item?];
>Foo : [Item?]

declare const foo: Foo;
>foo : Foo

for (let item of foo) {
>item : Item
>foo : Foo

item.value;
>item.value : string
>item : Item
>value : string
}

31 changes: 31 additions & 0 deletions ...reference/forOfOptionalTupleMember(exactoptionalpropertytypes=true,target=esnext).symbols
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
//// [tests/cases/compiler/forOfOptionalTupleMember.ts] ////

=== forOfOptionalTupleMember.ts ===
// repro from https://github.com/microsoft/TypeScript/issues/54302

type Item = {
>Item : Symbol(Item, Decl(forOfOptionalTupleMember.ts, 0, 0))

value: string;
>value : Symbol(value, Decl(forOfOptionalTupleMember.ts, 2, 13))

};

type Foo = [Item?];
>Foo : Symbol(Foo, Decl(forOfOptionalTupleMember.ts, 4, 2))
>Item : Symbol(Item, Decl(forOfOptionalTupleMember.ts, 0, 0))

declare const foo: Foo;
>foo : Symbol(foo, Decl(forOfOptionalTupleMember.ts, 8, 13))
>Foo : Symbol(Foo, Decl(forOfOptionalTupleMember.ts, 4, 2))

for (let item of foo) {
>item : Symbol(item, Decl(forOfOptionalTupleMember.ts, 10, 8))
>foo : Symbol(foo, Decl(forOfOptionalTupleMember.ts, 8, 13))

item.value;
>item.value : Symbol(value, Decl(forOfOptionalTupleMember.ts, 2, 13))
>item : Symbol(item, Decl(forOfOptionalTupleMember.ts, 10, 8))
>value : Symbol(value, Decl(forOfOptionalTupleMember.ts, 2, 13))
}

29 changes: 29 additions & 0 deletions ...s/reference/forOfOptionalTupleMember(exactoptionalpropertytypes=true,target=esnext).types
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
//// [tests/cases/compiler/forOfOptionalTupleMember.ts] ////

=== forOfOptionalTupleMember.ts ===
// repro from https://github.com/microsoft/TypeScript/issues/54302

type Item = {
>Item : { value: string; }

value: string;
>value : string

};

type Foo = [Item?];
>Foo : [Item?]

declare const foo: Foo;
>foo : Foo

for (let item of foo) {
>item : Item
>foo : Foo

item.value;
>item.value : string
>item : Item
>value : string
}

18 changes: 18 additions & 0 deletions tests/cases/compiler/forOfOptionalTupleMember.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
// @strict: true
// @target: es5, esnext
// @exactOptionalPropertyTypes: true, false
// @noEmit: true

// repro from https://github.com/microsoft/TypeScript/issues/54302

type Item = {
value: string;
};

type Foo = [Item?];

declare const foo: Foo;

for (let item of foo) {
item.value;
}