Skip to content
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

Simplify conditionals upon comparison, rather than instantiation #31374

Conversation

weswigham
Copy link
Member

@weswigham weswigham commented May 13, 2019

Additional work on top of #31354 to retain our better simplification behavior (this is currently set to merge into that PR).

styled-components on #31354:

Files:            30
Lines:         79011
Nodes:        142289
Identifiers:   39324
Symbols:      157061
Types:         95977
Memory used: 182493K
I/O read:      0.03s
I/O write:     0.00s
Parse time:    0.58s
Bind time:     0.24s
Check time:    5.66s
Emit time:     0.00s
Total time:    6.48s

This PR:

Files:            30
Lines:         79011
Nodes:        142289
Identifiers:   39324
Symbols:      157061
Types:         96130
Memory used: 185759K
I/O read:      0.03s
I/O write:     0.00s
Parse time:    0.55s
Bind time:     0.16s
Check time:    5.41s
Emit time:     0.00s
Total time:    6.12s

completionEntryDetails typing delay (via repro from #31302) I sampled at 211ms on #31354 as-is, with this PR I sampled 147ms.

@ahejlsberg
Copy link
Member

I was about to commit a very similar change to my PR. I also removed the root-based simplifications from getConditionalType such that simplification only occurs in getSimplifiedType. Opinion on having both? Don't much care for the duplicated code.

@weswigham
Copy link
Member Author

Opinion on having both? Don't much care for the duplicated code.

The only real difference should be in alias symbols - by deferring all simplifications we guarantee that the conditional keeps it's alias, even if it simplifies (since its identity still exists). It's arguably better that way, so yeah, could do that.

@ahejlsberg ahejlsberg merged commit 4e040f7 into microsoft:deferConditionalTypes May 13, 2019
@weswigham weswigham deleted the defer-conditionals-with-simplification branch May 13, 2019 22:53
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.

None yet

2 participants