Optimize intersections of unions of unit types

[ahejlsberg]

This PR optimizes creation of intersection types containing unions of unit types. Such types occur for example when obtaining keyof X where X is large union of object types with many properties.

Fixes #23977.

[weswigham]

Just some comments on what we're doing here in general; looks good.

[weswigham]

With this, TypeFlags is full. We're going to need a new field for flags like this (very) soon (as I don't see any that are obviously ObjectFlags). (This and the propagating flags are effectively a union's equivalent of ObjectFlags)

[ahejlsberg]

Well, TypeFlags being full is nothing new. Seems like it has been for the last many years. I actually think it is fine to put all of the flags to good use and then work out alternate solutions as the need arises. This particular flag could easily be moved to a property local to union types, but there's no reason to do so now.

[weswigham]

It's worth a comment that we skip over getUnionType and call getUnionTypeFromSortedList directly here because we believe we've already replicated its work in a more efficient fashion.

[weswigham]

Possible optimization: unionOfUnitTypes should just be additionalFlags, and we should calculate getPropagatingFlagsOfTypes piece-wise while doing includes, and just pass them in (rather then making another pass over the type here to calculate them).... although I know right now to save flag space we re-purpose the propagating flags during include calculation, which could make that.... hard.

[ahejlsberg]

Yeah, but I don't really want to mess with that now.

[weswigham]

Ah! before I forget! This has the same downside (incompleteness?) my PR has (which I asked about being worth doing): It doesn't handle a union containing symbol or number (or other primitives), which is potentially problematic, since keyof (the easiest way to make a huge union) can return at least number. For example:

// @strict: true
// Modified repro from #23977

declare global {
    interface ElementTagNameMap {
        [index: number]: HTMLElement
    }

    interface HTMLElement {
        [index: number]: HTMLElement;
    }
}

export function assertIsElement(node: Node | null): node is Element {
    let nodeType = node === null ? null : node.nodeType;
    return nodeType === 1;
}
  
export function assertNodeTagName<
    T extends keyof ElementTagNameMap,
    U extends ElementTagNameMap[T]>(node: Node | null, tagName: T): node is U {
    if (assertIsElement(node)) {
        const nodeTagName = node.tagName.toLowerCase();
         return nodeTagName === tagName;
    }
    return false;
}
  
export function assertNodeProperty<
    T extends keyof ElementTagNameMap,
    P extends keyof ElementTagNameMap[T],
    V extends HTMLElementTagNameMap[T][P]>(node: Node | null, tagName: T, prop: P, value: V) {
    if (assertNodeTagName(node, tagName)) {
        node[prop];
    }
}

will still happily churn through types for a long time. (I checked out your branch and double checked, it does have this problem)

It's probably worth fixing - it doesn't take too deep an object structure before this becomes problematic, and a single numeric indexer foiling the optimization is not great.

[ahejlsberg]

@weswigham Yes, I will look at adding handling for string, number, and symbol.

[ahejlsberg]

@mhegazy @weswigham Adding generalized support for string, number, and symbol is going to add a non-trivial amount of complexity. I'm still mulling over the best algorithm, but meanwhile I think we should merge the PR as it sits now and get the real world bug resolved.

[mhegazy]

sounds good to me.