Skip to content

Fix circular mapped type instantiations for arrays and tuples #27911

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.

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

Merged
merged 8 commits into from
Oct 15, 2018

Conversation

ahejlsberg
Copy link
Member

Mapped types can circularly reference themselves (because member resolution is deferred), but array and tuple type instantiations cannot. This causes an issue when a circular homomorphic mapped type is instantiated for an array or tuple type as we now create array and tuple instantiations for such mappings (see #26063). With is PR we quickly detect and stop the runaway recursion that can occur by substituting errorType for the instantiation. We would eventually do this after 50 levels of nested instantiations, but exponential recursive fan-out could keep us from ever getting there.

Fixes #27881.

!!! error TS2322: Type '{ a: number; }' is not assignable to type '{ a?: undefined; b?: undefined; }'.
!!! error TS2322: Types of property 'a' are incompatible.
!!! error TS2322: Type 'number' is not assignable to type 'undefined'.
!!! error TS2322: Type '{ a: number; }' is not assignable to type '{ a: number; b: number; }'.
Copy link
Member

Choose a reason for hiding this comment

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

any idea why this changes? I don't see any mapped types here.

Copy link
Member

Choose a reason for hiding this comment

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

Never mind, didn't see the change to discriminant types.

@sandersn sandersn self-assigned this Oct 15, 2018
@@ -14212,12 +14222,26 @@ namespace ts {
return undefined;
}

function isDiscriminantType(type: Type): boolean {
Copy link
Member

Choose a reason for hiding this comment

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

this seems unrelated to the bug fix in the PR description

}
let combined = 0;
for (const t of (<UnionType>type).types) combined |= t.flags;
if (combined & TypeFlags.Unit && !(combined & TypeFlags.Instantiable)) {
Copy link
Member

Choose a reason for hiding this comment

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

what happens if you have two two non-unit types? Should that still be discriminable?

Copy link
Member

Choose a reason for hiding this comment

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

I'm pretty sure this change isn't supposed to be here, since it's #27695

Copy link
Member

@sandersn sandersn left a comment

Choose a reason for hiding this comment

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

Couple of questions

@ahejlsberg
Copy link
Member Author

I accidentally based this PR on the mixedDiscriminantTypes branch (which has been merged now). Just look at the commits starting with "Handle circular mapped type instantiations for arrays and tuples".

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants