-
Notifications
You must be signed in to change notification settings - Fork 12.6k
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
No contextual types from circular mapped type properties #38653
Conversation
@typescript-bot test this |
Heya @ahejlsberg, I've started to run the extended test suite on this PR at 1dcf4ab. You can monitor the build here. |
Heya @ahejlsberg, I've started to run the parallelized Definitely Typed test suite on this PR at 1dcf4ab. You can monitor the build here. |
Heya @ahejlsberg, I've started to run the parallelized community code test suite on this PR at 1dcf4ab. You can monitor the build here. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems sensible to me.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We also do type
resolution stack inspection for avoiding some circularities in JS checking, so we have some precedent for doing this. We probably need to monitor LS bug reports to to make sure this doesn't cause a bug in the category of thing-gets-contextual-type-if-mouseover-A-then-B-but-not-B-then-A. I don't think the results of this calculation should be cached anywhere, so it should be OK; but we should be vigilant in case we get a report that sounds like that.
* upstream/master: Support naming tuple members (microsoft#38234) LEGO: check in for master to temporary branch. fix: extract const in jsx (microsoft#37912) No contextual types from circular mapped type properties (microsoft#38653) Ensure formatter can always get a newline character (microsoft#38579) Fix debug command for Node debugging Remove mentions of runtests-browser in CONTRIBUTING.md fix(33233): add outlining for comments before property access expression regression(38485): allow using rawText property in processing a tagged template
@typescript-bot cherry-pick this to release-3.9 |
Heya @RyanCavanaugh, I've started to run the task to cherry-pick this into |
Hey @RyanCavanaugh, I've opened #38687 for you. |
)" This reverts commit 9ec5fc5.
With this PR we no longer attempt to produce contextual types from mapped type properties that are circular. Specifically, in
getTypeOfPropertyOfContextualType
we now check if the property originates in mapped type and if resolution of the type of the property would cause a circularity. If so, instead of resolving the type (which would produce a circularity error) we simply returnundefined
to indicate that there is no contextual type.I have added the simplified repro from #38279 to the tests, plus I have manually verified that each of the complex repros in that issue no longer error.
Fixes #38279.