From 782ce016d1ad137163e2b47f221e05b71eafd7d8 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Wed, 9 Mar 2022 11:22:36 +0000 Subject: [PATCH] Wrap EventTile rather than its children in an error boundary (#7945) --- src/components/structures/MessagePanel.tsx | 8 ++--- src/components/views/rooms/EventTile.tsx | 20 +++++++------ .../structures/MessagePanel-test.js | 30 +++++++------------ 3 files changed, 26 insertions(+), 32 deletions(-) diff --git a/src/components/structures/MessagePanel.tsx b/src/components/structures/MessagePanel.tsx index 20af21a9e5..f5c47e16c9 100644 --- a/src/components/structures/MessagePanel.tsx +++ b/src/components/structures/MessagePanel.tsx @@ -31,7 +31,7 @@ import SettingsStore from '../../settings/SettingsStore'; import RoomContext, { TimelineRenderingType } from "../../contexts/RoomContext"; import { Layout } from "../../settings/enums/Layout"; import { _t } from "../../languageHandler"; -import EventTile, { haveTileForEvent, IReadReceiptProps } from "../views/rooms/EventTile"; +import EventTile, { UnwrappedEventTile, haveTileForEvent, IReadReceiptProps } from "../views/rooms/EventTile"; import { hasText } from "../../TextForEvent"; import IRCTimelineProfileResizer from "../views/elements/IRCTimelineProfileResizer"; import DMRoomMap from "../../utils/DMRoomMap"; @@ -251,7 +251,7 @@ export default class MessagePanel extends React.Component { private scrollPanel = createRef(); private readonly showTypingNotificationsWatcherRef: string; - private eventTiles: Record = {}; + private eventTiles: Record = {}; // A map to allow groupers to maintain consistent keys even if their first event is uprooted due to back-pagination. public grouperKeyMap = new WeakMap(); @@ -336,7 +336,7 @@ export default class MessagePanel extends React.Component { return this.eventTiles[eventId]?.ref?.current; } - public getTileForEventId(eventId: string): EventTile { + public getTileForEventId(eventId: string): UnwrappedEventTile { if (!this.eventTiles) { return undefined; } @@ -919,7 +919,7 @@ export default class MessagePanel extends React.Component { return receiptsByEvent; } - private collectEventTile = (eventId: string, node: EventTile): void => { + private collectEventTile = (eventId: string, node: UnwrappedEventTile): void => { this.eventTiles[eventId] = node; }; diff --git a/src/components/views/rooms/EventTile.tsx b/src/components/views/rooms/EventTile.tsx index c184f8813f..28073fed32 100644 --- a/src/components/views/rooms/EventTile.tsx +++ b/src/components/views/rooms/EventTile.tsx @@ -15,7 +15,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -import React, { createRef, MouseEvent } from 'react'; +import React, { createRef, forwardRef, MouseEvent, RefObject } from 'react'; import classNames from "classnames"; import { EventType, MsgType, RelationType } from "matrix-js-sdk/src/@types/event"; import { EventStatus, MatrixEvent, MatrixEventEvent } from "matrix-js-sdk/src/models/event"; @@ -356,7 +356,7 @@ interface IState { // MUST be rendered within a RoomContext with a set timelineRenderingType @replaceableComponent("views.rooms.EventTile") -export default class EventTile extends React.Component { +export class UnwrappedEventTile extends React.Component { private suppressReadReceiptAnimation: boolean; private isListeningForReceipts: boolean; // TODO: Types @@ -1129,7 +1129,7 @@ export default class EventTile extends React.Component { return false; } - private renderEvent() { + public render() { const msgtype = this.props.mxEvent.getContent().msgtype; const eventType = this.props.mxEvent.getType() as EventType; const { @@ -1652,14 +1652,16 @@ export default class EventTile extends React.Component { } } } - - public render() { - return - { this.renderEvent() } - ; - } } +// Wrap all event tiles with the tile error boundary so that any throws even during construction are captured +const SafeEventTile = forwardRef((props: IProps, ref: RefObject) => { + return + + ; +}); +export default SafeEventTile; + // XXX this'll eventually be dynamic based on the fields once we have extensible event types const messageTypes = [EventType.RoomMessage, EventType.Sticker]; function isMessageEvent(ev: MatrixEvent): boolean { diff --git a/test/components/structures/MessagePanel-test.js b/test/components/structures/MessagePanel-test.js index 14ff34e3f9..c24180014d 100644 --- a/test/components/structures/MessagePanel-test.js +++ b/test/components/structures/MessagePanel-test.js @@ -21,6 +21,7 @@ import { EventEmitter } from "events"; import * as Matrix from 'matrix-js-sdk/src/matrix'; import FakeTimers from '@sinonjs/fake-timers'; import { mount } from "enzyme"; +import * as TestUtils from "react-dom/test-utils"; import { MatrixClientPeg } from '../../../src/MatrixClientPeg'; import sdk from '../../skinned-sdk'; @@ -28,15 +29,11 @@ import SettingsStore from "../../../src/settings/SettingsStore"; import MatrixClientContext from "../../../src/contexts/MatrixClientContext"; import RoomContext from "../../../src/contexts/RoomContext"; import DMRoomMap from "../../../src/utils/DMRoomMap"; -import { upsertRoomStateEvents } from '../../test-utils'; - -const TestUtils = require('react-dom/test-utils'); -const expect = require('expect'); +import { UnwrappedEventTile } from "../../../src/components/views/rooms/EventTile"; +import * as TestUtilsMatrix from "../../test-utils"; const MessagePanel = sdk.getComponent('structures.MessagePanel'); -const TestUtilsMatrix = require('../../test-utils'); - let client; const room = new Matrix.Room("!roomId:server_name"); @@ -288,8 +285,7 @@ describe('MessagePanel', function() { ); // just check we have the right number of tiles for now - const tiles = TestUtils.scryRenderedComponentsWithType( - res, sdk.getComponent('rooms.EventTile')); + const tiles = TestUtils.scryRenderedComponentsWithType(res, UnwrappedEventTile); expect(tiles.length).toEqual(10); }); @@ -299,9 +295,7 @@ describe('MessagePanel', function() { ); // just check we have the right number of tiles for now - const tiles = TestUtils.scryRenderedComponentsWithType( - res, sdk.getComponent('rooms.EventTile'), - ); + const tiles = TestUtils.scryRenderedComponentsWithType(res, UnwrappedEventTile); expect(tiles.length).toEqual(2); const summaryTiles = TestUtils.scryRenderedComponentsWithType( @@ -320,8 +314,7 @@ describe('MessagePanel', function() { />, ); - const tiles = TestUtils.scryRenderedComponentsWithType( - res, sdk.getComponent('rooms.EventTile')); + const tiles = TestUtils.scryRenderedComponentsWithType(res, UnwrappedEventTile); // find the
  • which wraps the read marker const rm = TestUtils.findRenderedDOMComponentWithClass(res, 'mx_RoomView_myReadMarker_container'); @@ -390,8 +383,7 @@ describe('MessagePanel', function() { readMarkerVisible={true} />, parentDiv); - const tiles = TestUtils.scryRenderedComponentsWithType( - mp, sdk.getComponent('rooms.EventTile')); + const tiles = TestUtils.scryRenderedComponentsWithType(mp, UnwrappedEventTile); const tileContainers = tiles.map(function(t) { return ReactDOM.findDOMNode(t); }); @@ -437,7 +429,7 @@ describe('MessagePanel', function() { it('should collapse creation events', function() { const events = mkCreationEvents(); - upsertRoomStateEvents(room, events); + TestUtilsMatrix.upsertRoomStateEvents(room, events); const res = mount( , ); @@ -447,7 +439,7 @@ describe('MessagePanel', function() { // should be outside of the room creation summary // - all other events should be inside the room creation summary - const tiles = res.find(sdk.getComponent('views.rooms.EventTile')); + const tiles = res.find(UnwrappedEventTile); expect(tiles.at(0).props().mxEvent.getType()).toEqual("m.room.create"); expect(tiles.at(1).props().mxEvent.getType()).toEqual("m.room.encryption"); @@ -455,7 +447,7 @@ describe('MessagePanel', function() { const summaryTiles = res.find(sdk.getComponent('views.elements.GenericEventListSummary')); const summaryTile = summaryTiles.at(0); - const summaryEventTiles = summaryTile.find(sdk.getComponent('views.rooms.EventTile')); + const summaryEventTiles = summaryTile.find(UnwrappedEventTile); // every event except for the room creation, room encryption, and Bob's // invite event should be in the event summary expect(summaryEventTiles.length).toEqual(tiles.length - 3); @@ -463,7 +455,7 @@ describe('MessagePanel', function() { it('should hide read-marker at the end of creation event summary', function() { const events = mkCreationEvents(); - upsertRoomStateEvents(room, events); + TestUtilsMatrix.upsertRoomStateEvents(room, events); const res = mount(