From 9aef1874db09f0eb0eefca83d42e98b6afac7f40 Mon Sep 17 00:00:00 2001 From: Michael Weimann Date: Thu, 30 Mar 2023 10:06:50 +0200 Subject: [PATCH] Strictify `stores/room-list` (#10474) --- src/stores/room-list/MessagePreviewStore.ts | 11 ++++- src/stores/room-list/RoomListStore.ts | 5 +- src/stores/room-list/algorithms/Algorithm.ts | 1 + .../list-ordering/ImportanceAlgorithm.ts | 48 ++++++++++++++----- .../list-ordering/OrderingAlgorithm.ts | 8 ++-- .../room-list/previews/MessageEventPreview.ts | 4 +- src/stores/room-list/previews/utils.ts | 2 +- src/utils/membership.ts | 4 +- 8 files changed, 62 insertions(+), 21 deletions(-) diff --git a/src/stores/room-list/MessagePreviewStore.ts b/src/stores/room-list/MessagePreviewStore.ts index 66ea6621a5..5bd927282f 100644 --- a/src/stores/room-list/MessagePreviewStore.ts +++ b/src/stores/room-list/MessagePreviewStore.ts @@ -210,9 +210,16 @@ export class MessagePreviewStore extends AsyncStoreWithClient { if (payload.action === "MatrixActions.Room.timeline" || payload.action === "MatrixActions.Event.decrypted") { const event = payload.event; // TODO: Type out the dispatcher + const roomId = event.getRoomId(); const isHistoricalEvent = payload.hasOwnProperty("isLiveEvent") && !payload.isLiveEvent; - if (!this.previews.has(event.getRoomId()) || isHistoricalEvent) return; // not important - await this.generatePreview(this.matrixClient.getRoom(event.getRoomId()), TAG_ANY); + + if (!roomId || !this.previews.has(roomId) || isHistoricalEvent) return; + + const room = this.matrixClient.getRoom(roomId); + + if (!room) return; + + await this.generatePreview(room, TAG_ANY); } } } diff --git a/src/stores/room-list/RoomListStore.ts b/src/stores/room-list/RoomListStore.ts index e7032f1073..630ccb46d7 100644 --- a/src/stores/room-list/RoomListStore.ts +++ b/src/stores/room-list/RoomListStore.ts @@ -244,7 +244,10 @@ export class RoomListStoreClass extends AsyncStoreWithClient implements logger.warn(`Queuing failed room update for retry as a result.`); window.setTimeout(async (): Promise => { const updatedRoom = this.matrixClient.getRoom(roomId); - await tryUpdate(updatedRoom); + + if (updatedRoom) { + await tryUpdate(updatedRoom); + } }, 100); // 100ms should be enough for the room to show up return; } else { diff --git a/src/stores/room-list/algorithms/Algorithm.ts b/src/stores/room-list/algorithms/Algorithm.ts index a97057cf35..36a84bf157 100644 --- a/src/stores/room-list/algorithms/Algorithm.ts +++ b/src/stores/room-list/algorithms/Algorithm.ts @@ -492,6 +492,7 @@ export class Algorithm extends EventEmitter { // Split out the easy rooms first (leave and invite) const memberships = splitRoomsByMembership(rooms); + for (const room of memberships[EffectiveMembership.Invite]) { newTags[DefaultTagID.Invite].push(room); } diff --git a/src/stores/room-list/algorithms/list-ordering/ImportanceAlgorithm.ts b/src/stores/room-list/algorithms/list-ordering/ImportanceAlgorithm.ts index 11a7314b8f..7c6ae4a617 100644 --- a/src/stores/room-list/algorithms/list-ordering/ImportanceAlgorithm.ts +++ b/src/stores/room-list/algorithms/list-ordering/ImportanceAlgorithm.ts @@ -25,9 +25,9 @@ import { OrderingAlgorithm } from "./OrderingAlgorithm"; import { NotificationColor } from "../../../notifications/NotificationColor"; import { RoomNotificationStateStore } from "../../../notifications/RoomNotificationStateStore"; -type CategorizedRoomMap = Partial<{ +type CategorizedRoomMap = { [category in NotificationColor]: Room[]; -}>; +}; type CategoryIndex = Partial<{ [category in NotificationColor]: number; // integer @@ -84,7 +84,7 @@ export class ImportanceAlgorithm extends OrderingAlgorithm { }; for (const room of rooms) { const category = this.getRoomCategory(room); - map[category].push(room); + map[category]?.push(room); } return map; } @@ -126,11 +126,21 @@ export class ImportanceAlgorithm extends OrderingAlgorithm { } } + private getCategoryIndex(category: NotificationColor): number { + const categoryIndex = this.indices[category]; + + if (categoryIndex === undefined) { + throw new Error(`Index of category ${category} not found`); + } + + return categoryIndex; + } + private handleSplice(room: Room, cause: RoomUpdateCause): boolean { if (cause === RoomUpdateCause.NewRoom) { const category = this.getRoomCategory(room); this.alterCategoryPositionBy(category, 1, this.indices); - this.cachedOrderedRooms.splice(this.indices[category], 0, room); // splice in the new room (pre-adjusted) + this.cachedOrderedRooms.splice(this.getCategoryIndex(category), 0, room); // splice in the new room (pre-adjusted) this.sortCategory(category); } else if (cause === RoomUpdateCause.RoomRemoved) { const roomIdx = this.getRoomIndex(room); @@ -160,7 +170,7 @@ export class ImportanceAlgorithm extends OrderingAlgorithm { const category = this.getRoomCategory(room); if (this.sortingAlgorithm === SortAlgorithm.Manual) { - return; // Nothing to do here. + return false; // Nothing to do here. } const roomIdx = this.getRoomIndex(room); @@ -175,7 +185,7 @@ export class ImportanceAlgorithm extends OrderingAlgorithm { // Move the room and update the indices this.moveRoomIndexes(1, oldCategory, category, this.indices); this.cachedOrderedRooms.splice(roomIdx, 1); // splice out the old index (fixed position) - this.cachedOrderedRooms.splice(this.indices[category], 0, room); // splice in the new room (pre-adjusted) + this.cachedOrderedRooms.splice(this.getCategoryIndex(category), 0, room); // splice in the new room (pre-adjusted) // Note: if moveRoomIndexes() is called after the splice then the insert operation // will happen in the wrong place. Because we would have already adjusted the index // for the category, we don't need to determine how the room is moving in the list. @@ -200,8 +210,8 @@ export class ImportanceAlgorithm extends OrderingAlgorithm { const nextCategoryStartIdx = category === CATEGORY_ORDER[CATEGORY_ORDER.length - 1] ? Number.MAX_SAFE_INTEGER - : this.indices[CATEGORY_ORDER[CATEGORY_ORDER.indexOf(category) + 1]]; - const startIdx = this.indices[category]; + : this.getCategoryIndex(CATEGORY_ORDER[CATEGORY_ORDER.indexOf(category) + 1]); + const startIdx = this.getCategoryIndex(category); const numSort = nextCategoryStartIdx - startIdx; // splice() returns up to the max, so MAX_SAFE_INT is fine const unsortedSlice = this.cachedOrderedRooms.splice(startIdx, numSort); const sorted = sortRoomsWithAlgorithm(unsortedSlice, this.tagId, this.sortingAlgorithm); @@ -215,6 +225,9 @@ export class ImportanceAlgorithm extends OrderingAlgorithm { const isLast = i === CATEGORY_ORDER.length - 1; const startIdx = indices[category]; const endIdx = isLast ? Number.MAX_SAFE_INTEGER : indices[CATEGORY_ORDER[i + 1]]; + + if (startIdx === undefined || endIdx === undefined) continue; + if (index >= startIdx && index < endIdx) { return category; } @@ -250,24 +263,37 @@ export class ImportanceAlgorithm extends OrderingAlgorithm { // index for the categories will be way off. const nextOrderIndex = CATEGORY_ORDER.indexOf(category) + 1; + if (n > 0) { for (let i = nextOrderIndex; i < CATEGORY_ORDER.length; i++) { const nextCategory = CATEGORY_ORDER[i]; - indices[nextCategory] += Math.abs(n); + + if (indices[nextCategory] === undefined) { + throw new Error(`Index of category ${category} not found`); + } + + indices[nextCategory]! += Math.abs(n); } } else if (n < 0) { for (let i = nextOrderIndex; i < CATEGORY_ORDER.length; i++) { const nextCategory = CATEGORY_ORDER[i]; - indices[nextCategory] -= Math.abs(n); + + if (indices[nextCategory] === undefined) { + throw new Error(`Index of category ${category} not found`); + } + + indices[nextCategory]! -= Math.abs(n); } } // Do a quick check to see if we've completely broken the index for (let i = 1; i <= CATEGORY_ORDER.length; i++) { const lastCat = CATEGORY_ORDER[i - 1]; + const lastCatIndex = indices[lastCat]; const thisCat = CATEGORY_ORDER[i]; + const thisCatIndex = indices[thisCat]; - if (indices[lastCat] > indices[thisCat]) { + if (lastCatIndex === undefined || thisCatIndex === undefined || lastCatIndex > thisCatIndex) { // "should never happen" disclaimer goes here logger.warn( `!! Room list index corruption: ${lastCat} (i:${indices[lastCat]}) is greater ` + diff --git a/src/stores/room-list/algorithms/list-ordering/OrderingAlgorithm.ts b/src/stores/room-list/algorithms/list-ordering/OrderingAlgorithm.ts index e791e741aa..6cf8b4606f 100644 --- a/src/stores/room-list/algorithms/list-ordering/OrderingAlgorithm.ts +++ b/src/stores/room-list/algorithms/list-ordering/OrderingAlgorithm.ts @@ -25,8 +25,10 @@ import { SortAlgorithm } from "../models"; * `cachedOrderedRooms` field. */ export abstract class OrderingAlgorithm { - protected cachedOrderedRooms: Room[]; - protected sortingAlgorithm: SortAlgorithm; + protected cachedOrderedRooms: Room[] = []; + + // set by setSortAlgorithm() in ctor + protected sortingAlgorithm!: SortAlgorithm; protected constructor(protected tagId: TagID, initialSortingAlgorithm: SortAlgorithm) { // noinspection JSIgnoredPromiseFromCall @@ -37,7 +39,7 @@ export abstract class OrderingAlgorithm { * The rooms as ordered by the algorithm. */ public get orderedRooms(): Room[] { - return this.cachedOrderedRooms || []; + return this.cachedOrderedRooms; } /** diff --git a/src/stores/room-list/previews/MessageEventPreview.ts b/src/stores/room-list/previews/MessageEventPreview.ts index af572ed0b0..e942101c35 100644 --- a/src/stores/room-list/previews/MessageEventPreview.ts +++ b/src/stores/room-list/previews/MessageEventPreview.ts @@ -72,7 +72,9 @@ export class MessageEventPreview implements IPreview { return _t("* %(senderName)s %(emote)s", { senderName: getSenderName(event), emote: body }); } - if (isThread || isSelf(event) || !shouldPrefixMessagesIn(event.getRoomId(), tagId)) { + const roomId = event.getRoomId(); + + if (isThread || isSelf(event) || (roomId && !shouldPrefixMessagesIn(roomId, tagId))) { return body; } else { return _t("%(senderName)s: %(message)s", { senderName: getSenderName(event), message: body }); diff --git a/src/stores/room-list/previews/utils.ts b/src/stores/room-list/previews/utils.ts index 116e97b515..2dfa75966e 100644 --- a/src/stores/room-list/previews/utils.ts +++ b/src/stores/room-list/previews/utils.ts @@ -37,5 +37,5 @@ export function shouldPrefixMessagesIn(roomId: string, tagId?: TagID): boolean { } export function getSenderName(event: MatrixEvent): string { - return event.sender ? event.sender.name : event.getSender(); + return event.sender ? event.sender.name : event.getSender() || ""; } diff --git a/src/utils/membership.ts b/src/utils/membership.ts index b272b7d37e..b004f5ada6 100644 --- a/src/utils/membership.ts +++ b/src/utils/membership.ts @@ -44,9 +44,9 @@ export enum EffectiveMembership { Leave = "LEAVE", } -export type MembershipSplit = Partial<{ +export type MembershipSplit = { [state in EffectiveMembership]: Room[]; -}>; +}; export function splitRoomsByMembership(rooms: Room[]): MembershipSplit { const split: MembershipSplit = {