Avoid using object union for serde enum serializing
Closed this issue · 2 comments
Dreaming-Codes commented
Problem
When working with typescript something like the following
#[derive(serde::Deserialize, serde::Serialize, Debug, TS)]
#[ts(export)]
#[serde(tag = "type", content = "data")]
pub enum SignalType {
#[serde(rename = "add")]
Add(AddStreamEvent),
#[serde(rename = "remove")]
Remove(RemoveStreamEvent),
#[serde(rename = "ice")]
ICE(ICEEvent),
#[serde(rename = "answer")]
Answer(AnswerOfferEvent),
#[serde(rename = "offer")]
Offer(AnswerOfferEvent),
}
#[derive(serde::Deserialize, serde::Serialize, Debug, TS)]
#[ts(export)]
pub struct ICEEvent {
#[serde(rename = "streamId")]
stream_id: Option<u8>,
candidate: String,
}
[...]
[...]
export interface ICEEvent { streamId: number | null, candidate: string, }
export type SignalType = { type: "add", data: AddStreamEvent } | { type: "remove", data: RemoveStreamEvent } | { type: "ice", data: ICEEvent } | { type: "answer", data: AnswerOfferEvent } | { type: "offer", data: AnswerOfferEvent };
could cause some problems in some edge cases like when using the following class
export class TypedEventTarget<EventDef extends { type: any }> extends EventTarget {
public dispatchEvent(e: Event & EventDef): boolean {
return super.dispatchEvent(e);
}
public dispatch(e: EventDef): boolean {
const event = Object.assign(new Event(e.type), e);
return this.dispatchEvent(event);
}
public addEventListener<
T extends EventDef['type'],
E extends EventDef & { type: T }
>(type: T, listener: ((e: Event & EventDef) => any) | null) {
super.addEventListener(type, listener);
}
public removeEventListener(type: EventDef['type']) {
super.removeEventListener(type, null)
}
}
in this case if I create a class that extend that class like the following
export class WS extends TypedEventTarget<SignalType> {
private ws: WebSocket;
constructor(url: string) {
super();
this.ws = new WebSocket(url);
this.ws.addEventListener("message", this.eventHandler);
}
public sendEvent(event: SignalType) {
this.ws.send(JSON.stringify(event));
}
private eventHandler(event: MessageEvent<SignalType>) {
this.dispatch(event.data);
}
}
there is a problem when using addEventListener since typescript is unable to infer the inner type of SignalType
ws.addEventListener("ice", (event ) => {
//In this case typescript doesn't know that event is of type Event & { type: "ice", data: ICEEvent }
//and so it amuses is Event & SignalType (which is not what we want since this is too generic)
})
Solution
To fix this issue, the SignalType type definition can be redefined to be more specific by creating individual types for each event type instead of using a union type. The resulting SignalType type definition would look like the following:
[...]
export interface ICEEvent { streamId: number | null, candidate: string, }
export type AddType = { type: "add", data: AddStreamEvent };
export type RemoveType = { type: "remove", data: RemoveStreamEvent };
export type IceType = { type: "ice", data: ICEEvent };
export type AnswerType = { type: "answer", data: AnswerOfferEvent };
export type OfferType = { type: "offer", data: AnswerOfferEvent };
export type SignalType = AddType | RemoveType | IceType | AnswerType | OfferType;
Brendonovich commented
No comment on whether this should be implemented or not, but I think you have the wrong signature for addEventListener
:
public addEventListener<
T extends EventDef['type']
>(type: T, listener: ((e: Event & Extract<EventDef, { type: T }>) => any) | null) {
super.addEventListener(type, listener);
}
escritorio-gustavo commented
Duplicate of #86