Aleph-Alpha/ts-rs

Avoid using object union for serde enum serializing

Closed this issue · 2 comments

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;

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);
}

Playground

Duplicate of #86