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

False positive error report for enum and class instance type missing index signature, thus not assignable to object type with unknown values. #30977

Closed
Veetaha opened this issue Apr 16, 2019 · 8 comments · Fixed by #31687
Assignees
Labels
Bug A bug in TypeScript Fixed A PR has been merged for this issue

Comments

@Veetaha
Copy link

Veetaha commented Apr 16, 2019

TypeScript Version: 3.5.0-dev.20190410, (v3.4.3 doesn't have this bug)

Search Terms:
enum, class, assignable, index signature

Code

enum Enum { A }

const e0: { [key: string]: any     } = Enum; // ok
const e1: { [key: string]: unknown } = Enum; // produces an unexpected error report

//

const obj = { A: 0 };

const o0: { [key: string]: any     } = obj; // ok
const o1: { [key: string]: unknown } = obj; // ok

// 

class Class {}

export const cl0: { [key: string]: any     } = new Class; // ok
export const cl1: { [key: string]: unknown } = new Class; // produces an unexpected error report

Expected behavior:
This code should compile with no errors.

Actual behavior:
Error near the definition of e1 and cl1:

Type 'typeof Enum/Class' is not assignable to type '{ [key: string]: unknown; }'.
Index signature is missing in type 'typeof Enum/Class'.

Playground Link:
This works well in the playground as it uses the stable version of tsc, but this bug appears in dev build. Klick me.

Related Issues:
None.

@Veetaha Veetaha changed the title False positive error report for enum type missing index signature. False positive error report for enum type missing index signature, thus not assignable type with unknown values. Apr 16, 2019
@Veetaha Veetaha changed the title False positive error report for enum type missing index signature, thus not assignable type with unknown values. False positive error report for enum type missing index signature, thus not assignable to type with unknown values. Apr 16, 2019
@Veetaha Veetaha changed the title False positive error report for enum type missing index signature, thus not assignable to type with unknown values. False positive error report for enum type missing index signature, thus not assignable to object type with unknown values. Apr 16, 2019
@Veetaha Veetaha changed the title False positive error report for enum type missing index signature, thus not assignable to object type with unknown values. False positive error report for enum and class instance type missing index signature, thus not assignable to object type with unknown values. Apr 16, 2019
@RyanCavanaugh RyanCavanaugh added the Bug A bug in TypeScript label Apr 17, 2019
@RyanCavanaugh RyanCavanaugh added this to the TypeScript 3.5.0 milestone Apr 17, 2019
@ahejlsberg
Copy link
Member

We have always had a rule that permits assigning any non-primitive type to a type with an index signature of type any. When we introduced the unknown type (#24439) we added a similar rule that permits any non-primitive type to be assigned to a type with an index signature of type unknown. However, 3.5 changes the default type parameter constraint from {} to unknown (#30637) and that necessitates removing the assignment rule for unknown index signatures. Otherwise, whenever type inference fails and yields the default unknown we become way too permissive for types of the form { [x: string]: T }.

Now, we have another rule that permits an object literal type to be assigned to a type with an index signature provided all property types of the object literal are assignable to that index signature. We permit this because we know object literal types are exact (i.e. there are no additional properties we don't know about). This is true for enum object types as well, so we should extend this rule to cover enum types. That would solve the first unexpected error in the OP example. But we can't extend the rule to cover classes since they're by design extensible (and therefore not exact).

@Veetaha
Copy link
Author

Veetaha commented Apr 30, 2019

First thing

@ahejlsberg I am excited about switching from {} to unknown. However, this raises the problem of defining the type-safe generic dictionary object.

In typescript@3.4 this was easy, but in 3.5 we are not able to do this.
Lets define a shortcut:

// This way we technically declare { [key: any]: TValues; },
// which we can't do literally, but this is out of scope
type Obj<TValues> = Record<any, TValues>; 

Now the main body:

export function dir<T extends Obj<unknown>>(dict: T): T {
    // some logging logic
    const value = dict.foo;

    value.someProp; // compile error, value is of `unknown` type which is great
 
    return dict;
}

const classDict = dir(new class {}); // nah, class instance is not allowed since 3.5
const objDict   = dir({});           // ok

If we use Obj<any> as the type constraint, it gets too permissive and value.someProp doesn't generate compile error anymore.

If we use {} or object/Object as the generic constraint, dict.foo generates an error, which is not intended.

Do you have any solution to this?

Second thing

Why do we get the following behavior?

type TakeObj<T extends Obj<unknown>>  = T;

interface EmptyInterface {}
type EmptyTypeAlias = {};

type t0 = TakeObj<EmptyTypeAlias>; // ok
type t1 = TakeObj<EmptyInterface>; // compile error ??

@ahejlsberg
Copy link
Member

First, here's one example of why we needed to retire the rule that permits anything to be assigned to { [x: string]: unknown }.

declare function foo<T>(x: Record<string, T>): T[];
declare function foo<T>(x: T[]): T[];

foo({ a: 1, b: 'abc' });  // (string | number)[]
foo([1, 'abc']);  // (string | number)[]

If we had kept the rule, the foo([1, 'abc']) call would resolve to type unknown[] because, during overload resolution, we infer unknown for T when inferring to the first overload (the common type of all properties of an array is unknown), and the rule then kicks in and picks the first overload. But, regardless of whether the element type is unknown or something more specific, you really don't want an array to be considered a dictionary.

I think you can reasonably make the argument that the new behavior is the consistent one. Our intent has always been that the only thing assignable to a dictionary type (a type of the form { [x: string]: T }) is another compatible dictionary type or an object literal with properties that are all assignable to T (because we know an object literal's type is exact). By these rules, something like new class {} is never assignable to a dictionary.

However, this raises the problem of defining the type-safe generic dictionary object.

I think Record<string, T> is the type-safe generic dictionary object. It's just that new class {} simply isn't a dictionary. So, I don't have a type-safe solution that allows new class {} to be passed as a dictionary. You need a type assertion to do that.

Why do we get the following behavior?

See #14736 (comment).

@Veetaha
Copy link
Author

Veetaha commented May 2, 2019

@ahejlsberg

the common type of all properties of an array is unknown

Interesting, I always thought it would be a union of all the properties of Array<T>.

you really don't want an array to be considered a dictionary.

It depends on what you consider a dictionary. Technically any instance of Object is a dictionary and because [] instanceof Object returns true, arrays are too. Swapping the overloads declaration would fix the problem as T[] is more specific than Record<string, unknown>. But yeah, often, considering array to be dictionary is indeed error-prone.

You need a type assertion to do that.

I'd like to avoid that. But regarding that classes may have private/protected properties that they want to hide it may be the case, otherwise, excess properties, like in derived instances should not bother the function that operates with a generic dictionary.

See #14736 (comment).

Thanks for pointing out. According to the compromise, you make for object literal types it is very easy to trick yourself, because if you define MyParams like type MyParams = { ... } instead of interface MyParams { ... }, compile error goes away and it causes a lot of confusion. It seems like you want to make object literal types invariant in one context, but covariant in other, and this may blow up.

@ahejlsberg
Copy link
Member

Interesting, I always thought it would be a union of all the properties of Array.

You're right. The correct reason we infer unknown for T when inferring from [1, 'abc'] to Record<string, T> is that an array type has no string index signature and is not an exact type. We therefore make no inferences for T and end up with the default unknown.

...compile error goes away and it causes a lot of confusion.

I agree, it is somewhat unfortunate to have this subtle difference between classes/interfaces and object type literals, but it's the best we've been able to come up with.

@Veetaha
Copy link
Author

Veetaha commented May 2, 2019

@ahejlsberg

it's the best we've been able to come up with

Hmm, I know it's not the place for proposals but, did you consider implementing explicitly invariant types like invariant class/interface {} which would let assigning only other invariant instances of the same structure.

I often miss such a feature when I am in a context where I can't be sure whether I can do Object.assign(a, b), because b may not be of the exact type I declared. And I think this is a problem and we should develop a clever solution.

@ahejlsberg
Copy link
Member

In #31687 we fix this as much as we can. The type of an enum object now has implicit index signatures that make it assignable to { [key: string]: unknown } (the first error in the original example). For the reasons outlined in #30977 (comment) we can't do the same for classes.

@ahejlsberg ahejlsberg added the Fixed A PR has been merged for this issue label Jun 2, 2019
@russelldavis
Copy link
Contributor

I agree, it is somewhat unfortunate to have this subtle difference between classes/interfaces and object type literals, but it's the best we've been able to come up with.

@ahejlsberg this was confusing for me as well -- can you help the community understand some of the tradeoffs that went into it?

In particular, at #14736 (comment) you mentioned the reason was "[so] that anything that applies to an inferred object literal type also applies to an explicitly specified object literal type". I'm wondering if it could be beneficial to revisit that, especially because there's already some precedent with typescript treating object literals differently than an equivalently-typed variable (the "Object literal may only specify known properties" check).

If typescript changed its behavior to only treat actual object literals as not having extra properties, we'd fix the inconsistency/confusion between interface and type, as well as making the language more type-safe (the error going away when switching from interface to type is hiding potential bugs). What would be the downsides to this approach?

Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug A bug in TypeScript Fixed A PR has been merged for this issue
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants