Wrap EventTile rather than its children in an error boundary (#7945)

This commit is contained in:
Michael Telatynski 2022-03-09 11:22:36 +00:00 committed by GitHub
parent a75100ef86
commit 782ce016d1
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
3 changed files with 26 additions and 32 deletions

View file

@ -31,7 +31,7 @@ import SettingsStore from '../../settings/SettingsStore';
import RoomContext, { TimelineRenderingType } from "../../contexts/RoomContext"; import RoomContext, { TimelineRenderingType } from "../../contexts/RoomContext";
import { Layout } from "../../settings/enums/Layout"; import { Layout } from "../../settings/enums/Layout";
import { _t } from "../../languageHandler"; 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 { hasText } from "../../TextForEvent";
import IRCTimelineProfileResizer from "../views/elements/IRCTimelineProfileResizer"; import IRCTimelineProfileResizer from "../views/elements/IRCTimelineProfileResizer";
import DMRoomMap from "../../utils/DMRoomMap"; import DMRoomMap from "../../utils/DMRoomMap";
@ -251,7 +251,7 @@ export default class MessagePanel extends React.Component<IProps, IState> {
private scrollPanel = createRef<ScrollPanel>(); private scrollPanel = createRef<ScrollPanel>();
private readonly showTypingNotificationsWatcherRef: string; private readonly showTypingNotificationsWatcherRef: string;
private eventTiles: Record<string, EventTile> = {}; private eventTiles: Record<string, UnwrappedEventTile> = {};
// A map to allow groupers to maintain consistent keys even if their first event is uprooted due to back-pagination. // A map to allow groupers to maintain consistent keys even if their first event is uprooted due to back-pagination.
public grouperKeyMap = new WeakMap<MatrixEvent, string>(); public grouperKeyMap = new WeakMap<MatrixEvent, string>();
@ -336,7 +336,7 @@ export default class MessagePanel extends React.Component<IProps, IState> {
return this.eventTiles[eventId]?.ref?.current; return this.eventTiles[eventId]?.ref?.current;
} }
public getTileForEventId(eventId: string): EventTile { public getTileForEventId(eventId: string): UnwrappedEventTile {
if (!this.eventTiles) { if (!this.eventTiles) {
return undefined; return undefined;
} }
@ -919,7 +919,7 @@ export default class MessagePanel extends React.Component<IProps, IState> {
return receiptsByEvent; return receiptsByEvent;
} }
private collectEventTile = (eventId: string, node: EventTile): void => { private collectEventTile = (eventId: string, node: UnwrappedEventTile): void => {
this.eventTiles[eventId] = node; this.eventTiles[eventId] = node;
}; };

View file

@ -15,7 +15,7 @@ See the License for the specific language governing permissions and
limitations under the License. limitations under the License.
*/ */
import React, { createRef, MouseEvent } from 'react'; import React, { createRef, forwardRef, MouseEvent, RefObject } from 'react';
import classNames from "classnames"; import classNames from "classnames";
import { EventType, MsgType, RelationType } from "matrix-js-sdk/src/@types/event"; import { EventType, MsgType, RelationType } from "matrix-js-sdk/src/@types/event";
import { EventStatus, MatrixEvent, MatrixEventEvent } from "matrix-js-sdk/src/models/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 // MUST be rendered within a RoomContext with a set timelineRenderingType
@replaceableComponent("views.rooms.EventTile") @replaceableComponent("views.rooms.EventTile")
export default class EventTile extends React.Component<IProps, IState> { export class UnwrappedEventTile extends React.Component<IProps, IState> {
private suppressReadReceiptAnimation: boolean; private suppressReadReceiptAnimation: boolean;
private isListeningForReceipts: boolean; private isListeningForReceipts: boolean;
// TODO: Types // TODO: Types
@ -1129,7 +1129,7 @@ export default class EventTile extends React.Component<IProps, IState> {
return false; return false;
} }
private renderEvent() { public render() {
const msgtype = this.props.mxEvent.getContent().msgtype; const msgtype = this.props.mxEvent.getContent().msgtype;
const eventType = this.props.mxEvent.getType() as EventType; const eventType = this.props.mxEvent.getType() as EventType;
const { const {
@ -1652,13 +1652,15 @@ export default class EventTile extends React.Component<IProps, IState> {
} }
} }
} }
}
public render() { // Wrap all event tiles with the tile error boundary so that any throws even during construction are captured
return <TileErrorBoundary mxEvent={this.props.mxEvent} layout={this.props.layout}> const SafeEventTile = forwardRef((props: IProps, ref: RefObject<UnwrappedEventTile>) => {
{ this.renderEvent() } return <TileErrorBoundary mxEvent={props.mxEvent} layout={props.layout}>
<UnwrappedEventTile ref={ref} {...props} />
</TileErrorBoundary>; </TileErrorBoundary>;
} });
} export default SafeEventTile;
// XXX this'll eventually be dynamic based on the fields once we have extensible event types // XXX this'll eventually be dynamic based on the fields once we have extensible event types
const messageTypes = [EventType.RoomMessage, EventType.Sticker]; const messageTypes = [EventType.RoomMessage, EventType.Sticker];

View file

@ -21,6 +21,7 @@ import { EventEmitter } from "events";
import * as Matrix from 'matrix-js-sdk/src/matrix'; import * as Matrix from 'matrix-js-sdk/src/matrix';
import FakeTimers from '@sinonjs/fake-timers'; import FakeTimers from '@sinonjs/fake-timers';
import { mount } from "enzyme"; import { mount } from "enzyme";
import * as TestUtils from "react-dom/test-utils";
import { MatrixClientPeg } from '../../../src/MatrixClientPeg'; import { MatrixClientPeg } from '../../../src/MatrixClientPeg';
import sdk from '../../skinned-sdk'; import sdk from '../../skinned-sdk';
@ -28,15 +29,11 @@ import SettingsStore from "../../../src/settings/SettingsStore";
import MatrixClientContext from "../../../src/contexts/MatrixClientContext"; import MatrixClientContext from "../../../src/contexts/MatrixClientContext";
import RoomContext from "../../../src/contexts/RoomContext"; import RoomContext from "../../../src/contexts/RoomContext";
import DMRoomMap from "../../../src/utils/DMRoomMap"; import DMRoomMap from "../../../src/utils/DMRoomMap";
import { upsertRoomStateEvents } from '../../test-utils'; import { UnwrappedEventTile } from "../../../src/components/views/rooms/EventTile";
import * as TestUtilsMatrix from "../../test-utils";
const TestUtils = require('react-dom/test-utils');
const expect = require('expect');
const MessagePanel = sdk.getComponent('structures.MessagePanel'); const MessagePanel = sdk.getComponent('structures.MessagePanel');
const TestUtilsMatrix = require('../../test-utils');
let client; let client;
const room = new Matrix.Room("!roomId:server_name"); 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 // just check we have the right number of tiles for now
const tiles = TestUtils.scryRenderedComponentsWithType( const tiles = TestUtils.scryRenderedComponentsWithType(res, UnwrappedEventTile);
res, sdk.getComponent('rooms.EventTile'));
expect(tiles.length).toEqual(10); expect(tiles.length).toEqual(10);
}); });
@ -299,9 +295,7 @@ describe('MessagePanel', function() {
); );
// just check we have the right number of tiles for now // just check we have the right number of tiles for now
const tiles = TestUtils.scryRenderedComponentsWithType( const tiles = TestUtils.scryRenderedComponentsWithType(res, UnwrappedEventTile);
res, sdk.getComponent('rooms.EventTile'),
);
expect(tiles.length).toEqual(2); expect(tiles.length).toEqual(2);
const summaryTiles = TestUtils.scryRenderedComponentsWithType( const summaryTiles = TestUtils.scryRenderedComponentsWithType(
@ -320,8 +314,7 @@ describe('MessagePanel', function() {
/>, />,
); );
const tiles = TestUtils.scryRenderedComponentsWithType( const tiles = TestUtils.scryRenderedComponentsWithType(res, UnwrappedEventTile);
res, sdk.getComponent('rooms.EventTile'));
// find the <li> which wraps the read marker // find the <li> which wraps the read marker
const rm = TestUtils.findRenderedDOMComponentWithClass(res, 'mx_RoomView_myReadMarker_container'); const rm = TestUtils.findRenderedDOMComponentWithClass(res, 'mx_RoomView_myReadMarker_container');
@ -390,8 +383,7 @@ describe('MessagePanel', function() {
readMarkerVisible={true} readMarkerVisible={true}
/>, parentDiv); />, parentDiv);
const tiles = TestUtils.scryRenderedComponentsWithType( const tiles = TestUtils.scryRenderedComponentsWithType(mp, UnwrappedEventTile);
mp, sdk.getComponent('rooms.EventTile'));
const tileContainers = tiles.map(function(t) { const tileContainers = tiles.map(function(t) {
return ReactDOM.findDOMNode(t); return ReactDOM.findDOMNode(t);
}); });
@ -437,7 +429,7 @@ describe('MessagePanel', function() {
it('should collapse creation events', function() { it('should collapse creation events', function() {
const events = mkCreationEvents(); const events = mkCreationEvents();
upsertRoomStateEvents(room, events); TestUtilsMatrix.upsertRoomStateEvents(room, events);
const res = mount( const res = mount(
<WrappedMessagePanel className="cls" events={events} />, <WrappedMessagePanel className="cls" events={events} />,
); );
@ -447,7 +439,7 @@ describe('MessagePanel', function() {
// should be outside of the room creation summary // should be outside of the room creation summary
// - all other events should be inside 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(0).props().mxEvent.getType()).toEqual("m.room.create");
expect(tiles.at(1).props().mxEvent.getType()).toEqual("m.room.encryption"); 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 summaryTiles = res.find(sdk.getComponent('views.elements.GenericEventListSummary'));
const summaryTile = summaryTiles.at(0); 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 // every event except for the room creation, room encryption, and Bob's
// invite event should be in the event summary // invite event should be in the event summary
expect(summaryEventTiles.length).toEqual(tiles.length - 3); 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() { it('should hide read-marker at the end of creation event summary', function() {
const events = mkCreationEvents(); const events = mkCreationEvents();
upsertRoomStateEvents(room, events); TestUtilsMatrix.upsertRoomStateEvents(room, events);
const res = mount( const res = mount(
<WrappedMessagePanel <WrappedMessagePanel
className="cls" className="cls"