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

Make enum indexes non-enumerable #53973

Closed
2 of 5 tasks
svr93 opened this issue Apr 23, 2023 · 8 comments
Closed
2 of 5 tasks

Make enum indexes non-enumerable #53973

svr93 opened this issue Apr 23, 2023 · 8 comments
Labels
Declined The issue was declined as something which matches the TypeScript vision Suggestion An idea for TypeScript

Comments

@svr93
Copy link

svr93 commented Apr 23, 2023

Suggestion

For now TS numeric enums are compiled with indexes so the result can be confusing at runtime:

Object.keys(enum_); // "A", "B", 0, 1

The behaviour is unusual so many community members just suggest not use enums at all. It’s the key point because some really useful language feature became “forbidden” because no one understands how to use it correctly:

  1. TypeScript Enums are Terrible. Here's Why

  2. Why TypeScript enums suck

  3. TypeScript Enums are Bad // Alternatives to use

My idea (not only my - see) is to use enumerable: false for indexes.

UPD. Some steps for string & numeric enums unification were already made for TS 5.0; the whole part of roadmap concerned the unification:

Unification Between Numeric and Literal enums

@ahejlsberg wrote some meaningful things:

The distinction between these two kinds of enums is subtle and has been the source of a fair amount of confusion over time.

🔍 Search Terms

enums, enumerable, iteration

✅ Viability Checklist

My suggestion meets these guidelines:

  • This wouldn't be a breaking change in existing TypeScript/JavaScript code
  • This wouldn't change the runtime behavior of existing JavaScript code
  • This could be implemented without emitting different JS based on the types of the expressions
  • This isn't a runtime feature (e.g. library functionality, non-ECMAScript syntax with JavaScript output, new syntax sugar for JS, etc.)
  • This feature would agree with the rest of TypeScript's Design Goals.

Other related issues:

  1. #4753 (Suggestion: Iterate through Enum values)

  2. #2457 (Allow iterating through enum variants with for of)

  3. #51150

  4. #48193 (see the comment)

@jcalz
Copy link
Contributor

jcalz commented Apr 23, 2023

(Note: I'm not a TS team member, so I do not speak authoritatively)

#4753 was already declined, and this is just asking for the same thing in a different way, except more intrusively because it breaks existing behavior.

I'd love to go back in time to change enums, but in this timeline I think we're probably stuck with this. If you want to iterate through the forward mappings only you can suppress numeric-like keys yourself:

enum E { A, B }
console.log(Object.keys(E).filter(k => "" + (+k) !== k)); // "A", "B"

(which works, ignoring weirdnesses like #48956). No, it's not ergonomic, but at least you don't have to wait for TypeScript enums to change.

@svr93
Copy link
Author

svr93 commented Apr 23, 2023

@jcalz
Both solutions will break existing behaviour but my solution is closer to TS Design Goals because for now different JS emitted for different types of enums: indexed keys only added for numeric enums.

#4753 suggests more complicated solution because additional field should be used - [Symbol.iterator]. So it was the reason to decline it. You can check it here:

…to emit such a helper for all enums that every TypeScript user writes…

Also,

I'd love to go back in time to change enums, but in this timeline I think we're probably stuck with this

But what’s the reason? TS 5.0 already broke current enum behaviour (UPD: see 50528) and it’s not the first time (also - this, this). Do you see some limiting design decisions that block my suggestion? If so, can you share your thoughts?

but at least you don't have to wait for TypeScript enums to change.

I strongly disagree with this point because developers listen the “community voice” and don’t use the feature at all - you can check links I provided before. So it leads to more complicated solutions with custom enum implementations; also const = … as const noisy constructions are used:

export const LoginMode = {
    device: 'device',
    email:   'email',
    social:  'social',
} as const;
export type LoginMode = keyof typeof LoginMode;

(Thanks @basarat for provided example)

UPD. Yes, it works, but it’s not enough semantical; it has drawbacks similar to using <div />s for all HTML elements - only keyword is different here - const vs div 😃 Some thoughts: the “real JavaScript” solution looks less JavaScript-y than enum because it uses type, keyof & typeof - all keywords are not exist or not semantically related with JavaScript. So, why "runtime purity" > clean code?

I think better to use single solution for real dev cases instead of keeping current non-ergonomic behaviour.

@jcalz
Copy link
Contributor

jcalz commented Apr 23, 2023

Both solutions will break existing behaviour

How would making enum objects iterable break people? Do people depend on enum objects not being iterable?

#4753 suggests _more complicated solution ... So it was the reason to decline it.

This solution is also more complicated because it Object.defineProperty() instead of just setting a property. Am I missing something?

What’s the reason? Do you see some limiting design decisions that block my suggestion? If so, can you share your thoughts?

I assume that since enumerable reverse mappings in enum objects have been around for quite a while there might well be real world code that enumerates them. But I don't have any concrete statistics here so maybe it's an acceptable break? 🤷‍♂️ I expect that the TS team will decline this, but I'm just a community member that expects reverse mappings to be present in the key list, and I don't intend to speak on behalf of others, so I'll stop speculating and let them give reasons if they do decline it.

TS 5.0 already broke current enum behaviour and it’s not the first time.

Cross-referencing to #50528.

I strongly disagree with this point because developers listen the “community voice” and don’t use the feature at all.

Unfortunately I think that expression-level TypeScript features that don't exist in JavaScript should probably be avoided where possible, including enums (and parameter properties, and I guess namespace and... is that it? I guess decorators are off that list now). If enums are good for TypeScript they'd be good for JavaScript and then there'd be some actual standard behavior that we could use to make decisions. If there were a time machine and MS sent retrofactor teams back to right past wrongs the result would probably be to destroy enums entirely instead of just enumerable reverse mappings.

I'll stop interacting now to let you and others discuss if you want. Good luck!

@svr93
Copy link
Author

svr93 commented Apr 23, 2023

Some thoughts for other community members:

  1. Do people depend on enum objects not being iterable?

The short answer is “no” but I prefer to not guess it because technically adding the new “system” field is breaking change. For example, someone can write lodash-like helper that returns the last element of collection if iterable and returns last indexed value otherwise. We don’t know what people expect for this enum:

enum X {
  X = 2,
  Y = 1,
}
  1. This solution is also more complicated because it Object.defineProperty() instead of just setting a property. Am I missing something?

I mean “complicated in runtime” when some helpers should be added to prototype chain in case of [Symbol.iterator]

  1. since enumerable reverse mappings in enum objects have been around for quite a while there might well be real world code that enumerates them

Reverse mapping & enumeration are not related; idk who wanna to depend on index enumeration but technically it’s a breaking change. I pointed earlier that I prefer to not speculate what breaking change is.

  1. expression-level TypeScript features that don't exist in JavaScript should probably be avoided where possible, including enums

When some functionality already exists and it’s not private then someone will be use it. If enums are bad why not remove it completely (with deprecation step)? Also, enums behaviour is not “frozen” because some breaking changes were added earlier (see previous comment). Keeping non-ergonomic behaviour is OK only for JS (“don’t break the web”). Namespaces are also not bad-by-default just because JS already has modules. Namespace functionality OK and maybe preferred for some real-world cases (I know at least 2 cases but I don’t wanna to discuss it here).

@svr93
Copy link
Author

svr93 commented Apr 24, 2023

P.S. A good example that TS enums behaviour was changed for better ergonomic is my old issue:

#15591

My suggestion was applied in TS 5.0 😉

@RyanCavanaugh RyanCavanaugh added Suggestion An idea for TypeScript Declined The issue was declined as something which matches the TypeScript vision labels Apr 24, 2023
@RyanCavanaugh
Copy link
Member

I don't think this change alone meaningfully moves the needle on whether you should want to use enums or not; there are plenty of other complaints people have which are not even in-principle addressable.

Breaking a ton of working code (I assure you it exists) to change not-wrong behavior that's been shipping for 10 years just isn't a good trade-off.

@svr93
Copy link
Author

svr93 commented Apr 24, 2023

@RyanCavanaugh
Thanks for your response!
Ok, all my thoughts was presented here so I will not continue the long discussion; I just wanted to show why people really complain:

  1. type safety problem (Incomplete enum type check #15591, also you can check @basarat video - https://youtu.be/3aUHskjF7pc?t=115). It was already fixed in TS 5.0
  2. lookup problem (current problem - https://youtu.be/3aUHskjF7pc?t=160)

Issues with “unstable” enum indexes and nominal typing I don’t consider as a problem because it’s how enum works in other languages - the enum idea is to provide stable keys, not values - because enums allow a developer to define a set of named constants. With best regards to @basarat nevertheless I don't know why he has a different opinion with considering enum & union type are 100% same terms. For this issues/"issues" I suggest to follow Handbook terms. Serializing numeric enums as raw values & using it for API looks like a code smell & should be avoided everywhere because for most cases it hasn't stable values for whole years (of course, some counter examples exist - HTTP error codes, XHR statuses, etc)

So, following the great work that was done for enums unification (release 5.0, special thanks to you, @ahejlsberg and @sandersn for working with the PR), some additional changes can be made & it will cover 80-90% problem cases.

You are welcome to close the issue or leave it for other opinions

@typescript-bot
Copy link
Collaborator

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

@typescript-bot typescript-bot closed this as not planned Won't fix, can't repro, duplicate, stale Jun 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Declined The issue was declined as something which matches the TypeScript vision Suggestion An idea for TypeScript
Projects
None yet
Development

No branches or pull requests

4 participants