Skip to content
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

Strict function check failure due to union type with matching parameters in the unioned interfaces #20714

Closed
liminf opened this issue Dec 15, 2017 · 5 comments
Labels
In Discussion Not yet reached consensus Suggestion An idea for TypeScript

Comments

@liminf
Copy link

liminf commented Dec 15, 2017

TypeScript Version: 2.7.0-dev.20171214

Code

export class Missing {
    public str : string;
}

export type ResultOrMissing<A> = A | Missing;

export interface IBar<A> {
    myParam: ResultOrMissing<A>;
}
export interface IFoo<A> {
    myParam: A;
}

export type BothOrUndefined<A> = (IFoo<A> | IBar<A>);

export interface IReturnsBoth {
    <A>() : (BothOrUndefined<A> | undefined);
}

const helper : IReturnsBoth = <A>() : (BothOrUndefined<A> | undefined) => {
        return undefined;
    };

Expected behavior:
Compiling the above code with "tsc .ts -strictFunctionTypes" should succeed.

Actual behavior:
Somehow wires get crossed and we get this error:

repro.ts(20,7): error TS2322: Type '<A>() => IFoo<A> | IBar<A>' is not assignable to type 'IReturnsBoth'.
  Type 'IFoo<ResultOrMissing<A>> | IBar<ResultOrMissing<A>>' is not assignable to type 'IFoo<A> | IBar<A>'.
    Type 'IBar<ResultOrMissing<A>>' is not assignable to type 'IFoo<A> | IBar<A>'.
      Type 'IBar<ResultOrMissing<A>>' is not assignable to type 'IBar<A>'.
        Type 'ResultOrMissing<A>' is not assignable to type 'A'.
          Type 'Missing' is not assignable to type 'A'.

Note the unexplained switch from IBar<A> to IBar<ResultOrMissing<A>>.

@RyanCavanaugh
Copy link
Member

@ahejlsberg can you elucidate? Simplified repro with unique names:

interface IBar<B> {
    myParam: B | "";
}
interface IFoo<F> {
    myParam: F;
}

var helper: <A>() => (IFoo<A> | IBar<A>);
helper = <T>(): (IFoo<T> | IBar<T>) => {
    return <any>null;
};

@geovanisouza92
Copy link

Seems similar to #20702

@ahejlsberg
Copy link
Member

Here's what happens: In order to check the assignment to helper we need to check that <T>() => (IFoo<T> | IBar<T>) (the source type) is assignable to <A>() => (IFoo<A> | IBar<A>) (the target type). In --strictFunctionTypes mode that means we need to instantiate source type in the context of the target type, which means we need to infer from <A>() => (IFoo<A> | IBar<A>) to <T>() => (IFoo<T> | IBar<T>) for the type variable T. In that process we make two inferences for T: A and "". We get two inferences because IFoo and IBar are structurally identical, so we succeed in making inferences from both IFoo<A> and IBar<A> to IFoo<T>. In other words, structurally both are candidates.

We could potentially consider being more selective in inference between union types by attempting to pairwise match types in the source and target to avoid "cross contamination", but it gets complicated pretty quickly.

@liminf
Copy link
Author

liminf commented Dec 16, 2017

My original issue had the two props being functions that returned different things:
interface IFoo<B> { handler: () => B } interface IBar<C> { handler: () => () => C }
And I can see the issue here even better - in trying to infer T, it would come up with a few possibilities. In addition to the obvious T mapping to A, there is also the possibility of IBar<A> can be assigned to IFoo<T> if T is () => A. This makes it so that T : A | () => A which results in an effective return type of (IFoo<A | () => A> | IBar<A | () => A>) because the inference is separated from the application of the inferred value to the function signature.

It sounds like the "right" solution would be along the lines that you mention: break up the union and infer (and compare) them all pairwise. Is this something that could reasonably be done or is it too destabilizing and should be saved as part of some bigger refactor?

@mhegazy mhegazy added Suggestion An idea for TypeScript In Discussion Not yet reached consensus labels Jan 10, 2018
@RyanCavanaugh
Copy link
Member

This works as expected now

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
In Discussion Not yet reached consensus Suggestion An idea for TypeScript
Projects
None yet
Development

No branches or pull requests

5 participants