Skip to content

Commit 7635a4c

Browse files
committed
fix: stabilize message viewer subComponent identity to prevent Monaco editor remounting
The subComponent prop passed to DataTable was an inline arrow function, causing React to treat it as a new component type on every MobX-triggered re-render. During gRPC streaming searches, each arriving message triggered a re-render that unmounted and remounted all expanded Monaco editors, leading to 21+ seconds of scripting on protobuf-schema topics. - Wrap subComponent, onCopyKey, onCopyValue in useCallback for stable refs - Disable Monaco automaticLayout, use single debounced ResizeObserver instead - Memoize JSON.stringify in KowlJsonView to avoid redundant serialization - Remove dead expandedKeys/toggleRecordExpand code from pre-DataTable era
1 parent f14840d commit 7635a4c

2 files changed

Lines changed: 80 additions & 63 deletions

File tree

frontend/src/components/misc/KowlJsonView.tsx

Lines changed: 35 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -11,15 +11,40 @@
1111

1212
import { Box } from '@redpanda-data/ui';
1313
import { observer } from 'mobx-react';
14-
import type { CSSProperties } from 'react';
15-
import KowlEditor from './KowlEditor';
14+
import { type CSSProperties, useEffect, useMemo, useRef } from 'react';
15+
import KowlEditor, { type IStandaloneCodeEditor } from './KowlEditor';
1616

1717
export const KowlJsonView = observer(
1818
(props: {
1919
srcObj: object | string | null | undefined;
2020
style?: CSSProperties;
2121
}) => {
22-
const str = typeof props.srcObj === 'string' ? props.srcObj : JSON.stringify(props.srcObj, undefined, 4);
22+
const str = useMemo(
23+
() => (typeof props.srcObj === 'string' ? props.srcObj : JSON.stringify(props.srcObj, undefined, 4)),
24+
[props.srcObj],
25+
);
26+
27+
const editorRef = useRef<IStandaloneCodeEditor | null>(null);
28+
const containerRef = useRef<HTMLDivElement | null>(null);
29+
30+
useEffect(() => {
31+
const container = containerRef.current;
32+
if (!container) return;
33+
34+
let timeoutId: ReturnType<typeof setTimeout>;
35+
const observer = new ResizeObserver(() => {
36+
clearTimeout(timeoutId);
37+
timeoutId = setTimeout(() => {
38+
editorRef.current?.layout();
39+
}, 150);
40+
});
41+
42+
observer.observe(container);
43+
return () => {
44+
observer.disconnect();
45+
clearTimeout(timeoutId);
46+
};
47+
}, []);
2348

2449
return (
2550
<Box
@@ -34,16 +59,19 @@ export const KowlJsonView = observer(
3459
We have a problem in Safari with Monaco editor, when used with automaticLayout: true, which is a default,
3560
it causes an infinite loop in Safari. It recalculates in a wrong way, changes the dimensions of a parent
3661
and that triggers ResizeObserver again (see the internal implementation).
37-
We tried to play with overflow, boxSizing, even manually using ResizeObserver.
38-
Changing the parent to absolutely positioned element works around the issue for now.
62+
We disable automaticLayout and instead use a single debounced ResizeObserver per editor to avoid both
63+
the Safari loop and the performance overhead of many concurrent ResizeObservers during re-renders.
3964
*/}
40-
<Box position="absolute" h="full" w="full">
65+
<Box position="absolute" h="full" w="full" ref={containerRef}>
4166
<KowlEditor
4267
value={str}
4368
language="json"
69+
onMount={(editor) => {
70+
editorRef.current = editor;
71+
}}
4472
options={{
4573
readOnly: true,
46-
// automaticLayout: false // too much lag on chrome
74+
automaticLayout: false,
4775
}}
4876
/>
4977
</Box>

frontend/src/components/pages/topics/Tab.Messages/index.tsx

Lines changed: 45 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -10,18 +10,9 @@
1010
*/
1111

1212
import { DownloadIcon, KebabHorizontalIcon, SkipIcon, SyncIcon, XCircleIcon } from '@primer/octicons-react';
13-
import {
14-
type IReactionDisposer,
15-
action,
16-
autorun,
17-
computed,
18-
makeObservable,
19-
observable,
20-
transaction,
21-
untracked,
22-
} from 'mobx';
13+
import { type IReactionDisposer, autorun, computed, makeObservable, observable, transaction, untracked } from 'mobx';
2314
import { observer } from 'mobx-react';
24-
import React, { Component, type FC, type ReactNode, useState } from 'react';
15+
import React, { Component, type FC, type ReactNode, useCallback, useState } from 'react';
2516
import { type MessageSearch, type MessageSearchRequest, api, createMessageSearch } from '../../../../state/backendApi';
2617
import type { Payload, Topic, TopicAction } from '../../../../state/restInterfaces';
2718
import type { TopicMessage } from '../../../../state/restInterfaces';
@@ -250,8 +241,6 @@ export class TopicMessageView extends Component<TopicMessageViewProps> {
250241
currentSearchRun: string | null = null;
251242

252243
@observable downloadMessages: TopicMessage[] | null = null;
253-
@observable expandedKeys: React.Key[] = [];
254-
255244
constructor(props: TopicMessageViewProps) {
256245
super(props);
257246
this.executeMessageSearch = this.executeMessageSearch.bind(this); // needed because we must pass the function directly as 'submit' prop
@@ -750,29 +739,35 @@ export class TopicMessageView extends Component<TopicMessageViewProps> {
750739
const tsFormat = uiState.topicSettings.previewTimestamps;
751740
const hasKeyTags = uiState.topicSettings.previewTags.count((x) => x.isActive && x.searchInMessageKey) > 0;
752741

753-
function onCopyValue(original: TopicMessage) {
754-
navigator.clipboard
755-
.writeText(getPayloadAsString(original.value.payload ?? original.value.rawBytes))
756-
.then(() => {
757-
toast({
758-
status: 'success',
759-
description: 'Value copied to clipboard',
760-
});
761-
})
762-
.catch(navigatorClipboardErrorHandler);
763-
}
742+
const onCopyValue = useCallback(
743+
(original: TopicMessage) => {
744+
navigator.clipboard
745+
.writeText(getPayloadAsString(original.value.payload ?? original.value.rawBytes))
746+
.then(() => {
747+
toast({
748+
status: 'success',
749+
description: 'Value copied to clipboard',
750+
});
751+
})
752+
.catch(navigatorClipboardErrorHandler);
753+
},
754+
[toast],
755+
);
764756

765-
function onCopyKey(original: TopicMessage) {
766-
navigator.clipboard
767-
.writeText(getPayloadAsString(original.key.payload ?? original.key.rawBytes))
768-
.then(() => {
769-
toast({
770-
status: 'success',
771-
description: 'Key copied to clipboard',
772-
});
773-
})
774-
.catch(navigatorClipboardErrorHandler);
775-
}
757+
const onCopyKey = useCallback(
758+
(original: TopicMessage) => {
759+
navigator.clipboard
760+
.writeText(getPayloadAsString(original.key.payload ?? original.key.rawBytes))
761+
.then(() => {
762+
toast({
763+
status: 'success',
764+
description: 'Key copied to clipboard',
765+
});
766+
})
767+
.catch(navigatorClipboardErrorHandler);
768+
},
769+
[toast],
770+
);
776771

777772
const isValueDeserializerActive =
778773
uiState.topicSettings.searchParams.valueDeserializer !== null &&
@@ -991,18 +986,21 @@ export class TopicMessageView extends Component<TopicMessageViewProps> {
991986
query.pageSize = String(pageSize);
992987
});
993988
})}
994-
subComponent={({ row: { original } }) => (
995-
<ExpandedMessage
996-
msg={original}
997-
loadLargeMessage={() =>
998-
this.loadLargeMessage(this.props.topic.topicName, original.partitionID, original.offset)
999-
}
1000-
onDownloadRecord={() => {
1001-
this.downloadMessages = [original];
1002-
}}
1003-
onCopyKey={onCopyKey}
1004-
onCopyValue={onCopyValue}
1005-
/>
989+
subComponent={useCallback(
990+
({ row: { original } }: { row: { original: TopicMessage } }) => (
991+
<ExpandedMessage
992+
msg={original}
993+
loadLargeMessage={() =>
994+
this.loadLargeMessage(this.props.topic.topicName, original.partitionID, original.offset)
995+
}
996+
onDownloadRecord={() => {
997+
this.downloadMessages = [original];
998+
}}
999+
onCopyKey={onCopyKey}
1000+
onCopyValue={onCopyValue}
1001+
/>
1002+
),
1003+
[onCopyKey, onCopyValue],
10061004
)}
10071005
/>
10081006
<Button
@@ -1044,15 +1042,6 @@ export class TopicMessageView extends Component<TopicMessageViewProps> {
10441042
);
10451043
});
10461044

1047-
@action toggleRecordExpand(r: TopicMessage) {
1048-
const key = `${r.offset} ${r.partitionID}${r.timestamp}`;
1049-
// try collapsing it, removeAll returns the number of matches
1050-
const removed = this.expandedKeys.removeAll((x) => x === key);
1051-
if (removed === 0)
1052-
// wasn't expanded, so expand it now
1053-
this.expandedKeys.push(key);
1054-
}
1055-
10561045
async executeMessageSearch(): Promise<TopicMessage[]> {
10571046
const searchParams = uiState.topicSettings.searchParams;
10581047
const canUseFilters =

0 commit comments

Comments
 (0)