Repository navigation
Suggestion: short-circuit type inference for ternary operators when both branches have the same type #38399
Description
From https://github.com/squidfunk/typescript-issue-38339-repro (courtesy of Martin Donath (@squidfunk))
TypeScript Version: 3.9.1-rc
Search Terms:
Subtype reduction, ternary
Code
import { JSX } from "preact"
type IntrinsicElements = JSX.IntrinsicElements[keyof JSX.IntrinsicElements]
export type IntrinsicElements2<
T = IntrinsicElements
> = {
[K in keyof T]: K extends "accept"
? T[K] | [string]
: T[K]
}
export type IntrinsicElements3<
T = IntrinsicElements2
> = {
[K in keyof T]: K extends "class"
? T[K] | [boolean]
: T[K]
}
export type IntrinsicElements4<
T = IntrinsicElements3
> = {
[K in keyof T]: K extends "id"
? T[K] | [number]
: T[K]
}
export function foo(): IntrinsicElements4 | undefined {
let t: IntrinsicElements4 | undefined
// this takes 2 seconds to check
return t
// this takes 10 seconds to check
// return true ? t : t
}Expected behavior: return t and return true ? t : t check in a comparable amount of time
Actual behavior: the ternary is much slower
Related Issues: #38339
Activity
ahejlsberg commentedon May 9, 2020
So, the culprit here is the type generated by:
This creates a union of 108 object types each with over 300 properties. The types are all similar because they share a large number of data properties, but they are also subtly different in their event handler properties because of explicit target types. This union type is then further subjected to four layers of mapped and conditional types that add a bunch of new type identities.
But then the really bad thing that happens is the subtype reduction kicked off by the ternary operator in
true ? t : t. This causes us to relate 108 x 108 huge object types that are all very similar, meaning we have to do a bunch of processing for every pair to just to realize they're different. And, to make matters worse, the types are based on mapped types that can only be related structurally because we can't reliably compute their variance.Now, in reality the definition of
IntrinsicElementsabove is really pretty useless because all of the event handler properties have impossible target types (you can't be every possible element type at the same time). I think a better way to write that type isBasically either
HTMLAttributesbased on the root HTML element type orSVGAttributesbased on the root SVG element type. When I do that the check time drops from 4.37s to 0.07s. In other words a 60x reduction! And, best I can tell, there's little loss in type fidelity.I suppose I could be missing somthing, but otherwise that's the fix I'd suggest.
ahejlsberg commentedon May 10, 2020
Also, the
IntrinsicElementsXmapped types can be written more efficiently as:This flattens the type into a union instead of stacking multiple mapped types on top of each other.
Even more efficient is to have a single type do the augmentation:
squidfunk commentedon May 11, 2020
Anders Hejlsberg (@ahejlsberg) thanks for the tips on improving the typings. Unfortunately, this does not produce the same result, as it doesn't allow me to assign the enhanced value:
The latter results in:
Note that my intention was to enhance a specific member of each type of a union with a new type, in this case
[DefaultValue]which has{ type: ValueType.IDENT, ...}.Also, note that this repository is only meant to reproduce the ternary performance bottleneck, so I'm quite aware that the typings are non-sensical.
amcasey commentedon May 11, 2020
Martin Donath (@squidfunk) Is this comment a good example of the sort of thing you'd like the optimized types to be able to handle? What we need is a clear understanding of your goals so that we can identify/invent a more performant way to accomplish them (maybe).
squidfunk commentedon May 12, 2020
Andrew Casey (@amcasey) Yes, that's one of the things that slow the compiler down when munching through my codebase. Also, as posted in this issue, enhancing of unions is something I use a lot.
Maybe it's really worth trying to compile some popular projects which use generics with 3.9.1 and trace down performance bottlenecks, as I discovered in #38339 (comment).
ramdaandrxjsmight be good candidates.amcasey commentedon May 12, 2020
Martin Donath (@squidfunk) We've been building popular OSS projects looking for perf issues, but it's very valuable to have author input on intentions and expectations. Otherwise, it's hard to for us to tell whether the performance we're seeing is worse than it ought to be and whether the types can be changed without losing something important.
squidfunk commentedon May 13, 2020
That sounds like a good approach. It would be awesome if Anders Hejlsberg (@ahejlsberg) could comment on #38399 (comment) when he finds some time, as I'd really like to reduce the pressure put on the compiler without losing type safety 😊I haven't found a better approach, but if there is, it would be documented here.