Fix excessive re-renders, dragging multiple panels

This commit is contained in:
Aleksandar Petrov
2025-12-02 10:02:10 -04:00
parent 0fc79c4655
commit ba65f18678
8 changed files with 194 additions and 98 deletions
@@ -251,9 +251,7 @@ export function ExploreMapCanvas() {
tabIndex={0}
>
{Object.values(panels).map((panel) => {
// Use remoteVersion in the key to force re-render when remote state changes
// remoteVersion only increments on remote updates, so local edits won't trigger re-render
return <ExploreMapPanelContainer key={`${panel.id}-v${panel.remoteVersion || 0}`} panel={panel} />;
return <ExploreMapPanelContainer key={panel.id} panel={panel} />;
})}
{Object.values(cursors).map((cursor) => (
<UserCursor key={cursor.sessionId} cursor={cursor} />
@@ -1,5 +1,5 @@
import { css, cx } from '@emotion/css';
import { useCallback, useRef, useState } from 'react';
import React, { useCallback, useRef, useState } from 'react';
import { Rnd, RndDragCallback, RndResizeCallback } from 'react-rnd';
import { GrafanaTheme2 } from '@grafana/data';
@@ -10,9 +10,11 @@ import { useDispatch, useSelector } from 'app/types/store';
import { splitClose } from '../../explore/state/main';
import {
bringPanelToFront,
clearActiveDrag,
duplicatePanel,
removePanel,
selectPanel,
setActiveDrag,
updateMultiplePanelPositions,
updatePanelPosition,
updatePanelSize,
@@ -26,7 +28,7 @@ interface ExploreMapPanelContainerProps {
panel: ExploreMapPanel;
}
export function ExploreMapPanelContainer({ panel }: ExploreMapPanelContainerProps) {
function ExploreMapPanelContainerComponent({ panel }: ExploreMapPanelContainerProps) {
const styles = useStyles2(getStyles);
const dispatch = useDispatch();
const rndRef = useRef<Rnd>(null);
@@ -36,6 +38,19 @@ export function ExploreMapPanelContainer({ panel }: ExploreMapPanelContainerProp
const viewport = useSelector((state) => selectViewport(state.exploreMapCRDT));
const isSelected = selectedPanelIds.includes(panel.id);
// Get the active drag info from a parent context (if any panel is being dragged)
const activeDragInfo = useSelector((state) => state.exploreMapCRDT.local.activeDrag);
// Calculate effective position considering active drag
let effectiveX = panel.position.x;
let effectiveY = panel.position.y;
// If another panel in the selection is being dragged, apply the offset to this panel
if (activeDragInfo && activeDragInfo.draggedPanelId !== panel.id && isSelected) {
effectiveX += activeDragInfo.deltaX;
effectiveY += activeDragInfo.deltaY;
}
const handleDragStart: RndDragCallback = useCallback(
(_e, data) => {
setDragStartPos({ x: data.x, y: data.y });
@@ -52,23 +67,24 @@ export function ExploreMapPanelContainer({ panel }: ExploreMapPanelContainerProp
const deltaX = data.x - dragStartPos.x;
const deltaY = data.y - dragStartPos.y;
// If this panel is selected and there are multiple selections, move all others
// If dragging multiple panels, store the drag offset in local state
// This will cause other panels to visually move without CRDT operations
if (isSelected && selectedPanelIds.length > 1) {
dispatch(
updateMultiplePanelPositions({
panelId: panel.id,
deltaX,
deltaY,
})
);
setDragStartPos({ x: data.x, y: data.y });
dispatch(setActiveDrag({
draggedPanelId: panel.id,
deltaX: data.x - panel.position.x,
deltaY: data.y - panel.position.y,
}));
}
},
[dispatch, panel.id, dragStartPos, isSelected, selectedPanelIds.length]
[dispatch, panel.id, panel.position.x, panel.position.y, dragStartPos, isSelected, selectedPanelIds.length]
);
const handleDragStop: RndDragCallback = useCallback(
(_e, data) => {
// Clear the active drag state
dispatch(clearActiveDrag());
if (dragStartPos) {
const deltaX = data.x - dragStartPos.x;
const deltaY = data.y - dragStartPos.y;
@@ -77,9 +93,9 @@ export function ExploreMapPanelContainer({ panel }: ExploreMapPanelContainerProp
const hasMoved = Math.abs(deltaX) > 0.5 || Math.abs(deltaY) > 0.5;
if (hasMoved) {
// Final position update for all panels
if (isSelected && selectedPanelIds.length > 1) {
// Update other panels with the remaining delta
// Multi-panel drag: update all selected panels with the delta
// This creates CRDT operations that will be broadcast
dispatch(
updateMultiplePanelPositions({
panelId: panel.id,
@@ -87,24 +103,16 @@ export function ExploreMapPanelContainer({ panel }: ExploreMapPanelContainerProp
deltaY,
})
);
// Update the dragged panel's position in Redux
dispatch(
updatePanelPosition({
panelId: panel.id,
x: data.x,
y: data.y,
})
);
} else {
// Single panel drag
dispatch(
updatePanelPosition({
panelId: panel.id,
x: data.x,
y: data.y,
})
);
}
// Always update the dragged panel's final position
dispatch(
updatePanelPosition({
panelId: panel.id,
x: data.x,
y: data.y,
})
);
}
}
setDragStartPos(null);
@@ -185,7 +193,7 @@ export function ExploreMapPanelContainer({ panel }: ExploreMapPanelContainerProp
return (
<Rnd
ref={rndRef}
position={{ x: panel.position.x, y: panel.position.y }}
position={{ x: effectiveX, y: effectiveY }}
size={{ width: panel.position.width, height: panel.position.height }}
scale={viewport.zoom}
onDragStart={handleDragStart}
@@ -230,6 +238,9 @@ export function ExploreMapPanelContainer({ panel }: ExploreMapPanelContainerProp
exploreId={panel.exploreId}
width={panel.position.width}
height={panel.position.height - 36}
remoteVersion={panel.remoteVersion}
mode={panel.mode}
exploreState={panel.exploreState}
/>
</div>
</div>
@@ -237,6 +248,25 @@ export function ExploreMapPanelContainer({ panel }: ExploreMapPanelContainerProp
);
}
export const ExploreMapPanelContainer = React.memo(ExploreMapPanelContainerComponent, (prevProps, nextProps) => {
const prev = prevProps.panel;
const next = nextProps.panel;
// Only re-render if these specific properties change
// This prevents re-renders when unrelated panels update
return (
prev.id === next.id &&
prev.position.x === next.position.x &&
prev.position.y === next.position.y &&
prev.position.width === next.position.width &&
prev.position.height === next.position.height &&
prev.position.zIndex === next.position.zIndex &&
prev.remoteVersion === next.remoteVersion &&
prev.exploreId === next.exploreId &&
prev.mode === next.mode
);
});
const getStyles = (theme: GrafanaTheme2) => {
return {
panelContainer: css({
@@ -1,10 +1,10 @@
import { css } from '@emotion/css';
import { useEffect, useMemo, useState } from 'react';
import React, { useEffect, useMemo, useState } from 'react';
import { EventBusSrv, GrafanaTheme2 } from '@grafana/data';
import { Trans } from '@grafana/i18n';
import { useStyles2 } from '@grafana/ui';
import { useDispatch, useSelector } from 'app/types/store';
import { useDispatch } from 'app/types/store';
import { ExplorePaneContainer } from '../../explore/ExplorePaneContainer';
import { DEFAULT_RANGE } from '../../explore/state/constants';
@@ -12,7 +12,6 @@ import { initializeExplore } from '../../explore/state/explorePane';
import { usePanelStateSync } from '../hooks/usePanelStateSync';
// import { useExploreStateReceiver } from '../hooks/useExploreStateReceiver';
// import { useExploreStateSync } from '../hooks/useExploreStateSync';
import { selectPanels } from '../state/selectors';
import { ExploreMapLogsDrilldownPanel } from './Drilldown/ExploreMapLogsDrilldownPanel';
import { ExploreMapMetricsDrilldownPanel } from './Drilldown/ExploreMapMetricsDrilldownPanel';
@@ -24,6 +23,9 @@ interface ExploreMapPanelContentProps {
exploreId: string;
width: number;
height: number;
remoteVersion?: number;
mode?: 'explore' | 'traces-drilldown' | 'metrics-drilldown' | 'profiles-drilldown' | 'logs-drilldown';
exploreState?: any;
}
@@ -83,7 +85,7 @@ function patchGetBoundingClientRect() {
isPatched = true;
}
export function ExploreMapPanelContent({ panelId, exploreId, width, height }: ExploreMapPanelContentProps) {
export const ExploreMapPanelContent = React.memo(function ExploreMapPanelContent({ panelId, exploreId, width, height, remoteVersion, mode, exploreState }: ExploreMapPanelContentProps) {
const styles = useStyles2(getStyles);
const dispatch = useDispatch();
const [isInitialized, setIsInitialized] = useState(false);
@@ -117,19 +119,13 @@ export function ExploreMapPanelContent({ panelId, exploreId, width, height }: Ex
patchGetBoundingClientRect();
}, []);
// Get panel from CRDT state (which has the latest exploreState)
const panel = useSelector((state) => {
const panels = selectPanels(state.exploreMapCRDT);
return panels[panelId];
});
// Initialize Explore pane on mount (only for standard Explore panels)
useEffect(() => {
if (
panel?.mode === 'traces-drilldown' ||
panel?.mode === 'metrics-drilldown' ||
panel?.mode === 'profiles-drilldown' ||
panel?.mode === 'logs-drilldown'
mode === 'traces-drilldown' ||
mode === 'metrics-drilldown' ||
mode === 'profiles-drilldown' ||
mode === 'logs-drilldown'
) {
// Drilldown panels don't need Explore initialization
setIsInitialized(true);
@@ -138,9 +134,7 @@ export function ExploreMapPanelContent({ panelId, exploreId, width, height }: Ex
const initializePane = async () => {
// Use saved state if available, otherwise defaults
const savedState = panel?.exploreState;
console.log('[ExploreMapPanelContent] Initializing with saved state:', savedState);
const savedState = exploreState;
await dispatch(
initializeExplore({
@@ -162,21 +156,21 @@ export function ExploreMapPanelContent({ panelId, exploreId, width, height }: Ex
eventBus.removeAllListeners();
};
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [dispatch, exploreId, eventBus, panel?.exploreState, panel?.mode]);
}, [dispatch, exploreId, eventBus, exploreState, mode]);
if (panel?.mode === 'traces-drilldown') {
if (mode === 'traces-drilldown') {
return <ExploreMapTracesDrilldownPanel exploreId={exploreId} width={width} height={height} />;
}
if (panel?.mode === 'metrics-drilldown') {
if (mode === 'metrics-drilldown') {
return <ExploreMapMetricsDrilldownPanel exploreId={exploreId} width={width} height={height} />;
}
if (panel?.mode === 'profiles-drilldown') {
if (mode === 'profiles-drilldown') {
return <ExploreMapProfilesDrilldownPanel exploreId={exploreId} width={width} height={height} />;
}
if (panel?.mode === 'logs-drilldown') {
if (mode === 'logs-drilldown') {
return <ExploreMapLogsDrilldownPanel exploreId={exploreId} width={width} height={height} />;
}
@@ -202,7 +196,23 @@ export function ExploreMapPanelContent({ panelId, exploreId, width, height }: Ex
<ExplorePaneContainer exploreId={exploreId} />
</div>
);
}
}, (prevProps, nextProps) => {
// Only re-render if width, height, remoteVersion, or mode changes
// This prevents re-renders when only position changes (local drag operations)
//
// NOTE: We deliberately exclude exploreState from comparison because:
// 1. It's only used during initialization (mount)
// 2. After initialization, the component manages state via Redux
// 3. exploreState is an object that gets new references even when data is the same
return (
prevProps.panelId === nextProps.panelId &&
prevProps.exploreId === nextProps.exploreId &&
prevProps.width === nextProps.width &&
prevProps.height === nextProps.height &&
prevProps.remoteVersion === nextProps.remoteVersion &&
prevProps.mode === nextProps.mode
);
});
const getStyles = (theme: GrafanaTheme2) => {
return {
+18 -2
View File
@@ -501,10 +501,14 @@ export class CRDTStateManager {
const xUpdated = panelData.positionX.set(x, operation.timestamp);
const yUpdated = panelData.positionY.set(y, operation.timestamp);
const updated = xUpdated || yUpdated;
// Note: We don't increment remoteVersion for position changes
// Position updates should not trigger content re-renders
return {
success: true,
applied: xUpdated || yUpdated,
applied: updated,
};
}
@@ -518,10 +522,14 @@ export class CRDTStateManager {
const widthUpdated = panelData.width.set(width, operation.timestamp);
const heightUpdated = panelData.height.set(height, operation.timestamp);
const updated = widthUpdated || heightUpdated;
// Note: Size changes are handled by width/height props in ExploreMapPanelContent
// They trigger re-renders through the React.memo comparison, not remoteVersion
return {
success: true,
applied: widthUpdated || heightUpdated,
applied: updated,
};
}
@@ -535,6 +543,9 @@ export class CRDTStateManager {
const updated = panelData.zIndex.set(zIndex, operation.timestamp);
// Note: We don't increment remoteVersion for zIndex changes
// zIndex is a visual property that doesn't affect content
return {
success: true,
applied: updated,
@@ -572,6 +583,11 @@ export class CRDTStateManager {
const updated = panelData.iframeUrl.set(iframeUrl, operation.timestamp);
// Increment remoteVersion only for remote operations
if (updated && operation.nodeId !== this.nodeId) {
panelData.remoteVersion++;
}
return {
success: true,
applied: updated,
@@ -6,6 +6,7 @@
*/
import { useEffect, useRef } from 'react';
import { shallowEqual } from 'react-redux';
import { useDispatch, useSelector } from 'app/types/store';
@@ -30,16 +31,20 @@ export function usePanelStateSync({ panelId, exploreId }: UsePanelStateSyncOptio
// Track the last synced state
const lastSyncedStateRef = useRef<string | null>(null);
// Get current selection state
const selectedPanelIds = useSelector((state) => selectSelectedPanelIds(state.exploreMapCRDT));
const isSelected = selectedPanelIds.includes(panelId);
// Get current selection state - memoized to only change when THIS panel's selection changes
const isSelected = useSelector((state) => {
const selectedIds = selectSelectedPanelIds(state.exploreMapCRDT);
return selectedIds.includes(panelId);
});
// Get current explore state
const explorePane = useSelector((state) => state.explore?.panes?.[exploreId]);
// Get current explore state - using shallowEqual to prevent re-renders when content is the same
const explorePane = useSelector((state) => state.explore?.panes?.[exploreId], shallowEqual);
// Get the panel's saved state
const panels = useSelector((state) => selectPanels(state.exploreMapCRDT));
const panel = panels[panelId];
// Get the panel's saved exploreState only - using shallowEqual to prevent re-renders
const panelExploreState = useSelector((state) => {
const panels = selectPanels(state.exploreMapCRDT);
return panels[panelId]?.exploreState;
}, shallowEqual);
useEffect((): void | (() => void) => {
// Update tracking ref
@@ -48,13 +53,10 @@ export function usePanelStateSync({ panelId, exploreId }: UsePanelStateSyncOptio
// Detect deselection (was selected, now not selected)
if (previouslySelected && !isSelected) {
console.log('[PanelStateSync] Panel deselected:', panelId);
// Add a small delay to ensure explore state updates have been committed to Redux
const syncTimer = setTimeout(() => {
// Get current explore state (after delay to ensure it's updated)
if (!explorePane) {
console.log('[PanelStateSync] No explore pane found for', exploreId);
return;
}
@@ -68,13 +70,6 @@ export function usePanelStateSync({ panelId, exploreId }: UsePanelStateSyncOptio
compact: explorePane.compact,
};
// Log detailed info about queries and datasource
console.log('[PanelStateSync] Current state:', {
datasourceUid: currentState.datasourceUid,
queriesCount: currentState.queries.length,
queries: currentState.queries,
});
// Create a stable string representation focusing on the important fields
const currentStateStr = JSON.stringify({
datasourceUid: currentState.datasourceUid,
@@ -82,15 +77,10 @@ export function usePanelStateSync({ panelId, exploreId }: UsePanelStateSyncOptio
range: currentState.range,
});
console.log('[PanelStateSync] Last synced state:', lastSyncedStateRef.current?.substring(0, 100));
console.log('[PanelStateSync] Current state str:', currentStateStr.substring(0, 100));
// Check if state has changed since last sync
const hasChanged = lastSyncedStateRef.current !== currentStateStr;
if (hasChanged) {
console.log('[PanelStateSync] State changed, syncing full state');
// Dispatch sync action with full state
dispatch(savePanelExploreState({
panelId,
@@ -99,8 +89,6 @@ export function usePanelStateSync({ panelId, exploreId }: UsePanelStateSyncOptio
// Update last synced state
lastSyncedStateRef.current = currentStateStr;
} else {
console.log('[PanelStateSync] State unchanged, skipping sync');
}
}, 100); // Small delay to ensure state is updated
@@ -111,15 +99,14 @@ export function usePanelStateSync({ panelId, exploreId }: UsePanelStateSyncOptio
// Initialize last synced state from panel's saved state
useEffect(() => {
if (panel?.exploreState && lastSyncedStateRef.current === null) {
if (panelExploreState && lastSyncedStateRef.current === null) {
// Use the same format as in the main effect for consistency
const initStateStr = JSON.stringify({
datasourceUid: panel.exploreState.datasourceUid,
queries: panel.exploreState.queries || [],
range: panel.exploreState.range,
datasourceUid: panelExploreState.datasourceUid,
queries: panelExploreState.queries || [],
range: panelExploreState.range,
});
lastSyncedStateRef.current = initStateStr;
console.log('[PanelStateSync] Initialized with saved state:', initStateStr.substring(0, 100));
}
}, [panel?.exploreState]);
}, [panelExploreState]);
}
@@ -83,7 +83,7 @@ export function validateOperation(
validateAddComment(operation, opts, errors);
break;
case 'remove-comment':
validateRemoveComment(operation, errors);
validateRemoveComment(operation, opts, errors);
break;
case 'batch':
validateBatchOperation(operation, opts, errors);
@@ -11,7 +11,7 @@ import { v4 as uuidv4 } from 'uuid';
import { generateExploreId } from 'app/core/utils/explore';
import { CRDTStateManager } from '../crdt/state';
import { CRDTOperation, CommentData } from '../crdt/types';
import { CRDTOperation, CommentData, CRDTExploreMapStateJSON } from '../crdt/types';
import { CanvasViewport, SerializedExploreState, UserCursor } from './types';
@@ -41,6 +41,11 @@ export interface ExploreMapCRDTState {
cursors: Record<string, UserCursor>;
isOnline: boolean;
isSyncing: boolean;
activeDrag?: {
draggedPanelId: string;
deltaX: number;
deltaY: number;
};
};
}
@@ -70,6 +75,7 @@ export function createInitialCRDTState(mapUid?: string): ExploreMapCRDTState {
cursors: {},
isOnline: false,
isSyncing: false,
activeDrag: undefined,
},
};
}
@@ -111,7 +117,7 @@ const crdtSlice = createSlice({
/**
* Load CRDT state from server
*/
loadState: (state, action: PayloadAction<{ crdtState: any }>) => {
loadState: (state, action: PayloadAction<{ crdtState: CRDTExploreMapStateJSON }>) => {
const manager = CRDTStateManager.fromJSON(action.payload.crdtState, state.nodeId);
saveCRDTManager(state, manager);
// Restore uid from the loaded CRDT state
@@ -507,7 +513,7 @@ const crdtSlice = createSlice({
width: sourcePanel.position.width,
height: sourcePanel.position.height,
},
(sourcePanel.mode || 'explore') as 'explore' | 'traces-drilldown' | 'metrics-drilldown' | 'profiles-drilldown' | 'logs-drilldown'
sourcePanel.mode || 'explore'
);
manager.applyOperation(addOperation);
@@ -612,6 +618,20 @@ const crdtSlice = createSlice({
state.local.isSyncing = action.payload.isSyncing;
},
/**
* Set active drag info (for multi-panel drag visual feedback)
*/
setActiveDrag: (state, action: PayloadAction<{ draggedPanelId: string; deltaX: number; deltaY: number }>) => {
state.local.activeDrag = action.payload;
},
/**
* Clear active drag info
*/
clearActiveDrag: (state) => {
state.local.activeDrag = undefined;
},
/**
* Clear all state (reset)
*/
@@ -648,6 +668,8 @@ export const {
removeCursor,
setOnlineStatus,
setSyncingStatus,
setActiveDrag,
clearActiveDrag,
clearMap,
} = crdtSlice.actions;
@@ -24,8 +24,18 @@ function getCRDTManager(state: ExploreMapCRDTState): CRDTStateManager {
return CRDTStateManager.fromJSON(json, state.nodeId);
}
/**
* Cache for individual panel objects
* Key: panelId, Value: { panel object, serialized panel data for comparison }
*/
const panelCache = new Map<string, { panel: ExploreMapPanel; data: string }>();
/**
* Select all panels as a Record (for compatibility with existing UI)
*
* OPTIMIZATION: This selector caches individual panel objects and only creates
* new panel objects when the underlying CRDT data for that specific panel changes.
* This prevents unnecessary re-renders of unchanged panels when another panel is modified.
*/
export const selectPanels = createSelector(
[
@@ -50,11 +60,34 @@ export const selectPanels = createSelector(
};
const manager = getCRDTManager(state);
const panels: Record<string, ExploreMapPanel> = {};
const currentPanelIds = new Set(manager.getPanelIds());
for (const panelId of manager.getPanelIds()) {
// Clean up cache for removed panels
for (const cachedPanelId of panelCache.keys()) {
if (!currentPanelIds.has(cachedPanelId)) {
panelCache.delete(cachedPanelId);
}
}
// Build panels object, reusing cached objects when data hasn't changed
for (const panelId of currentPanelIds) {
const panelData = manager.getPanelForUI(panelId);
if (panelData) {
panels[panelId] = panelData as ExploreMapPanel;
if (!panelData) {
continue;
}
// Serialize the panel data to detect changes
const serializedData = JSON.stringify(panelData);
const cached = panelCache.get(panelId);
// Reuse cached panel if data hasn't changed
if (cached && cached.data === serializedData) {
panels[panelId] = cached.panel;
} else {
// Panel is new or changed, create new object and cache it
const panel = panelData as ExploreMapPanel;
panels[panelId] = panel;
panelCache.set(panelId, { panel, data: serializedData });
}
}