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

Object.values(Enum) should return an array of Enum's value's type #33200

Closed
jimcullenaus opened this issue Sep 2, 2019 · 11 comments
Closed

Object.values(Enum) should return an array of Enum's value's type #33200

jimcullenaus opened this issue Sep 2, 2019 · 11 comments
Labels
Working as Intended The behavior described is the intended behavior; this is not a bug

Comments

@jimcullenaus
Copy link

Search Terms: Object.values

Code

enum Role {
	Admin = 'ADMIN',
	Results = 'RESULTS',
	Finish = 'FINISH',
	Config = 'CONFIG'
}

Object.values(Role).forEach(role => {
	console.log(typeof role); // prints 'string' four times
}); 

Object.values(Role).includes('hello'); // errors at build time

Expected behavior: Object.values(EnumType) at build time should be returning an array of the same type as the values of the EnumType.

Actual behavior: Object.values(EnumType).includes('enumValueType') errors out with

error TS2345: Argument of type '"enumValueType"' is not assignable to parameter of type 'EnumType'.

It appears as though, at build time, Object.values(EnumType) is parsed as returning an array of EnumTypes.

Error first encountered on Typescript 3.6.2. Still present as of typescript@next.

Playground Link: Playground link. Playground does not appear to be correctly using ES2017 and Typescript 3.6.2/3.7.0 are not available.

@jcalz
Copy link
Contributor

jcalz commented Sep 3, 2019

The error looks reasonable to me. Why are you checking for 'hello' instead of, say, Role.Config? If you check Object.values(Role).includes(Role.Config) there's no error.

Maybe you're not aware that the type named Role is not the type of the value named Role ? The value Role that exists at runtime is a mapping from keys to values, while the type named Role is the union of the value types. So I expect Object.values(Role) to produce an Array<Role>, which it does. That is, your "expected behavior" is what is actually happening.

@Lauraducky
Copy link

Lauraducky commented Sep 3, 2019

Our real-world use case here is that the input to includes comes from user input.

Object.values should be returning an array of the values contained on that object. The value of Role.Config should be the string CONFIG, so there should be no error.

Perhaps more importantly, prior to 3.6.2, this functionality worked exactly as expected. It was the (supposedly non-breaking, were semver followed) update that broke this purely at build time.

@jcalz
Copy link
Contributor

jcalz commented Sep 3, 2019

Here's the relevant type definitions for Object.values():

interface ObjectConstructor {
  values<T>(o: { [s: string]: T } | ArrayLike<T>): T[]; // first overload
  values(o: {}): any[]; // second overload
}

In TS3.5 and below, Object.values(Role) couldn't see Role as an indexable type, so it invoked the second overload signature above and returned an Array<any>, at which point all type safety on the array elements was lost.

But TS3.6 included #31687, allowing enum objects to be seen as implicitly having index signatures. So, starting in TS3.6, enum Role {...} is now assignable to a value of type {[k: string]: Role}. This is probably exactly what you want. And this now allows Object.values(Role) to invoke the first overload signature above and return Array<Role>, which is also probably exactly what you want.


That means in TS3.5 the compiler happily allowed you to call Object.values(Role).includes("randomThing") as well as Object.values(Role).includes(12345) and Object.values(Role).includes({name: "teapot", size: "little", height: "short", width: "stout"}). That's really too permissive; it probably should be an error to look for pottery in an array of enum values.

Starting in TS3.6, you are now forced to deal with the fact that Array<T>.includes(val: T) accepts a T and not something wider. In this case, it wants you to check a Role and not a string. Think that's too restrictive? That was brought up as #32201 and there are reasons and workarounds mentioned and linked in there

The short answer here is you should probably widen Object.values(Role) to string[] before doing includes() on some random string, if that's what you want (e.g., (Object.values(Role) as string[]).includes("randomString")).

@jimcullenaus
Copy link
Author

I really don't think what we are asking for is particularly complex or confusing. At runtime, Object.values(Role) returns an array of strings, because Role is an enum where each of the entries has a value of a string. We would like the build time to respect that. We have defined an enum with string values, so Object.values should be treated as an array of strings, or at the very least should not break when accessed as such.

In TS3.5 and below, Object.values(Role) couldn't see Role as an indexable type, so it invoked the second overload signature above and returned an Array, at which point all type safety on the array elements was lost.

This is certainly disappointing, and it is nice to see an attempt to tighten things up a bit. But unfortunately they have not been tightened up correctly.

this now allows Object.values(Role) to invoke the first overload signature above and return Array, which is also probably exactly what you want

No, we want Object.values(Role) to return an array of the values contained within each Role. Very explicitly not Array<Role>. That's what the "values" in Object.values is referring to. Role.Admin being one instance of Role, and 'ADMIN' being the value of that Role.

We would also like it if breaking changes were not made in a minor version update. If this breaking change is absolutely necessary, it should be made with TypeScript version 4.x, not in a minor version update. It's very notable that according to Dependabot, this version update saw an over 10% failure rate, despite being a minor update, when most other builds have less than 3%. Heck, even the 2.9.2 → 3.0.1 breaking change had a lower rate of failure than this one! Surely Microsoft can not consider that acceptable.

@fatcerberus
Copy link

fatcerberus commented Sep 3, 2019

No, we want Object.values(Role) to return an array of the values contained within each Role. Very explicitly not Array<Role>

Except that's exactly what Array<Role> is. Role is, in practice, a union of the possible values of the Role enum, which can be seen here:

enum Role {
	Admin = 'ADMIN',
	Results = 'RESULTS',
	Finish = 'FINISH',
	Config = 'CONFIG'
}

declare let x: Role;
let y: 'ADMIN' | 'RESULTS' | 'FINISH' | 'CONFIG' = x;  // ok

You wouldn't argue that if you had an array of type number[], you should be able to go .includes("foo"), right? That's obviously a category error. So your actual pain point comes from the fact that TS has literal types in addition to the standard primitives. Since Array is generic, how could you decide which uses of T in that type should be widened (if they're literals) vs. ones that shouldn't? There are cases where widening the generic type wouldn't be desirable, so TS is conservative and doesn't do it.

tl;dr: TS knows statically at compile time the exact string values contained in Role-the-object, so it gives you an Array<"ADMIN"|"RESULTS"|"FINISH"|"CONFIG">, or Array<Role> for short. You then get stung because literal types can't safely be widened in a generic context.

@jcalz
Copy link
Contributor

jcalz commented Sep 3, 2019

We would also like it if breaking changes were not made in a minor version update. If this breaking change is absolutely necessary, it should be made with TypeScript version 4.x, not in a minor version update.

For better or worse, TS doesn't do semantic versioning; see #31761

@jcalz
Copy link
Contributor

jcalz commented Sep 3, 2019

To reiterate: this all seems to be working as intended. You should change your code to something like

enum Role {
  Admin = 'ADMIN',
  Results = 'RESULTS',
  Finish = 'FINISH',
  Config = 'CONFIG'
}

const allRoles: string[] = Object.values(Role); // this is what I want, compiler!

allRoles.forEach(role => { console.log(typeof role);  }); 

allRoles.includes('hello'); 

Playground link

which communicates your intent to the compiler (allRoles should be a string[] and not an any[] or a Role[]), and works in both TS3.5 and TS3.6. I'll stop spamming contributing to this thread now and wait for official word from a TS repo maintainer. Good luck!

@sandersn sandersn changed the title Object.values(Enum).includes(EnumValue) errors at build time Object.values(Enum) should return an array of Enum's value's type Sep 3, 2019
@sandersn
Copy link
Member

sandersn commented Sep 3, 2019

The original PR suggests a workaround: Since numeric enums use older, looser rules than string enums, you can get close to the old behaviour by adding an entry to Role like Dummy = 0 (or your favourite number).

However, anytime something comes from user input, you should expect to assert or narrow its type before the compiler understands it. That code will necessarily involve some bending of the type system since the checks are runtime, but the errors you're getting are at compile-time. For 3.6, your checks that strings from user input are actually members of an enum now need associated assertions to make the type system leave you alone until the check is done. I would personally write it as

Object.values(Role).includes('hello' as Role)

@sandersn sandersn added the Working as Intended The behavior described is the intended behavior; this is not a bug label Sep 3, 2019
@typescript-bot
Copy link
Collaborator

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

2 similar comments
@typescript-bot
Copy link
Collaborator

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

@typescript-bot
Copy link
Collaborator

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

Asaf-S added a commit to Asaf-S/TypeScript that referenced this issue Sep 7, 2021
Array's `include` function checks whether or not the `searchElement` exists in it, and returns the appropriate boolean value as an answer.
However, in the current implementation, TypeScript is requesting the type of `searchElement` to be a member of the array, instead of receiving any value as `searchElement`.
This in turn forces the user to know in advance whether or not the `searchElement` is a member of the array, which defeats the whole purpose of using the 'include' function. The way users currently workaround that problem is by overriding the TypeScript mechanism by using "as", as suggested by @sandersn in here: microsoft#33200 (comment) . See also https://stackoverflow.com/questions/43804805/check-if-value-exists-in-enum-in-typescript .

I believe that the purpose of the `include` function is to allow any variable, even if its type is unknown, to be searched in the array and get a boolean answer, and therefore I suggest the following changes.

Playground:
https://www.typescriptlang.org/play?#code/LAKApgdgrgtgBAMQEoFUCSAVAynA3qOOAQQAUSAZAUTgF44ByAQ3oBoC4AhIgOR6NoYAjVqAC+oUADMoEAMYAXAJYB7CHEUBnAGqMANooAmWKAAcTygE7ywBhBaiL5ACigawFtBBNR5ALjgyANYQygDuEACU-oLKyrpgjGr4IIQA9ABUEilwGAAWYHCScbphihAA5nD6EAWacPL5cBpQ5eVgGtYGcKGMAJ71yt2WgYwWyjJdjixwiZPyQxaBGtOCPt25-QaDaHC5jABuBfKDyocWFoYFGL0mYFiyFybyAPzsAOobcFtwO3uHAzMNG4rPU9vMGgVXO5PN55nVkOhsNNQrlFPEfvR4CF5s0LLV5ni9Lp+nUnLNQYx5PQNKCCii4gVvBZzG44MpJAENGVKgADMqyXRQAxgHkRV4gdiEPHyKAWNQAeUEACswAoAHT7PRQdpOBGYLARNX8wXCjQuYEwtaMGl67ARADckrg7HSqXY0tlCuVqvkGq1OttBqNchNOqhHi8PgdcFSqRyuTGoRpiTg7jGFn8RAs5VgkHm7PqNwK9CCIXC9HUNOxgK55QgjEE6OOcBMo0YMDA1gsbI58iLDED9DVOX7JYgwTCEArdWr1tr9cbR0GfduA9Q+rVXF4vCHTgATABmAAsAFYImIsrJVBoGWqSuUnJodPojKZzFYbHYHM5A2rSBRKAiaNYzgSgAA9bgUGx-HkewwFAK8IBveI72UB8nz0QxjDMSxOi-RwnHoYQgPtGM43AyDOhguCEOvW970fbRMNfHCP1sewCPoWR6BIsjQIgn1oMKPQ3FopD6LQxjnywt9cM-DjnGgXRdF4kCKMEgx-EkET4JARDkLAVD0KYl9sPfPCFKcXBRFU8iBKgzThN0NwgA
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Working as Intended The behavior described is the intended behavior; this is not a bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants