Clean up code edge cases and add helpers

src/settings/Settings.ts

@@ -36,8 +36,9 @@ import { isMac } from '../Keyboard';
 import UIFeatureController from "./controllers/UIFeatureController";
 import { UIFeature } from "./UIFeature";
 import { OrderedMultiController } from "./controllers/OrderedMultiController";
-import {Layout} from "./Layout";
+import { Layout } from "./Layout";
 import ReducedMotionController from './controllers/ReducedMotionController';
+import IncompatibleController from "./controllers/IncompatibleController";
 
 // These are just a bunch of helper arrays to avoid copy/pasting a bunch of times
 const LEVELS_ROOM_SETTINGS = [
@@ -188,6 +189,8 @@ export const SETTINGS: {[setting: string]: ISetting} = {
         displayName: _td("Show message previews for reactions in DMs"),
         supportedLevels: LEVELS_FEATURE,
         default: false,
+        // this option is a subset of `feature_roomlist_preview_reactions_all` so disable it when that one is enabled
+        controller: new IncompatibleController("feature_roomlist_preview_reactions_all"),
     },
     "feature_roomlist_preview_reactions_all": {
         isFeature: true,

src/settings/controllers/IncompatibleController.ts

@@ -0,0 +1,46 @@
+/*
+Copyright 2021 The Matrix.org Foundation C.I.C.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+import SettingController from "./SettingController";
+import { SettingLevel } from "../SettingLevel";
+import SettingsStore from "../SettingsStore";
+
+/**
+ * Enforces that a boolean setting cannot be enabled if the incompatible setting
+ * is also enabled, to prevent cascading undefined behaviour between conflicting
+ * labs flags.
+ */
+export default class IncompatibleController extends SettingController {
+    public constructor(private settingName: string, private forcedValue = false) {
+        super();
+    }
+
+    public getValueOverride(
+        level: SettingLevel,
+        roomId: string,
+        calculatedValue: any,
+        calculatedAtLevel: SettingLevel,
+    ): any {
+        if (this.incompatibleSettingEnabled) {
+            return this.forcedValue;
+        }
+        return null; // no override
+    }
+
+    public get incompatibleSettingEnabled(): boolean {
+        return SettingsStore.getValue(this.settingName);
+    }
+}

src/stores/room-list/filters/CommunityFilterCondition.ts

@@ -19,17 +19,17 @@ import { FILTER_CHANGED, FilterPriority, IFilterCondition } from "./IFilterCondi
 import { Group } from "matrix-js-sdk/src/models/group";
 import { EventEmitter } from "events";
 import GroupStore from "../../GroupStore";
-import { arrayHasDiff } from "../../../utils/arrays";
 import { IDestroyable } from "../../../utils/IDestroyable";
 import DMRoomMap from "../../../utils/DMRoomMap";
+import { setHasDiff } from "../../../utils/sets";
 
 /**
  * A filter condition for the room list which reveals rooms which
  * are a member of a given community.
  */
 export class CommunityFilterCondition extends EventEmitter implements IFilterCondition, IDestroyable {
-    private roomIds: string[] = [];
-    private userIds: string[] = [];
+    private roomIds = new Set<string>();
+    private userIds = new Set<string>();
 
     constructor(private community: Group) {
         super();
@@ -45,19 +45,18 @@ export class CommunityFilterCondition extends EventEmitter implements IFilterCon
     }
 
     public isVisible(room: Room): boolean {
-        return this.roomIds.includes(room.roomId) ||
-            this.userIds.includes(DMRoomMap.shared().getUserIdForRoomId(room.roomId));
+        return this.roomIds.has(room.roomId) || this.userIds.has(DMRoomMap.shared().getUserIdForRoomId(room.roomId));
     }
 
     private onStoreUpdate = async (): Promise<any> => {
         // We don't actually know if the room list changed for the community, so just check it again.
         const beforeRoomIds = this.roomIds;
-        this.roomIds = (await GroupStore.getGroupRooms(this.community.groupId)).map(r => r.roomId);
+        this.roomIds = new Set((await GroupStore.getGroupRooms(this.community.groupId)).map(r => r.roomId));
 
         const beforeUserIds = this.userIds;
-        this.userIds = (await GroupStore.getGroupMembers(this.community.groupId)).map(u => u.userId);
+        this.userIds = new Set((await GroupStore.getGroupMembers(this.community.groupId)).map(u => u.userId));
 
-        if (arrayHasDiff(beforeRoomIds, this.roomIds) || arrayHasDiff(beforeUserIds, this.userIds)) {
+        if (setHasDiff(beforeRoomIds, this.roomIds) || setHasDiff(beforeUserIds, this.userIds)) {
             this.emit(FILTER_CHANGED);
         }
     };

src/utils/objects.ts

@@ -89,10 +89,9 @@ export function objectHasDiff<O extends {}>(a: O, b: O): boolean {
     if (a === b) return false;
     const aKeys = Object.keys(a);
     const bKeys = Object.keys(b);
-    const possibleChanges = arrayUnion(aKeys, bKeys);
-    // if the amalgamation of both sets of keys has the a different length to the inputs then there must be a change
-    if (possibleChanges.length !== aKeys.length) return true;
+    if (aKeys.length !== bKeys.length) return true;
 
+    const possibleChanges = arrayUnion(aKeys, bKeys);
     return possibleChanges.some(k => a[k] !== b[k]);
 }
 

src/utils/sets.ts

@@ -0,0 +1,34 @@
+/*
+Copyright 2021 The Matrix.org Foundation C.I.C.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+/**
+ * Determines if two sets are different through a shallow comparison.
+ * @param a The first set. Must be defined.
+ * @param b The second set. Must be defined.
+ * @returns True if they are different, false otherwise.
+ */
+export function setHasDiff<T>(a: Set<T>, b: Set<T>): boolean {
+    if (a.size === b.size) {
+        // When the lengths are equal, check to see if either set is missing an element from the other.
+        if (Array.from(b).some(i => !a.has(i))) return true;
+        if (Array.from(a).some(i => !b.has(i))) return true;
+
+        // if all the keys are common, say so
+        return false;
+    } else {
+        return true; // different lengths means they are naturally diverged
+    }
+}

test/components/views/rooms/MemberList-test.js

@@ -88,7 +88,7 @@ describe('MemberList', () => {
         };
         memberListRoom.currentState = {
             members: {},
-            getStateEvents: () => [], // ignore 3pid invites
+            getStateEvents: (eventType, stateKey) => stateKey === undefined ? [] : null, // ignore 3pid invites
         };
         for (const member of [...adminUsers, ...moderatorUsers, ...defaultUsers]) {
             memberListRoom.currentState.members[member.userId] = member;