|
| 1 | +--- |
| 2 | +adr: "0025" |
| 3 | +status: Proposed |
| 4 | +date: 2025-05-30 |
| 5 | +tags: [clients, typescript] |
| 6 | +--- |
| 7 | + |
| 8 | +# 0025 - Deprecate TypeScript Enum Types |
| 9 | + |
| 10 | +<AdrTable frontMatter={frontMatter}></AdrTable> |
| 11 | + |
| 12 | +## Context and Problem Statement |
| 13 | + |
| 14 | +TypeScript experts have long discouraged the use of enums. There are a variety of cited reasons. |
| 15 | +Among them: |
| 16 | + |
| 17 | +- Enums that are not `const` substantially increase output size. |
| 18 | +- Numeric enums implicitly cast from `number` to the enum type, which allows invalid enum values to |
| 19 | + be transmitted to functions. |
| 20 | +- String enums are named types that require a member is used, which is inconsistent with the |
| 21 | + behavior of numeric enums. |
| 22 | + |
| 23 | +These inconsistencies cause increased complexity from guard statements in the best of cases. In the |
| 24 | +worst cases, their limitations may be unknown, and thus unguarded. |
| 25 | + |
| 26 | +### Detection |
| 27 | + |
| 28 | +TypeScript deprecation can be linted using a fairly short ESLint plugin. The code has [already been |
| 29 | +contributed to main][no-enum-lint] and is configured as [an error-level |
| 30 | +lint][no-enum-configuration]. The same PR adds `FIXME` comments for each team to address. |
| 31 | + |
| 32 | +### The Enum-like Pattern |
| 33 | + |
| 34 | +In most cases, enums are unnecessary. A readonly (`as const`) object coupled with a type alias |
| 35 | +avoids both code generation and type inconsistencies. |
| 36 | + |
| 37 | +```ts |
| 38 | +// declare the raw data and reduce repetition with an internal type |
| 39 | +const _CipherType = { |
| 40 | + Login: 1, |
| 41 | + SecureNote: 2, |
| 42 | + Card: 3, |
| 43 | + Identity: 4, |
| 44 | + SshKey: 5, |
| 45 | +} as const; |
| 46 | + |
| 47 | +type _CipherType = typeof _CipherType; |
| 48 | + |
| 49 | +// derive the enum-like type from the raw data |
| 50 | +export type CipherType = _CipherType[keyof _CipherType]; |
| 51 | + |
| 52 | +// assert that the raw data is of the enum-like type |
| 53 | +export const CipherType: Readonly<{ [K in keyof _CipherType]: CipherType }> = |
| 54 | + Object.freeze(_CipherType); |
| 55 | +``` |
| 56 | + |
| 57 | +This code creates a `type CipherType` that allows arguments and variables to be typed similarly to |
| 58 | +an enum. It also strongly types the `const CiperType` so that direct accesses of its members |
| 59 | +preserve type safety. This ensures that type inference properly limits the accepted values to those |
| 60 | +allowed by `type CipherType`. Without the type assertion, the compiler infers `number` in these |
| 61 | +cases: |
| 62 | + |
| 63 | +```ts |
| 64 | +const s = new Subject(CipherType.Login); // `s` is a `Subject<CipherType>` |
| 65 | +const a = [CipherType.Login, CipherType.Card]; // `a` is an `Array<CipherType>` |
| 66 | +const m = new Map([[CipherType.Login, ""]]); // `m` is a `Map<CipherType, string>` |
| 67 | +``` |
| 68 | + |
| 69 | +:::warning |
| 70 | + |
| 71 | +- Types that use enums like [computed property names][computed-property-names] issue a compiler |
| 72 | + error with this pattern. [This issue is fixed as of TypeScript 5.8][no-member-fields-fixed]. |
| 73 | +- Certain objects are more difficult to create with this pattern. This is explored in |
| 74 | + [Appendix A](#appendix-a-mapped-types-and-enum-likes). |
| 75 | + |
| 76 | +::: |
| 77 | + |
| 78 | +## Considered Options |
| 79 | + |
| 80 | +- **Allow enums, but advise against them** - Allow enum use to be decided on a case-by-case basis by |
| 81 | + the team that owns the enum. Reduce the level of the lint to a "suggestion". |
| 82 | +- **Deprecate enum use** - Allow enums to exist for historic or technical purposes, but prohibit the |
| 83 | + introduction of new ones. Reduce the lint to a "warning" and allow the lint to be disabled. |
| 84 | +- **Eliminate enum use** - This is the current state of affairs. Prohibit the introduction of any |
| 85 | + new enum and replace all enums in the codebase with typescript objects. Prohibit disabling of the |
| 86 | + lint. |
| 87 | + |
| 88 | +## Decision Outcome |
| 89 | + |
| 90 | +Chosen option: **Deprecate enum use** |
| 91 | + |
| 92 | +### Positive Consequences |
| 93 | + |
| 94 | +- Allows for cases where autogenerated code introduces an enum by necessity. |
| 95 | +- Developers receive a warning in their IDE to discourage new enums. |
| 96 | +- The warning can direct them to our contributing docs, where they can learn typesafe alternatives. |
| 97 | +- Our compiled code size decreases when enums are replaced. |
| 98 | +- If all teams eliminate enums in practice, the warning can be increased to an error. |
| 99 | + |
| 100 | +### Negative Consequences |
| 101 | + |
| 102 | +- Unnecessary usage may persist indefinitely on teams carrying a high tech debt. |
| 103 | +- The lint increased the number of FIXME comments in the code by about 10%. |
| 104 | + |
| 105 | +### Plan |
| 106 | + |
| 107 | +- Update contributing docs with patterns and best practices for enum replacement. |
| 108 | +- Update the reporting level of the lint to "warning". |
| 109 | + |
| 110 | +## Appendix A: Mapped Types and Enum-likes |
| 111 | + |
| 112 | +Mapped types cannot determine that a mapped enum-like object is fully assigned. Code like the |
| 113 | +following causes a compiler error: |
| 114 | + |
| 115 | +```ts |
| 116 | +const instance: Record<CipherType, boolean> = { |
| 117 | + [CipherType.Login]: true, |
| 118 | + [CipherType.SecureNote]: false, |
| 119 | + [CipherType.Card]: true, |
| 120 | + [CipherType.Identity]: true, |
| 121 | + [CipherType.SshKey]: true, |
| 122 | +}; |
| 123 | +``` |
| 124 | + |
| 125 | +#### Why does this happen? |
| 126 | + |
| 127 | +The members of `const _CipherType` all have a [literal type][literal-type]. `_CipherType.Login`, for |
| 128 | +example, has a literal type of `1`. `type CipherType` maps over these members, aggregating them into |
| 129 | +the structural type `1 | 2 | 3 | 4 | 5`. |
| 130 | + |
| 131 | +`const CipherType` asserts its members have `type CipherType`, which overrides the literal types the |
| 132 | +compiler inferred for the member in `const _CipherType`. The compiler sees the type of |
| 133 | +`CipherType.Login` as `type CipherType` (which aliases `1 | 2 | 3 | 4 | 5`). |
| 134 | + |
| 135 | +Now consider a mapped type definition: |
| 136 | + |
| 137 | +```ts |
| 138 | +// `MappedType` is structurally identical to Record<CipherType, boolean> |
| 139 | +type MappedType = { [K in CipherType]: boolean }; |
| 140 | +``` |
| 141 | + |
| 142 | +When the compiler examines `instance`, it only knows that the type of each of its members is |
| 143 | +`CipherType`. That is, the type of `instance` to the compiler is |
| 144 | +`{ [K in 1 | 2 | 3 | 4 | 5]?: boolean }`. This doesn't sufficiently overlap with `MappedType`, which |
| 145 | +is looking for `{ [1]: boolean, [2]: boolean, [3]: boolean, [4]: boolean, [5]: boolean }`. The |
| 146 | +failure occurs, because the inferred type can have fewer fields than `MappedType`. |
| 147 | + |
| 148 | +### Workarounds |
| 149 | + |
| 150 | +**Option A: Assert the type is correct.** You need to manually verify this. The compiler cannot |
| 151 | +typecheck it. |
| 152 | + |
| 153 | +```ts |
| 154 | +const instance: MappedType = { |
| 155 | + [CipherType.Login]: true, |
| 156 | + // ... |
| 157 | +} as MappedType; |
| 158 | +``` |
| 159 | + |
| 160 | +**Option B: Define the mapped type as a partial.** Then, inspect its properties before using them. |
| 161 | + |
| 162 | +```ts |
| 163 | +type MappedType = { [K in CipherType]?: boolean }; |
| 164 | +const instance: MappedType = { |
| 165 | + [CipherType.Login]: true, |
| 166 | + // ... |
| 167 | +}; |
| 168 | + |
| 169 | +if (CipherType.Login in instance) { |
| 170 | + // work with `instance[CipherType.Login]` |
| 171 | +} |
| 172 | +``` |
| 173 | + |
| 174 | +**Option C: Use a collection.** Consider this approach when downstream code reflects over the result |
| 175 | +with `in` or using methods like `Object.keys`. |
| 176 | + |
| 177 | +```ts |
| 178 | +const collection = new Map([[CipherType.Login, true]]); |
| 179 | + |
| 180 | +const instance = collection.get(CipherType.Login); |
| 181 | +if (instance) { |
| 182 | + // work with `instance` |
| 183 | +} |
| 184 | + |
| 185 | +const available = [CipherType.Login, CipherType.Card]; |
| 186 | +if (available.includes(CipherType.Login)) { |
| 187 | + // ... |
| 188 | +} |
| 189 | +``` |
| 190 | + |
| 191 | +[computed-property-names]: |
| 192 | + https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Object_initializer#computed_property_names |
| 193 | +[literal-type]: https://www.typescriptlang.org/docs/handbook/2/everyday-types.html#literal-types |
| 194 | +[no-enum-lint]: https://github.com/bitwarden/clients/blob/main/libs/eslint/platform/no-enums.mjs |
| 195 | +[no-enum-configuration]: |
| 196 | + https://github.com/bitwarden/clients/blob/032fedf308ec251f17632d7d08c4daf6f41a4b1d/eslint.config.mjs#L77 |
| 197 | +[no-member-fields-fixed]: |
| 198 | + https://devblogs.microsoft.com/typescript/announcing-typescript-5-8-beta/#preserved-computed-property-names-in-declaration-files |
0 commit comments