diff --git a/src/components/views/rooms/RoomSublist.tsx b/src/components/views/rooms/RoomSublist.tsx index 1ca3a0006f..a15a6c9cde 100644 --- a/src/components/views/rooms/RoomSublist.tsx +++ b/src/components/views/rooms/RoomSublist.tsx @@ -48,6 +48,7 @@ import { polyfillTouchEvent } from "../../../@types/polyfill"; import { RoomNotificationStateStore } from "../../../stores/notifications/RoomNotificationStateStore"; import RoomListLayoutStore from "../../../stores/room-list/RoomListLayoutStore"; import { arrayHasDiff, arrayHasOrderChange } from "../../../utils/arrays"; +import { objectExcluding, objectHasValueChange } from "../../../utils/objects"; const SHOW_N_BUTTON_HEIGHT = 28; // As defined by CSS const RESIZE_HANDLE_HEIGHT = 4; // As defined by CSS @@ -174,6 +175,55 @@ export default class RoomSublist extends React.Component { } } + public shouldComponentUpdate(nextProps: Readonly, nextState: Readonly): boolean { + if (objectHasValueChange(this.props, nextProps)) { + // Something we don't care to optimize has updated, so update. + return true; + } + + // Do the same check used on props for state, without the rooms we're going to no-op + const prevStateNoRooms = objectExcluding(this.state, ['rooms']); + const nextStateNoRooms = objectExcluding(nextState, ['rooms']); + if (objectHasValueChange(prevStateNoRooms, nextStateNoRooms)) { + return true; + } + + // If we're supposed to handle extra tiles, take the performance hit and re-render all the + // time so we don't have to consider them as part of the visible room optimization. + const prevExtraTiles = this.props.extraBadTilesThatShouldntExist || []; + const nextExtraTiles = nextProps.extraBadTilesThatShouldntExist || []; + if (prevExtraTiles.length > 0 || nextExtraTiles.length > 0) { + return true; + } + + // If we're about to update the height of the list, we don't really care about which rooms + // are visible or not for no-op purposes, so ensure that the height calculation runs through. + if (RoomSublist.calcNumTiles(nextState.rooms, nextProps.extraBadTilesThatShouldntExist) !== this.numTiles) { + return true; + } + + // Finally, determine if the room update (as presumably that's all that's left) is within + // our visible range. If it is, then do a render. If the update is outside our visible range + // then we can skip the update. + // + // We also optimize for order changing here: if the update did happen in our visible range + // but doesn't result in the list re-sorting itself then there's no reason for us to update + // on our own. + const prevSlicedRooms = this.state.rooms.slice(0, this.numVisibleTiles); + const nextSlicedRooms = nextState.rooms.slice(0, this.numVisibleTiles); + if (arrayHasOrderChange(prevSlicedRooms, nextSlicedRooms)) { + return true; + } + + // Quickly double check we're not about to break something due to the number of rooms changing. + if (arrayHasDiff(this.state.rooms, nextState.rooms)) { + return true; + } + + // Finally, nothing happened so no-op the update + return false; + } + public componentWillUnmount() { this.state.notificationState.destroy(); defaultDispatcher.unregister(this.dispatcherRef); diff --git a/src/utils/objects.ts b/src/utils/objects.ts index 14fa928ce2..c3fbc64022 100644 --- a/src/utils/objects.ts +++ b/src/utils/objects.ts @@ -14,7 +14,41 @@ See the License for the specific language governing permissions and limitations under the License. */ -import { arrayDiff, arrayMerge, arrayUnion } from "./arrays"; +import { arrayDiff, arrayHasDiff, arrayMerge, arrayUnion } from "./arrays"; + +/** + * Gets a new object which represents the provided object, excluding some properties. + * @param a The object to strip properties of. Must be defined. + * @param props The property names to remove. + * @returns The new object without the provided properties. + */ +export function objectExcluding(a: any, props: string[]): any { + // We use a Map to avoid hammering the `delete` keyword, which is slow and painful. + const tempMap = new Map(Object.entries(a)); + for (const prop of props) { + tempMap.delete(prop); + } + + // Convert the map to an object again + return Array.from(tempMap.entries()).reduce((c, [k, v]) => { + c[k] = v; + return c; + }, {}); +} + +/** + * Determines if the two objects, which are assumed to be of the same + * key shape, have a difference in their values. If a difference is + * determined, true is returned. + * @param a The first object. Must be defined. + * @param b The second object. Must be defined. + * @returns True if there's a perceptual difference in the object's values. + */ +export function objectHasValueChange(a: any, b: any): boolean { + const aValues = Object.values(a); + const bValues = Object.values(b); + return arrayHasDiff(aValues, bValues); +} /** * Determines the keys added, changed, and removed between two objects.