-
Notifications
You must be signed in to change notification settings - Fork 144
Description
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 error
Objects 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!