Skip to content

3.5 regression: invalid type for index lookup using constrained generic #32017

@OliverJAsh

Description

@OliverJAsh
Contributor

TypeScript Version: 3.5.2

Search Terms: generic constraint constrained index lookup function union

Code

type MyRecord = {
    foo: { foo: number };
    bar: { bar: number };
};

type Tag = keyof MyRecord;

type MyUnion = MyRecord[Tag];

declare const values: {
    foo: MyRecord['foo'];
    bar: MyRecord['bar'];
};

declare const fns: {
    foo: () => MyRecord['foo'];
    bar: () => MyRecord['bar'];
};

declare const predicates: {
    foo: (value: MyUnion) => value is MyRecord['foo'];
    bar: (value: MyUnion) => value is MyRecord['bar'];
};

<T extends Tag>(tag: T) => {
    // 1.
    {
        const value = values[tag];
        type ExpectedType = MyRecord[T];
        // Expected: no error
        // 3.4 and 3.5: works as expected
        const test: ExpectedType = value;
    }

    // 2.
    {
        declare const value: MyUnion;
        // Expected: error
        // 3.4: no error
        // 3.5: works as expected
        const desired: MyRecord[T] = value;
    }

    // 3.
    {
        const fn = fns[tag];

        type ExpectedType = () => MyRecord[T];
        // Expected: no error
        // 3.4: works as expected
        // 3.5: unexpected error
        const test: ExpectedType = fn;
    }

    // 4. (this is our real world use case)
    {
        declare const value: MyUnion;
        const predicate = predicates[tag];

        if (predicate(value)) {
            // Expected: no error
            // 3.4: works as error
            // 3.5: unexpected error
            const desired: MyRecord[T] = value;
        }
    }
};

Activity

OliverJAsh

OliverJAsh commented on Jun 21, 2019

@OliverJAsh
ContributorAuthor

Could this be related to #30769?

It seems 3.5 fixed something which made example 2 (above) begin failing as it always should have, but this change in turn meant that examples 3 and 4 started to fail (which I believe they shouldn't).

ahejlsberg

ahejlsberg commented on Jun 21, 2019

@ahejlsberg
Member

Yes, this is an effect of #30769. In example 4 you're assigning a { foo: number } | { bar: number } to a MyRecord[T]. That may be correct (which was as far as 3.4 checked it), but it isn't known to be correct (which is what 3.5 checks). For example, it isn't correct if value is a { foo: number } and T is "bar".

OliverJAsh

OliverJAsh commented on Jun 21, 2019

@OliverJAsh
ContributorAuthor

In example 4 you're assigning a { foo: number } | { bar: number } to a MyRecord[T]

In example 4, shouldn't value be narrowed to MyRecord[T]? The reason I think that is because we index into predicates with T, which should mean the resulting predicate's type predicate will match MyRecord[T].

For example, it isn't correct if value is a { foo: number } and T is "bar".

IIUC, that should be impossible, because the predicate type predicate would always correspond to T. For example, if T is "bar", value would always be { bar: number }.

(Sorry if I'm being slow here, and thanks so much for your help!)

jack-williams

jack-williams commented on Jun 21, 2019

@jack-williams
Collaborator

In example 4 you're trying to narrowing something concrete to something generic using concrete predicates. In general TypeScript will never produce something generic from something concrete.

You're implicitly assuming assuming that there is a concrete lower bound on T; that is, there are no types assignable to "foo" | "bar" other than "foo" or "bar", and that also have values (so not never).

Personally I think this is dubious reasoning, but that is besides the point because TypeScript doesn't do this reasoning anyway.

I think the way to write this would be to pass in a generic predicate function:

type Predicates<T extends Tag> = {
    [K in T]: (value: MyUnion) => value is MyRecord[K];
}

<U extends Tag, T extends U>(tag: T, predicates: Predicates<U>) => {
///
}
karol-majewski

karol-majewski commented on Jun 24, 2019

@karol-majewski

@jack-williams Could you please elaborate why this is dubious reasoning?

What else then

is assignable to "foo" | "bar"?

OliverJAsh

OliverJAsh commented on Jun 25, 2019

@OliverJAsh
ContributorAuthor

I'm also curious to know the answer to @karol-majewski's question.

@jack-williams

In example 4 you're trying to narrowing something concrete to something generic using concrete predicates.

Can't the same be said for example 1? IIUC, the only difference is instead of concrete predicates/functions we have concrete values.

I think the way to write this would be to pass in a generic predicate function:

Unfortunately this workaround would defeat the point of our function, which is: given a tag from a tagged union, pass the given union value into a given function if the value has the required tag.

For context, here is our real world use case in full:

const runFnIf = <T extends Tag>(tag: T, fn: (value: MyRecord[T]) => void) => (value: MyUnion) => {
    const predicate = predicates[tag];

    if (predicate(value)) {
        // Expected: no error
        // 3.4: works as error
        // 3.5: unexpected error
        fn(value);
    }
};

const runMyFnIfFoo = runFnIf('foo', foo => {
    foo.foo;
});

declare const myUnion: MyUnion;
runMyFnIfFoo(myUnion);
ahejlsberg

ahejlsberg commented on Jun 26, 2019

@ahejlsberg
Member

Using the names Foo and Bar for { foo: number } and { bar: number }, your predicate variable has type

{ foo: (value: Foo | Bar) => value is Foo, bar: (value: Foo | Bar) => value is Bar }[T]

but that is not the same as

(value: Foo | Bar) => value is { foo: Foo, bar: Bar }[T]

which is the type predicate would need in order to narrow value to { foo: Foo, bar: Bar }[T]. You need a type assertion to convince the checker:

const predicate = predicates[tag] as (value: Foo | Bar) => value is { foo: Foo, bar: Bar}[T];

Without the type assertion, in the call predicate(value) the checker uses the constraint of T (i.e. 'foo' | 'bar') and ends up with the type (value: Foo | Bar) => value is Foo | Bar, which narrows value to the type it already has.

The reason it worked in 3.4 is the unsound checking we previously did for assignments to indexed access types (i.e. we'd allow Foo | Bar to be assigned to { foo: Foo, bar: Bar}[T]).

OliverJAsh

OliverJAsh commented on Jun 27, 2019

@OliverJAsh
ContributorAuthor

Thanks for the helpful reply @ahejlsberg.

{ foo: (value: Foo | Bar) => value is Foo, bar: (value: Foo | Bar) => value is Bar }[T]

but that is not the same as

(value: Foo | Bar) => value is { foo: Foo, bar: Bar }[T]

I'm struggling to see why the above two types can't be proved to be the same.

  • If T is 'foo', they would both "end up" as (value: Foo | Bar) => value is Foo.
  • If T is 'bar', they would both "end up" as (value: Foo | Bar) => value is Bar.
  • If T is 'foo' | 'bar', they would both "end up" as (value: Foo | Bar) => value is Foo | Bar.

Is there a case I'm missing where T is something else and in which case the above two types would not end up the same?

typescript-bot

typescript-bot commented on Jun 29, 2019

@typescript-bot
Collaborator

This issue has been marked 'Working as Intended' and has seen no recent activity. It has been automatically closed for house-keeping purposes.

OliverJAsh

OliverJAsh commented on Jun 30, 2019

@OliverJAsh
ContributorAuthor

Hoping we can still get some response to #32017 (comment) before this issue becomes locked.

jack-williams

jack-williams commented on Jul 1, 2019

@jack-williams
Collaborator

@karol-majewski

I think it's dubious reasoning because it violates parametricity and makes assumptions that are not future compatible. As I've mentioned before (probably to the tedium of others), the assumption that the only subtypes of "foo" | "bar" are the ones you list does not hold under name-subtyping, which is some I'd like to see in the future.

For example:

declare function swap<T extends "foo">(x: readonly ["foo", T]): readonly [T, "foo"];

The function swap really should (IMO) be the function that returns a new tuple with the elements swapped. However, if you follow the reasoning that you propose it could implemented by a function that returns the input unmodified.

Assuming that the type "foo" is denoted by a singleton set, rather than a set of uniformly behaving values, is unnecessarily transparent. IMO it just adds holes to the type-system that don't need to be there.

RyanCavanaugh

RyanCavanaugh commented on Jul 1, 2019

@RyanCavanaugh
Member

Even today "foo" & { sketchy: "brand" } is a subtype of "foo".

karol-majewski

karol-majewski commented on Jul 1, 2019

@karol-majewski

@jack-williams @RyanCavanaugh thank you for your answers!

Just to make sure: is this explanation correct then? 👇

Using T instead of "foo" | "bar" is a hint that it's about subtype relationships, so T gets limited from the top by "foo" | "bar" but in theory can be a subtype of that union. I know it's not practical, but it falls under the laws described by the specification.

declare function test<T extends "foo" | "bar">(arg: T): void;

declare const argument: "foo" & { length: 0 };

test(argument) // Passes

Even if something is a subtype of "foo", then it is still "foo" (covariance) and indexing should work just fine.

jack-williams

jack-williams commented on Jul 2, 2019

@jack-williams
Collaborator

@karol-majewski Yep

Even if something is a subtype of "foo", then it is still "foo" (covariance) and indexing should work just fine.

To elaborate: indexing using a T for something expecting a "foo" is fine - it is the converse that will not work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    Working as IntendedThe behavior described is the intended behavior; this is not a bug

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

      Development

      No branches or pull requests

        Participants

        @OliverJAsh@ahejlsberg@jack-williams@RyanCavanaugh@karol-majewski

        Issue actions

          3.5 regression: invalid type for index lookup using constrained generic · Issue #32017 · microsoft/TypeScript