Type inference for conditional callback parameters breaks down

[krryan]

This is based on #21879; I have a work-around for that issue but still having a separate issue.

TypeScript Version: 2.8.0-dev.20180204

Code

declare function broke(impossible: never): never; // used to ensure full case coverage

interface Foo { kind: 'foo'; }
declare function isFoo(foobar: Foo | Bar): foobar is Foo;
interface Bar { kind: 'bar'; }
declare function isBar(foobar: Foo | Bar): foobar is Bar;

function mapFooOrBar<FooBar extends Foo | Bar, R>(
    foobar: FooBar,
    mapFoo: FooBar extends Foo ? ((value: Foo) => R) : ((impossible: never) => never),
    mapBar: FooBar extends Bar ? ((value: Bar) => R) : ((impossible: never) => never),
): R {
    if (isFoo(foobar)) {
        // workaround for #21879
        const handle = mapFoo as (value: Foo) => R;
        return handle(foobar);
    }
    else if (isBar(foobar)) {
        // workaround for #21879
        const handle = mapBar as (value: Bar) => R;
        return handle(foobar);
    }
    else {
        // workaround for #20375
        return broke(foobar as never);
    }
}

declare function toFoo(): Foo;

mapFooOrBar(
    toFoo(),
    foo => foo.kind,
/*  ^^^
Parameter 'foo' implicitly has an 'any' type. */
    bar => broke(bar)
/*  ^^^^^^^^^^^^^^^^^
                 ^^^
Argument of type '(bar: any) => any' is not assignable to parameter of type '(impossible: never) => never'.
  Type 'any' is not assignable to type 'never'. */
);

mapFooOrBar<Foo, string>(
    toFoo(),
    foo => foo.kind,
    bar => broke(bar)
);

(the casting foobar as never for the broke function is a workaround for #20375)

Expected Behavior
Compile without errors.

Actual Behavior
Typescript fails to infer the type of foo or bar in the callbacks, despite correctly inferring the type of the generic FooBar parameter in mapFooOrBar. Hovering over the first invocation of mapFooOrBar in VS Code, I see

function mapFooOrBar<Foo, {}>(foobar: Foo, mapFoo: (value: Foo) => {}, mapBar: (impossible: never) => never): {}

Which all looks correct, excepting the {} instead of string or 'foo' as the second parameter. But despite getting all that right, the parameters in the callback are inferred as any, which is likely the source of the {} second parameter. (Eliminating the second parameter and making the callbacks return void does not improve matters.)

[ghost]

So basically:

declare function useStringOrNumber<T extends string | number>(t: T, useIt: T extends string ? ((s: string) => void) : ((n: number) => void)): void;
useStringOrNumber("", foo => {});

For some reason the else branch in the conditional type is getting taken into account when determining the type of foo, even though it should be ignored because T should be inferred as string. It works if an explicit type argument is provided or if the else branch is not a function type.

[krryan]

Yeah, that's it.

I also figured out a partial workaround: if you make the parameter type and return type conditional, instead of making the entire callback type conditional, you get closer to the correct behavior:

declare function broke(impossible: never): never; // used to ensure full case coverage

interface Foo { kind: 'foo'; }
declare function isFoo(foobar: Foo | Bar): foobar is Foo;
interface Bar { kind: 'bar'; }
declare function isBar(foobar: Foo | Bar): foobar is Bar;

function mapFooOrBar<FooBar extends Foo | Bar, R>(
    foobar: FooBar,
    mapFoo: ((value: FooBar extends Foo ? Foo : never) => FooBar extends Foo ? R : never),
    mapBar: ((value: FooBar extends Bar ? Bar : never) => FooBar extends Bar ? R : never),
): R {
    if (isFoo(foobar)) {
        const handle = mapFoo as (value: Foo) => R;
        return handle(foobar);
    }
    else if (isBar(foobar)) {
        const handle = mapBar as (value: Bar) => R;
        return handle(foobar);
    }
    else {
        return broke(foobar as never);
    }
}

declare function toFoo(): Foo;
declare function takeString(value: string): void;

const inferred = mapFooOrBar(
    toFoo(),
    foo => foo.kind,
    bar => broke(bar)
);
takeString(inferred);
/*         ^^^^^^^^
Argument of type '{}' is not assignable to parameter of type 'string'. */

const explicit = mapFooOrBar<Foo, string>(
    toFoo(),
    foo => foo.kind,
    bar => broke(bar)
);
takeString(explicit);

takeString(mapFooOrBar(
    toFoo(),
    foo => foo.kind,
    bar => broke(bar)
));

As you can see, the return type still doesn't get inferred correctly—unless you immediately use it in a context that demands that return type, oddly enough.

Even if it completely worked, though, this would get really verbose and repetitive if your callback is supposed to take multiple arguments in the valid case.

[ahejlsberg]

This is a mix of working as intended and design limitations. The core issue is that you're attempting to simultaneously infer for the FooBar type parameter and apply the inferred type. That's beyond the capabilities of our type inference implementation. We effectively first infer for the type parameters and then, only after inference is complete, apply the inferred types. The only exception to this rule is when an argument of a function type is contextually typed by a function type (but not a conditional type) that references the type parameters for which we're inferring. In those cases we have the ability to eagerly fix and apply the inferences made so far as we process the function type argument.

The upshot of this is that the best you'll be able to do is something like:

function mapFooOrBar<FooBar extends Foo | Bar, R>(
    foobar: FooBar,
    mapFoo: (value: FooBar extends Foo ? Foo : never) => R,
    mapBar: (value: FooBar extends Bar ? Bar : never) => R,
): R

This works, but only because the type we infer for R in the last argument is never, which subsequently disappears when unioned together with the "foo" we infer from the middle argument.

I think a better approach here is to use overloads.

[krryan]

Actually, I disagree—I like yours much better than overloads. You're right, I don't need the 'stub' to be mandated to return never since its input indicates it should never be called in the first place—and we can type-safely note the impossible situation, as with broke. That's a good workaround for our purposes.

In contrast, our team avoids overloads as much as possible. There is no checking or validation of the overloaded function body to make sure that the overloads actually work out, and we have had a lot of trouble with inferred overload selection being surprising, counter-intuitive, or simply impossible to get behaving the way we want or expect them to. We almost never think overloads are a better approach to things.

At this point, I have a partial workaround that will suffice for our purposes, and you've noted that this is just how things are. I'm inclined to leave it open just because I think it's a worthwhile feature and worth pointing out that there is interest in more development here if in the future it ever comes under consideration. But maybe closed is the proper status for that situation—I leave that you.

[typescript-bot]

Automatically closing this issue for housekeeping purposes. The issue labels indicate that it is unactionable at the moment or has already been addressed.