MetaMask / MetaMask/eslint-config
Disallow the use of enums
- Dominant language
- JavaScript
- Stars
- 12
- Forks
- 21
- PR merge metrics
- No merged PRs in 30d
Description
We have had multiple discussions in the past on disallowing the use of enums. We should follow through with this.
## The case for enums
The [popular argument](https://www.executeprogram.com/blog/typescript-features-to-avoid) for needing enums is this: if you have a collection of related values and you want to change one of them, you can easily do so without needing to update all instances of that value across your entire codebase.
Say you need to define a set of acceptable HTTP methods. You might use an enum to declare this:
``` typescript
enum HttpMethod {
Get = 'GET',
Post = 'POST',
}
```
Now say you have a bunch of places in your code that reference one of these values, e.g. `HttpMethod.Post`, and you want to change one of the values (e.g. `'post'` instead of `'POST'`). If you were using literal strings, you'd have to go and change all of them, but now you could merely change it in the enum, and everything would "just work".
## The case against enums
Enums have a surprisingly large number of features and quirks that are bloat at best and dangerous at worst.
The biggest problem with enums — the one that affects us most — is that they are examples of **nominal typing**. This means that if you have two enums with the same exact contents, [they are treated by TypeScript as two distinct types and are **not assignable** to each other](https://www.typescriptlang.org/play/?#code/PTAEDkHkFkElwIIBlQBUCaAFeBxAUHgKYB2ArgLagASALjQA7SE0AWA9gCagDeeooOZqAC8oAOQ4AoqjEAaPqExsAzjRHjMkAMoz5AXwIkK1Oo2bsOAdQCWrSQA8aAJwCGTcgCNCT5TwWC1UQlpOQUlVXUxTR1Q-kxSQI0AVV0wlxoAYxZIzARUAGEqMTwDIjJKWgYmVk4bO3sXDJotF3JCdy8fP34AyKlUuJVEqO1U0oAzUmIm6zZiUHIXAGtCACVCAEdSQlUACjaajgAuEyrzTgBKbtAQUAA6B5KCW9QWa18AdzmxNS+nJdAXgyLlIykIpzMhwATKAAISiSqQizyW6EABuJFANVIAHNsrZQCwXL4XAAbUlYljg5StcFtTzeZR4RYrdZbHY0XaI6oWOosBzONyEBk+O4BC4AbmeYFe71AX2IP3lbH+gMIwNB4O55w4AGY4QjTDzOCiwOjMdi8aACUTfKxwZIABoIfKoUBaBDQSQLYWdJkstabbZ7bWHPkORrNWkdRli5iSoA). This is not how TypeScript works usually! Everything else uses **structurable typing**: [if two types have the same members, they are the same type](https://www.typescriptlang.org/play/?#code/PTAEGUBUCUFUGFK2gQQDKkgTQAoEkA5AcQCgSBjAewDsBnAF1AAlJIcB9AWQFFImB5ACLhQAXlABvEqFBEApvQBcoAORFeKgDTTQOSg2Uqc-KFpIBfUAENaoKnXoBuEvQCeABznN69d5wUAFpQAJmKgABRunpQAZsysHDx8QuAAlADaANZyrrGgUXJ5LGxcvALCALrOFDQM8SVJ5eDsAOp4fOzcABowKKWcAELc0CLiUjLySqrqkGYyegaqxqbaljZ2tU4uHl5MPn6BIS0AlvQB3AAe9ABOVv4AtgBGcte24pE7RQmlycKt7UxOj1UP0hiMMtlcnECl8GmUUv8Ot1eqDhuAqmR7HViol4X82kiuihEOxwCgeKiRmFxrIFIYZnNdPopkYTLNVtZbFitgVvL5-GcjqdzhcrOR6OArPc5A9nq8wh9onEcT8mojAd1iZBSeTuJS0lkcnkYcrvo0EQSNUSSWSKTxBmiMSQYgBXaji440UD3KzZaByACOLrkDHC0sFwWUe35h2CqUkOhAoAAdKmLGQk5AAsdbAB3GgqRj566ZUDPchWF20Xb7AVBYIAJlAAEJxNGDhHtEm5AA3OTUfJBF0AcwCoFOoAC6ysABsZ4OvLQpV5pU8XrQSD6-YHg6GVeb8QCgSj7WDwMnJqlqpns3mC0XKCWy3IK1WazGIwBmFtt2uxrtgL2-aDpQI5jhOU62GcXiaogEC6t6chrq8m6+nI-pBiG9DhPueLNJaQJajqdrcA6IwXgoV5AA).
Structural typing is super convenient because it means adding a new property to an object type is a non-breaking change. But this is not the case with enums! There are several instances — in not only this repo but in other repos — where we've introduced breaking changes accidentally just by changing an enum. Even releasing a new version of a package (such as `utils`) that contains an enum can be breaking in a sense: if the package expects dependents to use a specific version of an enum and they use an older version instead, a type error will be produced. This introduces unnecessary friction.
Here are some other extraneous, strange, or outright annoying things about enums:
- Enums add code at runtime. This sounds fine, except it violates another one of the fundamental rules of TypeScript, which is that TypeScript should only ever be a layer on top of TypeScript, and that it should be possible to remove that layer without affecting the underlying code. Everything in TypeScript works this way, except enums (this is even noted at the very top of the [section on enums in the TypeScript handbook](https://www.typescriptlang.org/docs/handbook/enums.html)).
- Numeric enums can be [accessed in reverse](https://www.typescriptlang.org/docs/handbook/enums.html#reverse-mappings).
- When using numeric enums, [rearranging members breaks existing code](https://youtu.be/0fTdCSH_QEU?si=wLHkt_M0B3MKzeUg&t=68).
- Numeric enums can easily be used in other ways that break type safety ([source 1](https://www.totaltypescript.com/why-i-dont-like-typescript-enums), [source 2](https://www.jackfranklin.co.uk/blog/typescript-enums-0-values/)).
- Inlining enums via `const` can [cause surprising bugs](https://www.typescriptlang.org/docs/handbook/enums.html#const-enum-pitfalls), and ambient const enums are [incompatible](https://www.typescriptlang.org/docs/handbook/enums.html#const-enum-pitfalls) with `isolatedModules`.
## Looking ahead
As of 22.18, [Node can run TypeScript code using type erasure](https://nodejs.org/en/learn/typescript/run-natively), obviating the need to run TypeScript through a transform step (Babel, SWC, esbuild, or whatever the "fast TypeScript runner du jour" is). If we used this, it could theoretically save a bunch of time in CI and remove some tooling bloat. We could also turn on the [`--erasableSyntaxOnly` option introduced in TypeScript 5.8](https://devblogs.microsoft.com/typescript/announcing-typescript-5-8-beta/#the---erasablesyntaxonly-option) to enforce this.
To make use of this, we would need to stop using enums.
## How to stop using enums
This could be as simple as adding a lint rule:
``` json
{
"rules": {
"no-restricted-syntax": [
"error",
{
"selector": "TSEnumDeclaration",
"message": "Don't use enums. There are a number of reasons why enums are problematic, but the most important is that because they are treated nominally, not structurally, they make it very easy to accidentally introduce breaking changes. Instead, use an object + type, an array + type, or just a type. Learn more here: https://github.com/MetaMask/eslint-config/issues/417"
}
]
}
}
```
But if not enums, then what else? [This guide](https://mkosir.github.io/typescript-style-guide/#enums--const-assertion) outlines a few ways, but essentially there would be three options:
- If accessing individual enum members is important, use a literal object + `as const` + `type`. This might look something like:
```typescript
const HTTP_METHODS = {
Get: 'GET',
Post: 'POST',
} as const;
type HttpMethod = (typeof HTTP_METHODS)[keyof typeof HTTP_METHODS];
```
- If accessing enum members is not important, but iterating over enum values is, use a literal array + `as const` + a type union:
```typescript
const HTTP_METHODS = ['GET', 'POST'] as const;
type HttpMethod = (typeof HTTP_METHODS)[number];
```
- If none of the above, just use a union type:
```typescript
type HttpMethod = 'GET' | 'POST'
```
## Caveats
- If we switch away from enums we may not be able to enforce member names [as we do now](https://github.com/MetaMask/eslint-config/blob/26bae79e15068e7877d1c7af0703ade83e9e1c35/packages/typescript/src/index.mjs#L97).
Contributor guide
No contributing guide indexed for this repository
Research direction
Start with packages/typescript/src/index.mjs, especially the existing enum-member enforcement around line 97, and inspect how the shared ESLint configuration defines rules. Confirm the intended handling of enum declarations and member-name enforcement; done means the policy is implemented without leaving the documented caveat unresolved.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- typescript
- Domain
- tooling
- Issue type
- Feature
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 35/100