Suggestion: short-circuit type inference for ternary operators when both branches have the same type

[amcasey]

From https://github.com/squidfunk/typescript-issue-38339-repro (courtesy of @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

Playground Link: https://www.typescriptlang.org/play/?ts=Nightly#code/JYWwDg9gTgLgBAbzgKQMoA04F84DMoQhwBEYUApgIYDGMxAUPTAJ5jlwCSAdjFMFwGdg1AKIAbciHI8BcALwoMAOm69+Q0RKkyA2gGtyzCLkXoVPPoOHjJ0mAIC6jcgA9IsOCzacL661rsBACYAHno4OAAVeR81K01bGXoAPhiEcLgdAGk4fjgDIxNIhwAuOBzXGGkAE1liGmpyMDoMiIB+KOyHOAAfTIE4gHMnCIiyyK76LGc3aHgvdlVLDRttewBmMIjohSW-BLXglLSM7NyufMNjKNLyuEqauuoxSgEBBlG4Doms7r6dABGEAgEkoXBGo3Gk2m9Fc7nmrEWvniq0CABYtlEYnsUQEZOtjgp0hEznkCtdimUKi4qlxaiRgNUPqNvl1epkuABXEAA8hQCFjTq-KYzeF4TlcWjACAXXDAgAUAEoyjiVnj7Gj2RLquRcPxyNVEBkJPMVci1YkNVq6br9dUMgB6B0UGCcqAXeBOuAAMVeMAyLrdHqgnPYHXgZU9DrgqDEEAA7iKgA

Related Issues: #38339

[ahejlsberg]

So, the culprit here is the type generated by:

type IntrinsicElements = JSX.IntrinsicElements[keyof JSX.IntrinsicElements]

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 IntrinsicElements above 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 is

type IntrinsicElements = JSX.HTMLAttributes<HTMLElement> | JSX.SVGAttributes<SVGElement>;

Basically either HTMLAttributes based on the root HTML element type or SVGAttributes based 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]

Also, the IntrinsicElementsX mapped types can be written more efficiently as:

type IntrinsicElements2 = IntrinsicElements |
  { [K in keyof IntrinsicElements]-?: K extends "accept" ? [string] : never };

type IntrinsicElements3 = IntrinsicElements2 |
  { [K in keyof IntrinsicElements]-?: K extends "class" ? [boolean] : never };

type IntrinsicElements4 = IntrinsicElements3 |
  { [K in keyof IntrinsicElements]-?: K extends "id" ? [number] : never };

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:

type IntrinsicElements4 = IntrinsicElements |
  { [K in keyof IntrinsicElements]-?:
    K extends "accept" ? [string] :
    K extends "class" ? [boolean] :
    K extends "id" ? [number] :
    never
  };

[squidfunk]

@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:

export type Property<
  T = PropertyValue
> = {
  [K in keyof T]: K extends "data"
    ? T[K] | [DefaultValue]
    : T[K]
}

export type Property2 = PropertyValue | {
  [K in keyof PropertyValue]-?: K extends "data"
    ? [DefaultValue]
    : never
}

// works
const a: Property = {
  type: ValueType.PROPERTY,
  kind: "border",
  data: [
    { type: ValueType.IDENT, data: "initial"}
  ]
}

// doesn't work, see error message below
const b: Property2 = {
  type: ValueType.PROPERTY,
  kind: "border",
  data: [
    { type: ValueType.IDENT, data: "initial"}
  ]
}

The latter results in:

Type '{ type: ValueType.PROPERTY; kind: "border"; data: { type: ValueType.IDENT; data: string; }[]; }' is not assignable to type 'Property2'.
  Types of property 'data' are incompatible.
    Type '{ type: ValueType.IDENT; data: string; }' is not assignable to type 'ColorValue | LineWidthValue | LineStyleValue'.
      Types of property 'type' are incompatible.
        Type 'ValueType.IDENT' is not assignable to type 'ValueType.TYPE'.ts(2322)

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]

@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]

@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). ramda and rxjs might be good candidates.

[amcasey]

@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]

That sounds like a good approach. It would be awesome if @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.