From 0ee30414cfb5f55e967acf69f6a9e42755eb4bcb Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Wed, 2 Mar 2022 13:26:18 +0000 Subject: [PATCH 1/4] Wrap EventTile rather than its children in an error boundary --- src/components/views/rooms/EventTile.tsx | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/src/components/views/rooms/EventTile.tsx b/src/components/views/rooms/EventTile.tsx index 62784f6d5ce..d05f7afb4a9 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 { +class EventTile 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 { @@ -1653,14 +1653,17 @@ 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 { From 84d98019c3943d8d7ca41cb5fc3e60c9b9185ec1 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Wed, 2 Mar 2022 13:34:02 +0000 Subject: [PATCH 2/4] Fix types --- src/components/structures/MessagePanel.tsx | 8 ++++---- src/components/views/rooms/EventTile.tsx | 1 + 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/src/components/structures/MessagePanel.tsx b/src/components/structures/MessagePanel.tsx index cd7824ca507..f478eb2270e 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, { EventTileType, 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): EventTileType { 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: EventTileType): void => { this.eventTiles[eventId] = node; }; diff --git a/src/components/views/rooms/EventTile.tsx b/src/components/views/rooms/EventTile.tsx index d05f7afb4a9..b454ceef554 100644 --- a/src/components/views/rooms/EventTile.tsx +++ b/src/components/views/rooms/EventTile.tsx @@ -1663,6 +1663,7 @@ const SafeEventTile = forwardRef((props: IProps, ref: RefObject) => { }); export default SafeEventTile; +export type EventTileType = EventTile; // XXX this'll eventually be dynamic based on the fields once we have extensible event types const messageTypes = [EventType.RoomMessage, EventType.Sticker]; From 34084e167c5c42bdf1fff4eaba4790963490ed02 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Wed, 2 Mar 2022 14:33:44 +0000 Subject: [PATCH 3/4] Fix test --- .../components/structures/MessagePanel-test.js | 18 +++++++----------- 1 file changed, 7 insertions(+), 11 deletions(-) diff --git a/test/components/structures/MessagePanel-test.js b/test/components/structures/MessagePanel-test.js index 7bf74ce88b5..b9f742b0560 100644 --- a/test/components/structures/MessagePanel-test.js +++ b/test/components/structures/MessagePanel-test.js @@ -29,6 +29,7 @@ import MatrixClientContext from "../../../src/contexts/MatrixClientContext"; import RoomContext from "../../../src/contexts/RoomContext"; import DMRoomMap from "../../../src/utils/DMRoomMap"; import { upsertRoomStateEvents } from '../../test-utils'; +import EventTile from "../../../src/components/views/rooms/EventTile"; const TestUtils = require('react-dom/test-utils'); const expect = require('expect'); @@ -288,8 +289,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, EventTile); expect(tiles.length).toEqual(10); }); @@ -299,9 +299,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, EventTile); expect(tiles.length).toEqual(2); const summaryTiles = TestUtils.scryRenderedComponentsWithType( @@ -320,8 +318,7 @@ describe('MessagePanel', function() { />, ); - const tiles = TestUtils.scryRenderedComponentsWithType( - res, sdk.getComponent('rooms.EventTile')); + const tiles = TestUtils.scryRenderedComponentsWithType(res, EventTile); // find the
  • which wraps the read marker const rm = TestUtils.findRenderedDOMComponentWithClass(res, 'mx_RoomView_myReadMarker_container'); @@ -390,8 +387,7 @@ describe('MessagePanel', function() { readMarkerVisible={true} />, parentDiv); - const tiles = TestUtils.scryRenderedComponentsWithType( - mp, sdk.getComponent('rooms.EventTile')); + const tiles = TestUtils.scryRenderedComponentsWithType(mp, EventTile); const tileContainers = tiles.map(function(t) { return ReactDOM.findDOMNode(t); }); @@ -447,7 +443,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(EventTile); expect(tiles.at(0).props().mxEvent.getType()).toEqual("m.room.create"); expect(tiles.at(1).props().mxEvent.getType()).toEqual("m.room.encryption"); @@ -455,7 +451,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(EventTile); // 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); From 97f1bc0e742cdeeff5c6230f23c5d0d14e0e33cf Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Fri, 4 Mar 2022 11:14:21 +0000 Subject: [PATCH 4/4] Make tests happy --- src/components/structures/MessagePanel.tsx | 8 +++--- src/components/views/rooms/EventTile.tsx | 8 +++--- .../structures/MessagePanel-test.js | 26 ++++++++----------- 3 files changed, 18 insertions(+), 24 deletions(-) diff --git a/src/components/structures/MessagePanel.tsx b/src/components/structures/MessagePanel.tsx index 72bb84688fa..f5c47e16c97 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, { EventTileType, 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): EventTileType { + 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: EventTileType): 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 a8ea15d7a89..28073fed324 100644 --- a/src/components/views/rooms/EventTile.tsx +++ b/src/components/views/rooms/EventTile.tsx @@ -356,7 +356,7 @@ interface IState { // MUST be rendered within a RoomContext with a set timelineRenderingType @replaceableComponent("views.rooms.EventTile") -class EventTile extends React.Component { +export class UnwrappedEventTile extends React.Component { private suppressReadReceiptAnimation: boolean; private isListeningForReceipts: boolean; // TODO: Types @@ -1655,14 +1655,12 @@ class EventTile extends React.Component { } // 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) => { +const SafeEventTile = forwardRef((props: IProps, ref: RefObject) => { return - + ; }); - export default SafeEventTile; -export type EventTileType = EventTile; // XXX this'll eventually be dynamic based on the fields once we have extensible event types const messageTypes = [EventType.RoomMessage, EventType.Sticker]; diff --git a/test/components/structures/MessagePanel-test.js b/test/components/structures/MessagePanel-test.js index 97c7419c38b..5f4b52d096c 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,16 +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'; -import EventTile from "../../../src/components/views/rooms/EventTile"; - -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"); @@ -289,7 +285,7 @@ describe('MessagePanel', function() { ); // just check we have the right number of tiles for now - const tiles = TestUtils.scryRenderedComponentsWithType(res, EventTile); + const tiles = TestUtils.scryRenderedComponentsWithType(res, UnwrappedEventTile); expect(tiles.length).toEqual(10); }); @@ -299,7 +295,7 @@ describe('MessagePanel', function() { ); // just check we have the right number of tiles for now - const tiles = TestUtils.scryRenderedComponentsWithType(res, EventTile); + const tiles = TestUtils.scryRenderedComponentsWithType(res, UnwrappedEventTile); expect(tiles.length).toEqual(2); const summaryTiles = TestUtils.scryRenderedComponentsWithType( @@ -318,7 +314,7 @@ describe('MessagePanel', function() { />, ); - const tiles = TestUtils.scryRenderedComponentsWithType(res, EventTile); + const tiles = TestUtils.scryRenderedComponentsWithType(res, UnwrappedEventTile); // find the
  • which wraps the read marker const rm = TestUtils.findRenderedDOMComponentWithClass(res, 'mx_RoomView_myReadMarker_container'); @@ -387,7 +383,7 @@ describe('MessagePanel', function() { readMarkerVisible={true} />, parentDiv); - const tiles = TestUtils.scryRenderedComponentsWithType(mp, EventTile); + const tiles = TestUtils.scryRenderedComponentsWithType(mp, UnwrappedEventTile); const tileContainers = tiles.map(function(t) { return ReactDOM.findDOMNode(t); }); @@ -433,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( , ); @@ -443,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(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"); @@ -451,7 +447,7 @@ describe('MessagePanel', function() { const summaryTiles = res.find(sdk.getComponent('views.elements.GenericEventListSummary')); const summaryTile = summaryTiles.at(0); - const summaryEventTiles = summaryTile.find(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); @@ -459,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(