Skip to content

Fix recursive mapped type infinite recursion #21134

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 5 commits into from
Jan 11, 2018

Conversation

sandersn
Copy link
Member

@sandersn sandersn commented Jan 10, 2018

Fixes #17847

isGenericMappedType indirectly calls getResolvedBaseConstraint, but is called indirectly from inside getResolvedBaseConstraint. This avoids the circularity check in getResolvedBaseConstraint, which causes an infinite recursion for certain malformed types.

Interestingly, I found four approaches that fixed the bug:

  1. Delete lines 6559-6561 (if (isGenericMappedType(t)) { return emptyObjectType; })
  2. Pass typeStack to isGenericMappedType and check it in getConstraintOfTypeParameter instead of calling hasNonCircularBaseConstraint.
  3. Clone isGenericMappedType and its callees into getResolvedBaseConstraint (and check typeStack).
  4. Clone isGenericMappedType and its callees into getResolvedBaseConstraint, and merge into a single function (that checks typeStack).

And @weswigham suggested a fifth,

  1. Remove typeStack and use push/popTypeResolution instead.

Previously, I decided on (2) because it avoids forking the code paths, but (5) reuses code even better.

@sandersn
Copy link
Member Author

@weswigham, I got push/popTypeResolution to work by making circularConstraintType behave the same way as undefined, since it should. I think this change is ready to go.

@sandersn sandersn merged commit 658de7f into master Jan 11, 2018
@sandersn sandersn deleted the fix-recursive-mapped-type-infinite-recursion branch January 11, 2018 18:24
@microsoft microsoft locked and limited conversation to collaborators Jul 3, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Stack overflow with incorrect mapped type as constraint of type parameter
2 participants