matrix-org / matrix-js-sdk

Matrix Client-Server SDK for JavaScript
Apache License 2.0
1.6k stars 589 forks source link

Function parameter disrepancies between `CryptoStore` and some subclasses #2113

Open ShadowJonathan opened 2 years ago

ShadowJonathan commented 2 years ago
export interface ISessionInfo {
    deviceKey?: string;
    sessionId?: string;
    session?: string;
    lastReceivedMessageTs?: number;
}

interface CryptoStore {
    getEndToEndSession(
        deviceKey: string,
        sessionId: string,
        txn: unknown,
        func: (session: ISessionInfo) => void,
    ): void;
}
class Backend implements CryptoStore {
    public getEndToEndSession(
        deviceKey: string,
        sessionId: string,
        txn: IDBTransaction,
        func: (sessions: { [ sessionId: string ]: ISessionInfo }) => void,
    ): void {}
}
class IndexedDBCryptoStore implements CryptoStore {
    public getEndToEndSession(
        deviceKey: string,
        sessionId: string,
        txn: IDBTransaction,
        func: (sessions: { [ sessionId: string ]: ISessionInfo }) => void,
    ): void {}
}

This is allowed, because ISessionInfo (as an interface) only "comments" on the properties of corresponding types, so ISessionInfo, only having optional properties, could apply over any type.

turt2live commented 2 years ago

This seems more tractable as a PR than an issue.

ShadowJonathan commented 2 years ago

Agreed, I filed this issue as I found it, as it is a subtle bug that not even https://github.com/matrix-org/matrix-js-sdk/issues/2114 would pick up, as ISessionInfo only has optional parameters, which would make ISessionInfo mappable to any type that does not already define one of its properties (f.e. ISessionInfo is mappable to number, string, and null.)