delta icon indicating copy to clipboard operation
delta copied to clipboard

Proposal: Use Discriminated Union for Op Type for Stronger Type Safety

Open mlesin opened this issue 9 months ago • 1 comments

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

  1. 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  
    
  2. 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  
      }  
    } 
    
  3. Clearer Intent The type explicitly defines three distinct operation cases, making the API easier to understand.

  4. 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!

mlesin avatar Mar 31 '25 15:03 mlesin

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

mlesin avatar Mar 31 '25 17:03 mlesin