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

Incorrectly inferred type for union types #43962

Closed
RomanRolginSky opened this issue May 5, 2021 · 5 comments · Fixed by #49887
Closed

Incorrectly inferred type for union types #43962

RomanRolginSky opened this issue May 5, 2021 · 5 comments · Fixed by #49887
Assignees
Labels
Fix Available A PR has been opened for this issue Needs Investigation This issue needs a team member to investigate its status.
Milestone

Comments

@RomanRolginSky
Copy link

Bug Report

🔎 Search Terms

infer, never, union types, error TS2345

🕗 Version & Regression Information

  • This changed between versions 3.5.1 and all newer versions up to 4.3.0-beta. 3.5.1 doesn't throw such error, there is the error in 3.6.2

⏯ Playground Link

Playground link with relevant code

💻 Code

type Thing<T> = {
    value: T;
    pipe<A, B, C>(
        opA: Op<T, A>,
        opB: Op<A, B>,
        opC: Op<B, C>
    ): Thing<C>;
};
type Op<I, O> = (thing: Thing<I>) => Thing<O>;

declare function createThing<T>(result: T): Thing<T>;
declare function map<T, R>(project: (value: T) => R): Op<T, R>;
declare function tap<T>(next: (value: T) => void): Op<T, T>;

interface Wrapped<V> {
    data: V;
    props?: object;
}
declare function isWrapped<T>(obj: any): obj is Wrapped<T>;
declare function unwrap<T>(obj: T | Wrapped<T>): T;

type WrappedInferredData<T> = T extends Wrapped<infer R> ? R : T;
type WrappedInput<V> = () => Thing<V>;

declare function addSettings<V>(
    data: V | Wrapped<V>,
    props: object
): Wrapped<WrappedInferredData<V | Wrapped<V>>>;

const functionWithTypingIssue = <V>(factory: WrappedInput<V | Wrapped<V>>): Thing<V> => {
    return factory().pipe(map((item) => unwrap(item)), map(item => item), map(item => unwrap(item)));
}

const thisIsTargetValue = 1;

const a$ = functionWithTypingIssue(() => {
    const thing = createThing(thisIsTargetValue);
    return thing.pipe(
        map((data) => addSettings(data, { anyProp: 1 })),
        map((data) => data), // error here
        tap((v) => v)
    );
});

a$.value; // should be of type thisIsTargetValue
Argument of type 'Op<Wrapped<number>, Wrapped<number>>' is not assignable to parameter of type 'Op<Wrapped<number>, Wrapped<never>>'.
  Type 'Wrapped<number>' is not assignable to type 'Wrapped<never>'.
    Type 'number' is not assignable to type 'never'. 

🙁 Actual behavior

Type incorrectly infers from union type

🙂 Expected behavior

Type should be resolved correctly. typeof thisIsTargetValue === number for this example

@RyanCavanaugh RyanCavanaugh added the Needs Investigation This issue needs a team member to investigate its status. label May 5, 2021
@RyanCavanaugh RyanCavanaugh added this to the Backlog milestone May 5, 2021
@RyanCavanaugh
Copy link
Member

I'm not sure if this is a bug or not; a much shorter repro would be appreciated.

@RomanRolginSky
Copy link
Author

RomanRolginSky commented May 5, 2021

@RyanCavanaugh I removed some non-required declaration but even shorter is unreal 😊 Part of declarations is a mock of rxjs-library (like Op, Thing, map,tap) so it can't be simplified/removed

type Op<I, O> = (thing: Thing<I>) => Thing<O>;
type Thing<T> = {
    value: T;
    pipe<A, B, C>(
        opA: Op<T, A>,
        opB: Op<A, B>,
    ): Thing<B>;
};
type Wrapped<V> = { data: V }
type WrappedInferredData<T> = T extends Wrapped<infer R> ? R : T;

declare function createThing<T>(result: T): Thing<T>;
declare function map<T, R>(project: (value: T) => R): Op<T, R>;
declare function tap<T>(next: (value: T) => void): Op<T, T>;
declare function addSettings<V>(data: V | Wrapped<V>): Wrapped<WrappedInferredData<V | Wrapped<V>>>;
declare function functionWithTypingIssue<V>(factory: () => Thing<V | Wrapped<V>>): Thing<V>;

const thisIsTargetValue = 1;

const result = functionWithTypingIssue(() => {
    const thing = createThing(thisIsTargetValue);
    return thing.pipe(
        map((data) => addSettings(data)),
        tap(v => v)
    );
});

result.value.data; // should be of type thisIsTargetValue

Typescript playground

@jakebailey
Copy link
Member

Thanks for providing the versions, I was able to bisect this to #31662.

@jakebailey
Copy link
Member

Just to give the test case I'm actually testing here: Playground Link

(It's somewhat smaller but not much)

@jakebailey
Copy link
Member

My understanding so far is that when we are inferring and we detect that the two types have the same alias symbol, we will just infer from the type arguments and return early. #31662 changed things in a way that some inferences we made before become contravariant inferences instead. Then, we pick Wrapped<never> instead of Wrapped<number> because the former is a "normal" inference.

If I instead infer from type arguments, but don't return early and allow it to continue, the bug goes away. It seems like doing this equal symbol short circuit is not always correct because it gives a different result than a normal inference. Not returning also gets rid of a couple very silly looking error elaborations, but then we end up regressing #42320 as we determined that circularity to be okay (and this symbol short circuit is what makes it legal).

There are other bugs I've seen in #31662 while working on this issue, but I don't have any repros that actually change. Namely, inferFromTypeArguments has special behavior for functions when not in strictFunctionTypes which shouldn't have also been applied to aliases like this, but fixing that doesn't help in this situation as the fix would give us more contravariant inferences, not fewer.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Fix Available A PR has been opened for this issue Needs Investigation This issue needs a team member to investigate its status.
Projects
None yet
4 participants