Skip to content
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

More typescript linting #3310

Merged
merged 9 commits into from
Dec 2, 2024
1 change: 1 addition & 0 deletions .eslintrc.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,7 @@ module.exports = {
tryExtensions: [".ts"],
},
],
"no-extra-boolean-cast": "error",
},
},
{
Expand Down
22 changes: 22 additions & 0 deletions spec/integ/matrix-client-methods.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1906,6 +1906,28 @@ describe("MatrixClient", function () {
return prom;
});
});

describe("getDomain", () => {
it("should return null if no userId is set", () => {
const client = new MatrixClient({ baseUrl: "http://localhost" });
expect(client.getDomain()).toBeNull();
});

it("should return the domain of the userId", () => {
expect(client.getDomain()).toBe("localhost");
});
});

describe("getUserIdLocalpart", () => {
it("should return null if no userId is set", () => {
const client = new MatrixClient({ baseUrl: "http://localhost" });
expect(client.getUserIdLocalpart()).toBeNull();
});

it("should return the localpart of the userId", () => {
expect(client.getUserIdLocalpart()).toBe("alice");
});
});
});

function withThreadId(event: MatrixEvent, newThreadId: string): MatrixEvent {
Expand Down
3 changes: 3 additions & 0 deletions spec/test-utils/webrtc.ts
Original file line number Diff line number Diff line change
Expand Up @@ -481,6 +481,9 @@ export class MockCallMatrixClient extends TypedEventEmitter<EmittedEvents, Emitt
public getUserId(): string {
return this.userId;
}
public getSafeUserId(): string {
return this.userId;
}
Comment on lines 481 to +486
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we need the save version of this of getUserId already returns a non optional?
What is the difference?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a stub for MatrixClient, getUserId actually returns string | null
image

The code under test was changed to use getSafeUserId to avoid getting null and thus the test must also make this method available so it doesn't end up throwing an exception


public getDeviceId(): string {
return this.deviceId;
Expand Down
2 changes: 1 addition & 1 deletion spec/unit/webrtc/groupCall.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1430,7 +1430,7 @@ describe("Group Call", function () {
let client: MatrixClient;

beforeEach(() => {
client = new MatrixClient({ baseUrl: "base_url" });
client = new MatrixClient({ baseUrl: "base_url", userId: "my_user_id" });

jest.spyOn(client, "sendStateEvent").mockResolvedValue({} as any);
});
Expand Down
2 changes: 1 addition & 1 deletion src/@types/global.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,6 @@ declare global {
interface Navigator {
// We check for the webkit-prefixed getUserMedia to detect if we're
// on webkit: we should check if we still need to do this
webkitGetUserMedia: DummyInterfaceWeShouldntBeUsingThis;
webkitGetUserMedia?: DummyInterfaceWeShouldntBeUsingThis;
}
}
2 changes: 1 addition & 1 deletion src/autodiscovery.ts
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ export class AutoDiscovery {
* configuration, which may include error states. Rejects on unexpected
* failure, not when verification fails.
*/
public static async fromDiscoveryConfig(wellknown: IClientWellKnown): Promise<ClientConfig> {
public static async fromDiscoveryConfig(wellknown?: IClientWellKnown): Promise<ClientConfig> {
// Step 1 is to get the config, which is provided to us here.

// We default to an error state to make the first few checks easier to
Expand Down
14 changes: 4 additions & 10 deletions src/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1830,10 +1830,7 @@ export class MatrixClient extends TypedEventEmitter<EmittedEvents, ClientEventHa
* @returns MXID for the logged-in user, or null if not logged in
*/
public getUserId(): string | null {
if (this.credentials && this.credentials.userId) {
return this.credentials.userId;
}
return null;
return this.credentials?.userId ?? null;
}

/**
Expand All @@ -1855,7 +1852,7 @@ export class MatrixClient extends TypedEventEmitter<EmittedEvents, ClientEventHa
* @returns Domain of this MXID
*/
public getDomain(): string | null {
if (this.credentials && this.credentials.userId) {
if (this.credentials?.userId) {
return this.credentials.userId.replace(/^.*?:/, "");
}
return null;
Expand All @@ -1866,10 +1863,7 @@ export class MatrixClient extends TypedEventEmitter<EmittedEvents, ClientEventHa
* @returns The user ID localpart or null.
*/
public getUserIdLocalpart(): string | null {
if (this.credentials && this.credentials.userId) {
return this.credentials.userId.split(":")[0].substring(1);
}
return null;
return this.credentials?.userId?.split(":")[0].substring(1) ?? null;
}

/**
Expand Down Expand Up @@ -4309,7 +4303,7 @@ export class MatrixClient extends TypedEventEmitter<EmittedEvents, ClientEventHa
*/
public getIgnoredUsers(): string[] {
const event = this.getAccountData("m.ignored_user_list");
if (!event || !event.getContent() || !event.getContent()["ignored_users"]) return [];
if (!event?.getContent()["ignored_users"]) return [];
return Object.keys(event.getContent()["ignored_users"]);
}

Expand Down
2 changes: 1 addition & 1 deletion src/crypto/EncryptionSetup.ts
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ export class EncryptionSetupBuilder {
if (!this.keySignatures) {
this.keySignatures = {};
}
const userSignatures = this.keySignatures[userId] || {};
const userSignatures = this.keySignatures[userId] ?? {};
this.keySignatures[userId] = userSignatures;
userSignatures[deviceId] = signature;
}
Expand Down
6 changes: 3 additions & 3 deletions src/crypto/dehydration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,8 @@ import decryptAESSecretStorageItem from "../utils/decryptAESSecretStorageItem.ts
import encryptAESSecretStorageItem from "../utils/encryptAESSecretStorageItem.ts";

export interface IDehydratedDevice {
device_id: string; // eslint-disable-line camelcase
device_data: SecretStorageKeyDescription & {
device_id?: string; // eslint-disable-line camelcase
device_data?: SecretStorageKeyDescription & {
// eslint-disable-line camelcase
algorithm: string;
account: string; // pickle
Expand Down Expand Up @@ -90,7 +90,7 @@ export class DehydrationManager {
}

public async setKey(
key: Uint8Array,
key?: Uint8Array,
keyInfo: { [props: string]: any } = {},
deviceDisplayName?: string,
): Promise<boolean | undefined> {
Expand Down
4 changes: 2 additions & 2 deletions src/crypto/store/localStorage-crypto-store.ts
Original file line number Diff line number Diff line change
Expand Up @@ -162,15 +162,15 @@ export class LocalStorageCryptoStore extends MemoryCryptoStore implements Crypto
func: (session: ISessionInfo) => void,
): void {
const sessions = this._getEndToEndSessions(deviceKey);
func(sessions[sessionId] || {});
func(sessions[sessionId] ?? {});
}

public getEndToEndSessions(
deviceKey: string,
txn: unknown,
func: (sessions: { [sessionId: string]: ISessionInfo }) => void,
): void {
func(this._getEndToEndSessions(deviceKey) || {});
func(this._getEndToEndSessions(deviceKey) ?? {});
}

public getAllEndToEndSessions(txn: unknown, func: (session: ISessionInfo) => void): void {
Expand Down
8 changes: 4 additions & 4 deletions src/models/event.ts
Original file line number Diff line number Diff line change
Expand Up @@ -559,9 +559,9 @@ export class MatrixEvent extends TypedEventEmitter<MatrixEventEmittedEvents, Mat
return {} as T;
}
if (this.clearEvent) {
return (this.clearEvent.content || {}) as T;
return (this.clearEvent.content ?? {}) as T;
}
return (this.event.content || {}) as T;
return (this.event.content ?? {}) as T;
}

/**
Expand All @@ -575,7 +575,7 @@ export class MatrixEvent extends TypedEventEmitter<MatrixEventEmittedEvents, Mat
if (this._localRedactionEvent) {
return {} as T;
} else if (this._replacingEvent) {
return this._replacingEvent.getContent()["m.new_content"] || {};
return this._replacingEvent.getContent()["m.new_content"] ?? {};
} else {
return this.getOriginalContent();
}
Expand Down Expand Up @@ -1631,7 +1631,7 @@ export class MatrixEvent extends TypedEventEmitter<MatrixEventEmittedEvents, Mat
* @param otherEvent - The other event to check against.
* @returns True if the events are the same, false otherwise.
*/
public isEquivalentTo(otherEvent: MatrixEvent): boolean {
public isEquivalentTo(otherEvent?: MatrixEvent): boolean {
if (!otherEvent) return false;
if (otherEvent === this) return true;
const myProps = deepSortedObjectEntries(this.event);
Expand Down
12 changes: 6 additions & 6 deletions src/sliding-sync.ts
Original file line number Diff line number Diff line change
Expand Up @@ -196,8 +196,8 @@ class SlidingList {
* @param list - The new list parameters
*/
public replaceList(list: MSC3575List): void {
list.filters = list.filters || {};
list.ranges = list.ranges || [];
list.filters = list.filters ?? {};
list.ranges = list.ranges ?? [];
this.list = JSON.parse(JSON.stringify(list));
this.isModified = true;

Expand Down Expand Up @@ -894,9 +894,9 @@ export class SlidingSync extends TypedEventEmitter<SlidingSyncEvent, SlidingSync
l.setModified(false);
});
// set default empty values so we don't need to null check
resp.lists = resp.lists || {};
resp.rooms = resp.rooms || {};
resp.extensions = resp.extensions || {};
resp.lists = resp.lists ?? {};
resp.rooms = resp.rooms ?? {};
resp.extensions = resp.extensions ?? {};
Object.keys(resp.lists).forEach((key: string) => {
const list = this.lists.get(key);
if (!list || !resp) {
Expand Down Expand Up @@ -934,7 +934,7 @@ export class SlidingSync extends TypedEventEmitter<SlidingSyncEvent, SlidingSync
const listKeysWithUpdates: Set<string> = new Set();
if (!doNotUpdateList) {
for (const [key, list] of Object.entries(resp.lists)) {
list.ops = list.ops || [];
list.ops = list.ops ?? [];
if (list.ops.length > 0) {
listKeysWithUpdates.add(key);
}
Expand Down
2 changes: 1 addition & 1 deletion src/store/indexeddb.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ const WRITE_DELAY_MS = 1000 * 60 * 5; // once every 5 minutes

interface IOpts extends IBaseOpts {
/** The Indexed DB interface e.g. `window.indexedDB` */
indexedDB: IDBFactory;
indexedDB?: IDBFactory;
/** Optional database name. The same name must be used to open the same database. */
dbName?: string;
/** Optional factory to spin up a Worker to execute the IDB transactions within. */
Expand Down
4 changes: 2 additions & 2 deletions src/sync.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1807,8 +1807,8 @@ export class SyncApi {
// XXX: As above, don't do this...
//room.addLiveEvents(stateEventList || []);
// Do this instead...
room.oldState.setStateEvents(stateEventList || []);
room.currentState.setStateEvents(stateEventList || []);
room.oldState.setStateEvents(stateEventList ?? []);
room.currentState.setStateEvents(stateEventList ?? []);
}

// Execute the timeline events. This will continue to diverge the current state
Expand Down
6 changes: 3 additions & 3 deletions src/webrtc/call.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3016,9 +3016,9 @@ export function supportsMatrixCall(): boolean {
// is that the browser throwing a SecurityError will brick the client creation process.
try {
const supported = Boolean(
window.RTCPeerConnection ||
window.RTCSessionDescription ||
window.RTCIceCandidate ||
window.RTCPeerConnection ??
window.RTCSessionDescription ??
window.RTCIceCandidate ??
navigator.mediaDevices,
);
if (!supported) {
Expand Down
2 changes: 1 addition & 1 deletion src/webrtc/groupCall.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1445,7 +1445,7 @@ export class GroupCall extends TypedEventEmitter<
* Recalculates and updates the participant map to match the room state.
*/
private updateParticipants(): void {
const localMember = this.room.getMember(this.client.getUserId()!)!;
const localMember = this.room.getMember(this.client.getSafeUserId());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here it seems to be fine to just remove the exclamation mark but keep the getUserId method?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, as getUserId has return type string | null in reality, the test stub is misleading

if (!localMember) {
// The client hasn't fetched enough of the room state to get our own member
// event. This probably shouldn't happen, but sanity check & exit for now.
Expand Down
7 changes: 2 additions & 5 deletions src/webrtc/stats/callFeedStatsReporter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,8 +49,8 @@ export class CallFeedStatsReporter {
return {
id: track.id,
kind: track.kind,
settingDeviceId: settingDeviceId ? settingDeviceId : "unknown",
constrainDeviceId: constrainDeviceId ? constrainDeviceId : "unknown",
settingDeviceId: settingDeviceId ?? "unknown",
constrainDeviceId: constrainDeviceId ?? "unknown",
muted: track.muted,
enabled: track.enabled,
readyState: track.readyState,
Expand All @@ -63,9 +63,6 @@ export class CallFeedStatsReporter {
callFeeds: CallFeed[],
prefix = "unknown",
): CallFeedReport {
if (!report.callFeeds) {
report.callFeeds = [];
}
callFeeds.forEach((feed) => {
const audioTracks = feed.stream.getAudioTracks();
const videoTracks = feed.stream.getVideoTracks();
Expand Down
10 changes: 2 additions & 8 deletions src/webrtc/stats/media/mediaTrackHandler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,18 +49,12 @@ export class MediaTrackHandler {

public getLocalTrackIdByMid(mid: string): string | undefined {
const transceiver = this.pc.getTransceivers().find((t) => t.mid === mid);
if (transceiver !== undefined && !!transceiver.sender && !!transceiver.sender.track) {
return transceiver.sender.track.id;
}
return undefined;
return transceiver?.sender?.track?.id;
}

public getRemoteTrackIdByMid(mid: string): string | undefined {
const transceiver = this.pc.getTransceivers().find((t) => t.mid === mid);
if (transceiver !== undefined && !!transceiver.receiver && !!transceiver.receiver.track) {
return transceiver.receiver.track.id;
}
return undefined;
return transceiver?.receiver?.track?.id;
}

public getActiveSimulcastStreams(): number {
Expand Down