-
Notifications
You must be signed in to change notification settings - Fork 12.5k
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
Improve detection of type variables in instantiated object types #53246
Conversation
@typescript-bot test this |
Heya @ahejlsberg, I've started to run the diff-based top-repos suite on this PR at e3b542b. You can monitor the build here. Update: The results are in! |
Heya @ahejlsberg, I've started to run the extended test suite on this PR at e3b542b. You can monitor the build here. |
Heya @ahejlsberg, I've started to run the diff-based user code test suite on this PR at e3b542b. You can monitor the build here. Update: The results are in! |
Heya @ahejlsberg, I've started to run the abridged perf test suite on this PR at e3b542b. You can monitor the build here. Update: The results are in! |
Heya @ahejlsberg, I've started to run the parallelized Definitely Typed test suite on this PR at e3b542b. You can monitor the build here. Update: The results are in! |
@ahejlsberg Here are the results of running the user test suite comparing Everything looks good! |
@ahejlsberg Here they are:Comparison Report - main..53246
System
Hosts
Scenarios
Developer Information: |
@ahejlsberg Here are the results of running the top-repos suite comparing Everything looks good! |
Hey @ahejlsberg, the results of running the DT tests are ready. |
Oh wow, @mnajdova you might want to run your TypeScript benchmark against this change, see if we can measure the impact. |
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 can tighten these checks even further, when we want to, now that we're willing to rely on there not being any sneaky type variables outside the object's mapper (which, iirc, is what we've worried about when I've mentioned doing this in the past). Since mappers have inspectable sources and targets (where possible) nowadays, we can rig them to actually keep a set of which type parameters remain unmapped for a given stack of mappers after instantiation when we compose them, and then use that to check if any of the inference targets are actually still present in the target type (thus, allowing us to skip inference on arguments or parts of types which only contain type parameters for outer calls).
On material-ui's docs project:
|
Just in case anyone was confused as to why I'm not getting quite the great number, the above is the total time end-to-end measured via |
Hello, I was browsing that MR and the source code but have trouble understanding what is it about. It would be great to read a description for mortal human being because it's interesting. Great work BTW. Congrats on the results! |
When an object type that references type parameters is instantiated, it is often the case that the resulting instantiated type doesn't reference type parameters (because it is often the case that all type arguments are non-generic types). With this PR we improve our detection of such object types in order to short-circuit further pointless type instantiation and type inference involving those types.
The optimizations in this PR reduces the check time for the material-ui project by over 50%. (Note that the reduction isn't nearly as dramatic with the very old version of material-ui we use in our performance test suite.)