microsoft / TypeScript

TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
https://www.typescriptlang.org
Apache License 2.0
100.75k stars 12.46k forks source link

Incorrect Recursive Union Emmission #31605

Closed ericwooley closed 5 years ago

ericwooley commented 5 years ago

TypeScript Version: 3.4.5 & typescript@3.5.0-dev.20190525

Search Terms: Recursive Emit union Code

export const testRecFun= <T extends Object>(parent: T) => {
    return {
      result: parent,
      deeper: <U extends Object>(child: U) =>
        testRecFun<T & U>({ ...parent, ...child })
    };
}

let p1 = testRecFun({one: '1'})
console.log(p1.result.one)
let p2 = p1.deeper({two: '2'})
console.log(p2.result.one, p2.result.two);
let p3 = p2.deeper({three: '3'})
console.log(p3.result.one, p3.result.two, p3.result.three);

Expected behavior: each depth would produce a unique generic for U

export declare const testRecFun: <T extends Object>(parent: T) => {
    result: T;
    deeper: <U extends Object>(child: U) => {
        result: T & U;
        deeper: <Ua extends Object>(child: Ua) => {
            result: T & U & Ua;
            deeper: <Ub extends Object>(child: Ub) => {
                result: T & U & Ua & Ub;
                deeper: <Uc extends Object>(child: Uc) => {
                    result: T & U & Ua & Ub & Uc;
                    deeper: <Ud extends Object>(child: Ud) => {
                        result: T & U & Ua & Ub & Uc & Ud;
                        deeper: <Ue extends Object>(child: Ue) => {
                            result: T & U & Ua & Ub & Uc & Ud & Ue;
                            deeper: <Uf extends Object>(child: Uf) => {
                                result: T & U & Ua & Ub & Uc & Ud & Ue & Uf;
                                deeper: <Ug extends Object>(child: Ug) => {
                                    result: T & U & Ua & Ub & Uc & Ud & Ue & Uf & Ug;
                                    deeper: <Uh extends Object>(child: Uh) => {
                                        result: T & U & Ua & Ub & Uc & Ud & Ue & Uf & Ug & Uh;
                                        deeper: <Ui extends Object>(child: Ui) => {
                                            result: T & U & Ua & Ub & Uc & Ud & Ue & Uf & Ug & Uh & Ui;
                                            deeper: <Uj extends Object>(child: Uj) => any;
                                        };
                                    };
                                };
                            };
                        };
                    };
                };
            };
        };
    };
};

Actual behavior:

export declare const testRecFun: <T extends Object>(parent: T) => {
    result: T;
    deeper: <U extends Object>(child: U) => {
        result: T & U;
        deeper: <U extends Object>(child: U) => {
            result: T & U & U;
            deeper: <U extends Object>(child: U) => {
                result: T & U & U & U;
                deeper: <U extends Object>(child: U) => {
                    result: T & U & U & U & U;
                    deeper: <U extends Object>(child: U) => {
                        result: T & U & U & U & U & U;
                        deeper: <U extends Object>(child: U) => {
                            result: T & U & U & U & U & U & U;
                            deeper: <U extends Object>(child: U) => {
                                result: T & U & U & U & U & U & U & U;
                                deeper: <U extends Object>(child: U) => {
                                    result: T & U & U & U & U & U & U & U & U;
                                    deeper: <U extends Object>(child: U) => {
                                        result: T & U & U & U & U & U & U & U & U & U;
                                        deeper: <U extends Object>(child: U) => {
                                            result: T & U & U & U & U & U & U & U & U & U & U;
                                            deeper: <U extends Object>(child: U) => any;
                                        };
                                    };
                                };
                            };
                        };
                    };
                };
            };
        };
    };
};

// intermediate steps are lost.
let p1 = testRecFun({one: '1'})
console.log(p1.result.one)
let p2 = p1.deeper({two: '2'})
console.log(p2.result.one, p2.result.two);
let p3 = p2.deeper({three: '3'})
console.log(p3.result.one, p3.result.two, p3.result.three) // error p3.result.two is not ...

Playground Link:

I didn't make this in the playground, but I made a repo for reproduction. https://github.com/ericwooley/ts-recursive-test

Related Issues:

jcalz commented 5 years ago

I'm not reproducing this bug in my local environment with either 3.4.5 or 3.5.0-dev.20190525. The IntelliSense does show U & U & U & U but those are just unrelated types with the same names. The actual concrete results (e.g., p3.result.two) look correct to me. What am I missing?

looksokaytome

weswigham commented 5 years ago

The typechecking is correct - the declaration emit is just wrong. #31544 should fix this.

jcalz commented 5 years ago

So in the "actual behavior" from the original post,

console.log(p3.result.one, p3.result.two, p3.result.three) // error p3.result.two is not ...

that error message is made up? I just can't trust people on the Internet! 😒

weswigham commented 5 years ago

that error message is made up? I just can't trust people on the Internet!

That's the error you get if your consume our current declaration output - since every type variable is named U you only ever get one member after the first.

jcalz commented 5 years ago

Right, it just occurred to me that the actual issue is something like... you emit a declaration file for the first bit and then use those declarations with the collision-named types and fun times happen. Trust in Internet people restored! I'm going to go send some money to that Nigerian prince!

ericwooley commented 5 years ago

I'm glad my trust is restored! Sorry I was a bit behind in replying, but thanks everyone for picking up the slack!

31544 does look like it would resolve the issue to me.

It seems like there is a need for some kind of pointer or reference in type emissions so that cases like recursion could be followed the by the type checker the same was as they would be followed by interpreting the original code. Does anyone know if that idea has that already been discussed anywhere I could follow along?

weswigham commented 5 years ago

It seems like there is a need for some kind of pointer or reference in type emissions so that cases like recursion could be followed the by the type checker the same was as they would be followed by interpreting the original code.

The checker is actually what is converting the type into the node tree. It just wasn't picking good names for the nodes it manufactured, since it wasn't tracking names it'd already used~

ericwooley commented 5 years ago

:tada: :raised_hands:

Thanks!