Proposal: Use Discriminated Union for Op Type for Stronger Type Safety
Summary
The current Op interface allows invalid states (e.g., missing operations or multiple operations). I propose replacing it with a discriminated union type to enforce exactly one valid operation type per object, improving type safety and developer experience.
Current Implementation
The current Op interface:
interface Op {
// only one property out of {insert, delete, retain} will be present
insert?: string | Record<string, unknown>;
delete?: number;
retain?: number | Record<string, unknown>;
attributes?: AttributeMap;
}
This allows:
- Objects with no operation (no insert, delete, or retain).
- Objects with multiple operations (e.g.,
{ insert: "text", delete: 5 }).
Proposed Solution
Replace the interface with a discriminated union type :
type Op = (
| { insert: string | Record<string, unknown> }
| { delete: number }
| { retain: number | Record<string, unknown> }
) & { attributes?: AttributeMap };
This ensures that every Op must have exactly one operation type (insert, delete, or retain).
Benefits
-
Eliminates Invalid States The proposed type prevents:
Objects with no operation :
const invalidOp: Op = { attributes: {} }; // ❌ Compile-time errorObjects with multiple operations :
const invalidOp: Op = { insert: "text", delete: 5 }; // ❌ Compile-time error -
Improved Tooling & Autocompletion Editors/IDEs will suggest valid properties based on the operation type. Type guards simplify control flow:
function processOp(op: Op) { if ("insert" in op) { // `op` is inferred as `{ insert: ... } & { attributes? ... }` console.log(op.insert); // Valid console.log(op.delete); // ❌ Property 'delete' does not exist } } -
Clearer Intent The type explicitly defines three distinct operation cases, making the API easier to understand.
-
Simplified Validation Logic No need for manual property checks:
// Before (current interface): if ('insert' in op) { /* ... */ } else if ('delete' in op) { /* ... */ } else if ('retain' in op) { /* ... */ } // After (proposed type): if (op.insert) { /* TypeScript infers the correct type */ }
Example Use Cases
Invalid Code (Current Behavior)
const invalidOp: Op = { insert: "text", delete: 5 }; // ✅ Allowed (but incorrect!)
Valid Code (Proposed Behavior)
const validInsertOp: Op = { insert: "text", attributes: { bold: true } }; // ✅
const validDeleteOp: Op = { delete: 5 }; // ✅
const invalidMixedOp: Op = { insert: "text", delete: 5 }; // ❌ Compile-time error
Warning
This is a breaking change for typescript users of the library, but the benefits justify the adjustment.
Let me know your thoughts!
If attributes property is not applicable to delete op, the proposed code could be
type Op =
| { insert: string | Record<string, unknown>; attributes?: AttributeMap }
| { delete: number }
| { retain: number | Record<string, unknown>; attributes?: AttributeMap };
this question also unclear for me while reading original type without diving deep into full source code