Skip to content

indexing with T extends keyof <Large Union> causes OOM #23977

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

Closed
weswigham opened this issue May 8, 2018 · 1 comment
Closed

indexing with T extends keyof <Large Union> causes OOM #23977

weswigham opened this issue May 8, 2018 · 1 comment
Assignees
Labels
Bug A bug in TypeScript Fixed A PR has been merged for this issue

Comments

@weswigham
Copy link
Member

This breaks glimmer, and thereby our RWC (once sufficiently fixed in other ways)

Code

Isolated repro (needs dom lib for ElementTagNameMap and associated types):

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];
    }
}

Expected behavior:
No crash.

Actual behavior:
OOM crash after a very long delay.

Preliminary investigation shows the OOM occurs when we try to get the distributed form of keyof <Large Union> - namely the process is defined recursively, so when the union has ~170 members, we create 170 intermediate types (flattening one member at a time, while also wasting time recalculating type flags and other bits we will never need) before getting the final, distributed result. Now, picture doing this on every relation comparison operation for P (this distribution is uncached), and multiplying it for V in assertNodeProperty - the complexity is huge.

cc @DanielRosenwasser since I think you care about when we break glimmer.

@weswigham weswigham changed the title indexing keyof <Large Union> causes OOM indexing with keyof <Large Union> causes OOM May 8, 2018
@weswigham weswigham changed the title indexing with keyof <Large Union> causes OOM indexing with T extends keyof <Large Union> causes OOM May 8, 2018
@mhegazy mhegazy added the Bug A bug in TypeScript label May 8, 2018
@mhegazy mhegazy added this to the TypeScript 2.9 milestone May 8, 2018
@mhegazy
Copy link
Contributor

mhegazy commented May 8, 2018

//CC @ahejlsberg

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Bug A bug in TypeScript Fixed A PR has been merged for this issue
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants