Introduce ADR-0025: deprecate typescript enums

custom-words.txt

@@ -86,6 +86,7 @@ Yellowpages
 Yubico
 YubiKey
 YubiKeys
+typesafe
 
 # Forbidden words
 !auto-fill

docs/architecture/adr/0025-ts-deprecate-enums.md

@@ -0,0 +1,90 @@
+---
+adr: "0025"
+status: Proposed
+date: 2025-05-30
+tags: [clients, typescript]
+---
+
+# 0025 - Deprecate TypeScript Enum Types
+
+<AdrTable frontMatter={frontMatter}></AdrTable>
+
+## Context and Problem Statement
+
+TypeScript experts have long discouraged the use of enums. There are a variety of cited reasons.
+Among them:
+
+- Enums that are not `const` substantially increase output size.
+- Numeric enums implicitly cast from `number` to the enum type, which allows invalid enum values to
+  be transmitted to functions.
+- String enums are named types that require a member is used, which is inconsistent with the
+  behavior of numeric enums.
+
+These inconsistencies cause increased complexity from guard statements in the best of cases. In the
+worst cases, their limitations may be unknown, and thus unguarded. Reducing or eliminating
+
+### Detection
+
+Typescript deprecation can be linted using a fairly short ESlint plugin. The code has [already been
+contributed to main][no-enum-lint] as a suggestion. The same PR adds `FIXME` comments for each team
+to address.
+
+### Replacement code
+
+In most cases, enums are unnecessary. A readonly (`as const`) object coupled with a type alias
+avoids both code generation and type inconsistencies.
+
+```ts
+export const CipherType = {
+  Login: 1,
+  SecureNote: 2,
+  Card: 3,
+  Identity: 4,
+  SshKey: 5,
+} as const;
+
+export type CipherType = (typeof CipherType)[keyof typeof CipherType];
+
+// Can be imported together
+import { CipherType } from "./cipher-type";
+
+// Used as a type
+function doSomething(type: CipherType) {}
+
+// And used as a value (just like a regular `enum`)
+doSomething(CipherType.Card);
+```
+
+## Considered Options
+
+- **Allow enums, but advise against them** - This is the current state of affairs, as implemented by
+  the aforementioned PR. With this option, teams MUST address the readmes, but MAY address them by
+  disabling the lint.
+- **Deprecate enum use** - Allow enums to exist for historic purposes, but prohibit the introduction
+  of new ones. Increase the lint to a "warning". This lets legacy code continue running
+  indefinitely, and would allow for cases where autogenerated code introduces an enum by necessity.
+- **Eliminate enum use** - Prohibit the introduction of any new enum and replace all enums in the
+  codebase with typescript objects. Increase the lint to an "error".
+
+## Decision Outcome
+
+Chosen option: **Deprecate enum use**
+
+### Positive Consequences
+
+- Developers receive a warning in their IDE, which discourages new enums.
+- The warning can direct them to our contributing docs, where they can learn typesafe alternatives.
+- Over time, our code size decreases as enums are replaced.
+- If all teams eliminate enums in practice, the warning can be increased to an error.
+
+### Negative Consequences
+
+- Unnecessary usage may persist indefinitely on teams carrying a high tech debt.
+- The lint increases the number of FIXME comments in the code by about 10%.
+
+### Plan
+
+- Update contributing docs with patterns and best practices for enum replacement.
+- Update the reporting level of the lint to "warning".
+
+[no-enum-lint]: https://github.com/bitwarden/clients/blob/main/libs/eslint/platform/no-enums.mjs

docs/contributing/code-style/enums.md

@@ -1,25 +1,88 @@
-# TypeScript Const Enums
+# Avoid TypeScript Enums
 
-We don't use Typescript enums because they are not fully type-safe and can cause surprises. Instead
-of using enums, we use constant objects.
+TypeScript enums are not fully type-safe and can [cause surprises][enum-surprises]. Your code should
+use [constant objects][constant-object-pattern] instead of introducing a new enum.
 
-## Our Recommended Approach
+## Our Recommended Approach ([ADR-0025](../../architecture/adr/0025-ts-deprecate-enums.md))
+
+- Use the same name for your type- and value-declaration.
+- Use `type` to derive type information from the const object.
+- Create utilities to convert and identify enums modelled as primitives.
+
+:::tip
+
+This pattern should simplify the usage of your new objects, improve type safety in files that have
+adopted TS-strict, and make transitioning an enum to a const object much easier.
+
+:::
+
+### Example
+
+Given the following enum:
 
 ```ts
-export const PasskeyActions = {
-  Register: "register",
-  Authenticate: "authenticate",
+export enum CipherType = {
+  Login: 1,
+  SecureNote: 2,
+  Card: 3,
+  Identity: 4,
+  SshKey: 5,
+};
+```
+
+You can redefine it as an object like so:
+
+```ts
+const RawCipherType = {
+  Login: 1,
+  SecureNote: 2,
+  Card: 3,
+  Identity: 4,
+  SshKey: 5,
 } as const;
 
-export type PasskeyActionValue = (typeof PasskeyActions)[keyof typeof PasskeyActions];
+type RawCipherType = typeof RawCipherType;
 
-declare function usePasskeyAction(action: PasskeyActionValue): void;
+export type CipherType = RawCipherType[keyof RawCipherType];
 
-usePasskey(PasskeyActions.Register); // ✅ Valid
-usePasskey(0); // ❌ Invalid
+export const CipherType: Readonly<Record<keyof RawCipherType, CipherType>> =
+  Object.freeze(RawCipherType);
 ```
 
-## Resources Used
+And use it like so:
+
+```ts
+// Can be imported together
+import { CipherType } from "./cipher-type";
+
+// Used as a type
+function doSomething(type: CipherType) {}
+
+// And used as a value (just like a regular `enum`)
+doSomething(CipherType.Card);
+```
+
+The following utilities may assist introspection:
+
+```ts
+import { CipherType } from "./cipher-type";
+
+const namesByCipherType = new Map<CipherType, keyof CipherType>(
+  Array.fromEntries(Object.entries(CipherType), ([k, v]) => [v, k]),
+);
+
+export function isCipherType(value: number): value is CipherType {
+  return namesByCipherType.has(value);
+}
+
+export function asCipherType(value: number): CipherType | undefined {
+  return isCipherType(value) ? value : undefined;
+}
+
+export function nameOfCipherType(name: CipherType): keyof CipherType | undefined {
+  return namesByCipherType.get(value);
+}
+```
 
-- https://dev.to/ivanzm123/dont-use-enums-in-typescript-they-are-very-dangerous-57bh
-- https://www.typescriptlang.org/docs/handbook/enums.html#objects-vs-enums
+[enum-surprises]: https://dev.to/ivanzm123/dont-use-enums-in-typescript-they-are-very-dangerous-57bh
+[constant-object-pattern]: https://www.typescriptlang.org/docs/handbook/enums.html#objects-vs-enums