Skip to content

Breaking change in string enum member type in v3.6 #33403

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

Closed
aoberoi opened this issue Sep 12, 2019 · 7 comments
Closed

Breaking change in string enum member type in v3.6 #33403

aoberoi opened this issue Sep 12, 2019 · 7 comments
Labels
Working as Intended The behavior described is the intended behavior; this is not a bug

Comments

@aoberoi
Copy link

aoberoi commented Sep 12, 2019

In order to check whether a given string is a member of a string Enum, I use the code shown below. There seems to be an unintentional breaking change in v3.6.2 and later - this code compiles just fine in v3.3.x - v3.5.x.

Search Terms: enum, includes, string enum

Code

enum Colors {
  Red = 'red',
  Green = 'green',
  Blue = 'blue',
}

const a: string = 'orange';
const b = Object.values(Colors); // in <3.6: any[], in >=3.6 Color[]
const c = Object.values(Colors).includes(a)

Expected behavior: Compiles successfully 👍

Actual behavior: Error on L9 (on call site for includes(...))

Argument of type 'string' is not assignable to parameter of type 'Colors'.ts(2345)

The Object.values type definition doesn't change across these versions, and is the following:

interface ObjectConstructor {
    /**
     * Returns an array of values of the enumerable properties of an object
     * @param o Object that contains the properties and methods. This can be an object that you created or an existing Document Object Model (DOM) object.
     */
    values<T>(o: { [s: string]: T } | ArrayLike<T>): T[];

    /**
     * Returns an array of values of the enumerable properties of an object
     * @param o Object that contains the properties and methods. This can be an object that you created or an existing Document Object Model (DOM) object.
     */
    values(o: {}): any[];
}

So it seems that string Enums have begun to match the interface { [s: string]: T } where they didn't before. I can understand why it might, but is this intended?

@AnyhowStep
Copy link
Contributor

I'm on mobile but does Object.values<string>(Colors) help?

@aoberoi
Copy link
Author

aoberoi commented Sep 12, 2019

@AnyhowStep it does, but IMHO that is roughly equivalent to a cast. I can get my code to work with a cast, so that's not really my concern at the moment. But thank you for offering something that looks a little better than what I was going to do ((Object.values(Colors) as string[]).includes(a))!

My main concern is that it might not be intended, and we might want to consider this a regression and force it back. At least until that design choice can be explicitly made and then note it as part of 3.7's breaking changes.

@AnyhowStep
Copy link
Contributor

AnyhowStep commented Sep 13, 2019

It's different from a cast because the values of the enum are assignable to string. But you already knew that.

My main concern is that it might not be intended, and we might want to consider this a regression and force it back. At least until that design choice can be explicitly made and then note it as part of 3.7's breaking changes

It'll probably be marked intentional. But I'll wait for the official word!

@RyanCavanaugh
Copy link
Member

This is caused by #31687, which is already an identified breaking change

@jcalz
Copy link
Contributor

jcalz commented Sep 13, 2019

...meaning maybe this is a duplicate of #33200?

@RyanCavanaugh RyanCavanaugh added the Working as Intended The behavior described is the intended behavior; this is not a bug label Sep 13, 2019
@aoberoi
Copy link
Author

aoberoi commented Sep 13, 2019

Okay, if it was intentional then I think we're all good! Thanks for the quick turn around.

@RyanCavanaugh I was under the impression that the Breaking Changes wiki page was meant to be exhaustive, but I don't see the changes in #31687 mentioned there, the release notes, or anywhere else. Is that a mistake? Should I offer an edit to the wiki to update it?

@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.

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

No branches or pull requests

5 participants