Defer resolution of true and false branches in conditional types #31354

ahejlsberg · 2019-05-11T16:59:20Z

This PR restores our previous behavior of deferring resolution of the true and false branches of conditional types. This fixes the performance regression we were seeing with statement completion in projects using styled-components (performance is now similar to 3.3).

Because resolution of the true and false branches is deferred, simplifications of conditional types of the form T extends xxx ? T : never and T extends xxx ? never : T only occur when the actual declarations of the conditional types are written with identical types in the T position and type never in the other position (i.e. simplification doesn't occur when conditional types only match the pattern following instantiation). This shouldn't matter in practice as the types we really care about, Exclude and Extract, follow the pattern. For example, #28748 is still fixed.

EDIT: Conditional type simplification now takes place in getSimplifiedType and we simplify to the same extent as before, so the above comment no longer applies.

Fixes #31302.
Fixes #31341.

ahejlsberg · 2019-05-11T18:16:14Z

@typescript-bot test this

typescript-bot · 2019-05-11T18:16:17Z

Heya @ahejlsberg, I've started to run the extended test suite on this PR at bb9c5c9. You can monitor the build here. It should now contribute to this PR's status checks.

ahejlsberg · 2019-05-11T18:16:29Z

@typescript-bot run dt

typescript-bot · 2019-05-11T18:16:31Z

Heya @ahejlsberg, I've started to run the parallelized Definitely Typed test suite on this PR at bb9c5c9. You can monitor the build here. It should now contribute to this PR's status checks.

ahejlsberg · 2019-05-11T18:27:12Z

There are definitely significantly fewer types being created when the true and false branches are deferred.

Compiling styled-components with current master:

Files:            28
Lines:         21723
Nodes:         60563
Identifiers:   19996
Symbols:      129415
Types:        132769
Memory used: 207569K
I/O read:      0.01s
I/O write:     0.00s
Parse time:    0.40s
Bind time:     0.17s
Check time:    8.36s
Emit time:     0.00s
Total time:    8.92s

Compiling styled-components with this PR:

Files:            28
Lines:         21723
Nodes:         60563
Identifiers:   19996
Symbols:      121459
Types:         87840
Memory used: 168256K
I/O read:      0.01s
I/O write:     0.00s
Parse time:    0.39s
Bind time:     0.17s
Check time:    6.21s
Emit time:     0.00s
Total time:    6.77s

Compiling styled-components with 3.3.4:

Files:            28
Lines:         21776
Nodes:         61201
Identifiers:   20271
Symbols:      102024
Types:         63544
Memory used: 120873K
I/O read:      0.01s
I/O write:     0.00s
Parse time:    0.36s
Bind time:     0.18s
Check time:    4.84s
Emit time:     0.00s
Total time:    5.39s

Compiling styled-components with 3.4.5:

Files:            28
Lines:         21776
Nodes:         61203
Identifiers:   20273
Symbols:      129627
Types:        151297
Memory used: 183032K
I/O read:      0.01s
I/O write:     0.00s
Parse time:    0.37s
Bind time:     0.22s
Check time:    8.15s
Emit time:     0.00s
Total time:    8.75s

j-oliveras · 2019-05-11T20:15:30Z

Probably fixes #31341 too.

weswigham · 2019-05-11T23:33:13Z

@ahejlsberg we should be able to defer the simplification to assignability checking, similar to indexed accesses, to keep the fixed use-cases functional.

weswigham · 2019-05-13T18:28:57Z

@ahejlsberg #31374 is open with what I suggested - perf impact (compared to this PR as-is) in styled-components is either minimal or positive, and all the simplifications continue to work as they do in master.

…lification Simplify conditionals upon comparison, rather than instantiation

…tution types

ahejlsberg · 2019-05-13T23:36:33Z

@typescript-bot test this

typescript-bot · 2019-05-13T23:36:36Z

Heya @ahejlsberg, I've started to run the extended test suite on this PR at 066e4b6. You can monitor the build here. It should now contribute to this PR's status checks.

ahejlsberg · 2019-05-13T23:36:44Z

@typescript-bot run dt

typescript-bot · 2019-05-13T23:36:45Z

Heya @ahejlsberg, I've started to run the parallelized Definitely Typed test suite on this PR at 066e4b6. You can monitor the build here. It should now contribute to this PR's status checks.

weswigham · 2019-05-13T23:47:12Z

tests/baselines/reference/conditionalTypes2.errors.txt

-                    Type 'keyof B' is not assignable to type '"valueOf"'.
-                      Type 'string | number | symbol' is not assignable to type '"valueOf"'.
-                        Type 'string' is not assignable to type '"valueOf"'.
+            Type 'string' is not assignable to type 'number | "toString" | "charAt" | "charCodeAt" | "concat" | "indexOf" | "lastIndexOf" | "localeCompare" | "match" | "replace" | "search" | "slice" | "split" | "substring" | "toLowerCase" | "toLocaleLowerCase" | "toUpperCase" | "toLocaleUpperCase" | "trim" | "length" | "substr" | "valueOf" | keyof A'.


did keyof A expand into... itself? o.O

In the first keyof A, A is a substitution type. It then gets expanded into keyof (A & string) which becomes the union that ends in keyof A where A is the actual type variable. Not ideal, but neither was the prior error message.

DanielRosenwasser · 2019-05-14T17:03:47Z

src/compiler/checker.ts

+                    return getSimplifiedType(trueType, writing);
+                }
+                else if (isIntersectionEmpty(checkType, extendsType)) { // Always false
+                    return neverType;


Why wasn't this just a Debug.fail?

Because it can happen and is perfectly fine when it does. Or am I misunderstanding your question?

BTW, the comment "Always false" here refers to when the condition of the conditional type is known to always be false. It doesn't imply that the if condition is always false.

ahejlsberg added 2 commits May 11, 2019 09:39

Defer resolution of the true and false branches of conditional types

71fe8e8

Accept new baselines

33c7e7f

ahejlsberg requested review from weswigham, RyanCavanaugh and DanielRosenwasser May 11, 2019 16:59

Reuse existing type instantiations

bb9c5c9

ahejlsberg mentioned this pull request May 11, 2019

Newest version 1.33.1 has electron_node tsserver.js eating up 100% cpu randomly #31341

Closed

ahejlsberg mentioned this pull request May 12, 2019

Revert this-parameter filtering for methods #31349

Closed

Simplify conditionals upon comparison, rather than instantiation

8ba53b6

weswigham mentioned this pull request May 13, 2019

Simplify conditionals upon comparison, rather than instantiation #31374

Merged

ahejlsberg added 3 commits May 13, 2019 15:52

Merge pull request #31374 from weswigham/defer-conditionals-with-simp…

4e040f7

…lification Simplify conditionals upon comparison, rather than instantiation

Remove simplification logic from getConditionalType + simplify substi…

1b3589b

…tution types

Accept new baselines

066e4b6

weswigham reviewed May 13, 2019

View reviewed changes

weswigham approved these changes May 13, 2019

View reviewed changes

ahejlsberg merged commit fb6ae38 into master May 14, 2019

ahejlsberg deleted the deferConditionalTypes branch May 14, 2019 00:17

ahejlsberg added this to the TypeScript 3.5.0 milestone May 14, 2019

jack-williams mentioned this pull request May 14, 2019

Update with simplification logic jack-williams/ts-spec#2

Open

ahejlsberg mentioned this pull request May 14, 2019

Revert substitution type simplification #31400

Merged

DanielRosenwasser reviewed May 14, 2019

View reviewed changes

andrewbranch mentioned this pull request May 20, 2019

Weird heap overflow when compiling a single line of code with recursive type definition. #30831

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Defer resolution of true and false branches in conditional types #31354

Defer resolution of true and false branches in conditional types #31354

ahejlsberg commented May 11, 2019 •

edited

Loading

ahejlsberg commented May 11, 2019

typescript-bot commented May 11, 2019 •

edited

Loading

ahejlsberg commented May 11, 2019

typescript-bot commented May 11, 2019 •

edited

Loading

ahejlsberg commented May 11, 2019 •

edited

Loading

j-oliveras commented May 11, 2019

weswigham commented May 11, 2019

weswigham commented May 13, 2019

ahejlsberg commented May 13, 2019

typescript-bot commented May 13, 2019 •

edited

Loading

ahejlsberg commented May 13, 2019

typescript-bot commented May 13, 2019 •

edited

Loading

weswigham May 13, 2019 •

edited

Loading

ahejlsberg May 13, 2019

DanielRosenwasser May 14, 2019

ahejlsberg May 14, 2019

ahejlsberg May 14, 2019

Defer resolution of true and false branches in conditional types #31354

Defer resolution of true and false branches in conditional types #31354

Conversation

ahejlsberg commented May 11, 2019 • edited Loading

ahejlsberg commented May 11, 2019

typescript-bot commented May 11, 2019 • edited Loading

ahejlsberg commented May 11, 2019

typescript-bot commented May 11, 2019 • edited Loading

ahejlsberg commented May 11, 2019 • edited Loading

j-oliveras commented May 11, 2019

weswigham commented May 11, 2019

weswigham commented May 13, 2019

ahejlsberg commented May 13, 2019

typescript-bot commented May 13, 2019 • edited Loading

ahejlsberg commented May 13, 2019

typescript-bot commented May 13, 2019 • edited Loading

weswigham May 13, 2019 • edited Loading

Choose a reason for hiding this comment

ahejlsberg May 13, 2019

Choose a reason for hiding this comment

DanielRosenwasser May 14, 2019

Choose a reason for hiding this comment

ahejlsberg May 14, 2019

Choose a reason for hiding this comment

ahejlsberg May 14, 2019

Choose a reason for hiding this comment

ahejlsberg commented May 11, 2019 •

edited

Loading

typescript-bot commented May 11, 2019 •

edited

Loading

typescript-bot commented May 11, 2019 •

edited

Loading

ahejlsberg commented May 11, 2019 •

edited

Loading

typescript-bot commented May 13, 2019 •

edited

Loading

typescript-bot commented May 13, 2019 •

edited

Loading

weswigham May 13, 2019 •

edited

Loading