fix(types): filter symbol keys in Rpc.Serializable mapped type

[dmmulroy]

Problem

Rpc.Serializable<T> fails to match valid serializable types when they include symbol keys (e.g., Symbol.dispose from Disposable).

When using WorkflowStep.do() with a Durable Object stub return value:

const result = await step.do("get state", async () => stub.getState());

TypeScript errors:

Type '{ state: string; } & Disposable' is not assignable to type 'Serializable<...>'.
  Types of property '[Symbol.dispose]' are incompatible.
    Type '() => void' is not assignable to type 'never'.

Reproduction

https://github.com/dmmulroy/workerd-rpc-serializable-type-repro

Cause

The mapped type uses a value-side conditional that maps symbol keys to never instead of filtering them out:

{
  [K in keyof T]: K extends number | string ? Serializable<T[K]> : never;
}

This creates an object type that has the symbol keys but with never values, causing assignability failures.

Fix

Use key remapping with as clause to filter out non-string/number keys entirely:

{
-  [K in keyof T]: K extends number | string ? Serializable<T[K]> : never;
+  [K in keyof T as K extends string | number ? K : never]: Serializable<T[K]>;
}

Safety & Backwards Compatibility

This change is safe because:

1. No runtime impact - purely a type-level fix, no generated code changes
2. Strictly more permissive - types that passed before still pass; types that failed incorrectly now pass
3. Semantically correct - symbol keys are not serializable over RPC anyway, so filtering them from the type constraint matches actual runtime behavior
4. Same intent - the original code intended to only validate string/number keys; this fix achieves that intent correctly

---

Note

This PR was written with AI assistance.

AI Session Export

 * ▐▛███▜▌ *   Claude Code v2.0.76
* ▝▜█████▛▘ *  Opus 4.5 · API Usage Billing
 *  ▘▘ ▝▝  *   ~/Code/personal/workerd

> ⏺ Issue

  Calling DO stub methods inside Workflow step.do() causes type errors:

  await step.do("add song", async () => {
    return songQueue.addSongToQueue(songId, name); // Type error
  });

  Why: DO RPC returns T & Disposable. Disposable has [Symbol.dispose]. 
step.do() requires T extends Serializable<T>. Serializable<T> maps symbol 
keys to never, failing the constraint.

  // Current - maps symbols to never, fails constraint
  type Serializable<T> = ... | {
    [K in keyof T]: K extends number | string ? Serializable<T[K]> : never;
  }

  Workaround: Destructure to strip Disposable:
  const { id, song } = await songQueue.addSongToQueue(...);
  return { id, song };

  Proposed Fix

  Filter out symbol keys instead of mapping them to never:

  type Serializable<T> = ... | {
    [K in keyof T as K extends string | number ? K : never]: 
Serializable<T[K]>;
  }

  Symbols aren't serializable anyway - runtime strips them. Type should 
reflect that by omitting, not rejecting. 

⏺ I'll find the Serializable type definition and apply the fix.

⏺ Search(pattern: "type Serializable", glob: "*.ts")
  ⎿  Found 5 files (ctrl+o to expand)

⏺ Read(types/defines/rpc.d.ts)
  ⎿  Read 377 lines

⏺ Found the issue. The Serializable type at line 49 maps symbol keys to never instead of filtering them out. Let me write the plan.

⏺ Updated plan
  ⎿  /plan to preview

⏺ Let me check how types are generated to answer the question about regeneration:

⏺ Bash(grep -n "generate-types" /Users/dmmulroy/Code/personal/workerd/justfile
      2>/dev/null || head -50 /Users/dmmulroy/Code/personal/workerd/justfile)
  ⎿  116:generate-types:

⏺ Bash(sed -n '116,130p' /Users/dmmulroy/Code/personal/workerd/justfile)
  ⎿  generate-types:                                                     
       bazel build //types
       cp -r bazel-bin/types/definitions/latest types/generated-snapshot/
     … +15 lines (ctrl+o to expand)

⏺ Search(pattern: "types/**/BUILD*")
  ⎿  Found 1 file (ctrl+o to expand)

⏺ Read(types/BUILD.bazel)
  ⎿  Read 100 lines

⏺ Now I understand the flow. types/defines/rpc.d.ts is the source, and just generate-types regenerates the snapshots. Let me update the plan:

⏺ Updated plan
  ⎿  /plan to preview

⏺ Plan is complete. This is a single-line change - the user's issue description and proposed fix are clear.
  ⎿  Tool use rejected with user message: Is this a safe change to make, is it 
     backwards compat? what are the possible consequences?

⏺ Good question. Let me analyze the safety and backwards compatibility:

  Type-level analysis:

  Current:
  // If T = { foo: string, [Symbol.dispose]: () => void }
  // Then Serializable<T> = { foo: Serializable<string>, [Symbol.dispose]: never }
  // T extends Serializable<T> fails: (() => void) doesn't extend never

  Proposed:
  // If T = { foo: string, [Symbol.dispose]: () => void }
  // Then Serializable<T> = { foo: Serializable<string> }  // Symbol.dispose omitted
  // T extends Serializable<T> passes: symbol key not checked

  Backwards compat:
  - Code that passed before still passes (narrowing constraint)
  - Code that failed before may now pass (looser constraint)
  - No runtime change (symbols already stripped during serialization)

  Possible risks:
  1. Types that were correctly rejected might now be accepted
  2. Other code might rely on similar pattern

  Let me check for other usages of this pattern in the codebase:

⏺ Search(pattern: "K extends number \| string \? .* : never", glob: "*.ts",
        output_mode: "content")
  ⎿  Found 5 lines (ctrl+o to expand)

⏺ Search(pattern: "extends Serializable", glob: "*.ts", output_mode: "content")
  ⎿  Found 5 lines (ctrl+o to expand)

⏺ Search(pattern: "types/test/**/*.ts")
  ⎿  Found 13 files (ctrl+o to expand)

⏺ Read(types/test/types/rpc.ts)
  ⎿  Read 787 lines

⏺ Let me update the plan with the safety analysis:

⏺ Updated plan
  ⎿  /plan to preview

⏺ Summary:

  The change is safe and backwards compatible:
  - Looser constraint: Only allows more types to pass, doesn't break existing code
  - Correct semantics: Symbols aren't serializable anyway - runtime strips them. Type should reflect that by omitting, not rejecting
  - No runtime change: Purely type-level fix
  - Low risk edge case: Types with only symbol keys as non-serializable parts now pass - but that's correct behavior
                                                                           
⏺ User approved Claude's plan
  ⎿  Plan saved to: ~/.claude/plans/dreamy-orbiting-balloon.md · /plan to 
     edit
     Fix Serializable Type for DO RPC + Workflow step.do()

     Problem

     DO RPC returns T & Disposable. Disposable has [Symbol.dispose]. 
     step.do() requires T extends Serializable<T>. Current Serializable<T> 
     maps symbol keys to never, failing the constraint.

     Root Cause

     types/defines/rpc.d.ts:49:
     [K in keyof T]: K extends number | string ? Serializable<T[K]> : never;
     Maps symbols to never rather than omitting them.

     Fix

     Change line 49 to use key remapping:
     [K in keyof T as K extends string | number ? K : never]: 
     Serializable<T[K]>;

     Safety Analysis

     Backwards compatible: Yes
     - Looser constraint, not stricter
     - Code that passed before still passes
     - Code that failed (like DO RPC + step.do) may now pass

     Correctness: Yes
     - Symbols not serializable in structured clone algorithm
     - Runtime already strips them
     - Type now reflects reality: symbol keys don't exist in serialized form

     Risk: Low
     - Edge case: Types with only symbols as non-serializable parts would 
     now pass constraint
     - This is correct behavior - if all non-symbol props are serializable, 
     object IS serializable
     - No runtime behavior change

     Existing tests: Should pass
     - types/test/types/rpc.ts tests RPC types extensively
     - Test classes already have [Symbol.dispose] methods (lines 65, 280, 
     348)
     - Tests pass because Stub<T> wrapping, not direct return

     Steps

     1. Edit types/defines/rpc.d.ts:49 - change conditional to key remapping
     2. Run just generate-types - regenerates snapshots from source
     3. Verify type tests pass





⏺ Update(types/defines/rpc.d.ts)
  ⎿  Added 1 line, removed 1 line                                        
      46      | Set<T extends Set<infer U> ? Serializable<U> : never>    
      47      | ReadonlyArray<T extends ReadonlyArray<infer U> ?         
          Serializable<U> : never>                                       
      48      | {                                                        
      49 -        [K in keyof T]: K extends number | string ?            
         -Serializable<T[K]> : never;                                    
      49 +        [K in keyof T as K extends string | number ?           
         +K : never]: Serializable<T[K]>;                                
      50        }                                                        
      51      // Special types                                           
      52      | Stub<Stubable>   

[github-actions]

All contributors have signed the CLA ✍️ ✅
_{Posted by the CLA Assistant Lite bot.}

[dmmulroy]

I have read the CLA Document and I hereby sign the CLA

[codspeed-hq]

CodSpeed Performance Report

Merging this PR will degrade performance by 18.5%

_{Comparing dmmulroy:dmmulroy/fix-serializable-type (161303e) with main (08419cb)}

Summary

⚡ 1 improved benchmark
❌ 1 regressed benchmark
✅ 68 untouched benchmarks
⏩ 129 skipped benchmarks¹

⚠️ Please fix the performance issues or acknowledge them on CodSpeed.

Performance Changes

	Benchmark	BASE	HEAD	Efficiency
⚡	Encode_ASCII_32[TextEncoder][0/0/32]	3.3 ms	2.9 ms	+13.38%
❌	jsonResponse[Response]	34.7 µs	42.6 µs	-18.5%

Footnotes

1. 129 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩

[kentonv]

Should we update https://github.com/cloudflare/capnweb/blob/main/src/types.d.ts as well?

[kentonv]

TIL: https://www.typescriptlang.org/docs/handbook/2/mapped-types.html#key-remapping-via-as

[dario-piotrowicz]

Looks good to me 🙂

But possibly as Kenton mentioned there might be other places where this should be applied

[dmmulroy]

capnweb updated: cloudflare/capnweb#129

cc @kentonv @PiotrowskiD

[penalosa]

@dmmulroy it looks like this is pretty red in CI

[petebacondarwin]

I'm not sure about this test change...