-
Notifications
You must be signed in to change notification settings - Fork 504
fix(types): filter symbol keys in Rpc.Serializable mapped type #5804
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
All contributors have signed the CLA ✍️ ✅ |
|
I have read the CLA Document and I hereby sign the CLA |
CodSpeed Performance ReportMerging #5804 will not alter performanceComparing Summary
Footnotes
|
kentonv
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we update https://github.com/cloudflare/capnweb/blob/main/src/types.d.ts as well?
| | ReadonlyArray<T extends ReadonlyArray<infer U> ? Serializable<U> : never> | ||
| | { | ||
| [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]>; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Problem
Rpc.Serializable<T>fails to match valid serializable types when they include symbol keys (e.g.,Symbol.disposefromDisposable).When using
WorkflowStep.do()with a Durable Object stub return value:TypeScript errors:
Reproduction
https://github.com/dmmulroy/workerd-rpc-serializable-type-repro
Cause
The mapped type uses a value-side conditional that maps symbol keys to
neverinstead of filtering them out:This creates an object type that has the symbol keys but with
nevervalues, causing assignability failures.Fix
Use key remapping with
asclause 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:
Note
This PR was written with AI assistance.
AI Session Export