From 50a197014f9a05b6310316e40f3107b2d43fe5da Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Torkel=20=C3=96degaard?= Date: Tue, 8 Nov 2022 16:26:02 +0100 Subject: [PATCH] Panels: Fixes crashing issue when migrating angular panels (#58232) --- .../components/PanelEditor/state/actions.ts | 6 ++-- .../dashboard/dashgrid/DashboardGrid.test.tsx | 3 +- .../dashboard/dashgrid/DashboardGrid.tsx | 17 ++--------- .../dashboard/dashgrid/PanelChromeAngular.tsx | 3 ++ .../app/features/dashboard/state/actions.ts | 2 ++ public/app/features/panel/state/actions.ts | 30 +++---------------- public/app/features/panel/state/reducers.ts | 24 +++------------ 7 files changed, 18 insertions(+), 67 deletions(-) diff --git a/public/app/features/dashboard/components/PanelEditor/state/actions.ts b/public/app/features/dashboard/components/PanelEditor/state/actions.ts index 320f78de20d..b10c845b3f5 100644 --- a/public/app/features/dashboard/components/PanelEditor/state/actions.ts +++ b/public/app/features/dashboard/components/PanelEditor/state/actions.ts @@ -66,10 +66,9 @@ export function updateDuplicateLibraryPanels( panel.configRev++; if (pluginChanged) { - const cleanUpKey = panel.key; panel.generateNewKey(); - dispatch(panelModelAndPluginReady({ key: panel.key, plugin: panel.plugin!, cleanUpKey })); + dispatch(panelModelAndPluginReady({ key: panel.key, plugin: panel.plugin! })); } // Resend last query result on source panel query runner @@ -129,10 +128,9 @@ export function exitPanelEditor(): ThunkResult { if (panelTypeChanged) { // Loaded plugin is not included in the persisted properties so is not handled by restoreModel sourcePanel.plugin = panel.plugin; - const cleanUpKey = sourcePanel.key; sourcePanel.generateNewKey(); - await dispatch(panelModelAndPluginReady({ key: sourcePanel.key, plugin: panel.plugin!, cleanUpKey })); + await dispatch(panelModelAndPluginReady({ key: sourcePanel.key, plugin: panel.plugin! })); } // Resend last query result on source panel query runner diff --git a/public/app/features/dashboard/dashgrid/DashboardGrid.test.tsx b/public/app/features/dashboard/dashgrid/DashboardGrid.test.tsx index 00b0e7e8d37..05af4a5831e 100644 --- a/public/app/features/dashboard/dashgrid/DashboardGrid.test.tsx +++ b/public/app/features/dashboard/dashgrid/DashboardGrid.test.tsx @@ -5,7 +5,7 @@ import { DashboardMeta } from 'app/types'; import { DashboardModel } from '../state'; -import { DashboardGridUnconnected as DashboardGrid, Props } from './DashboardGrid'; +import { DashboardGrid, Props } from './DashboardGrid'; jest.mock('app/features/dashboard/dashgrid/LazyLoader', () => { const LazyLoader: React.FC = ({ children }) => { @@ -58,7 +58,6 @@ describe('DashboardGrid', () => { editPanel: null, viewPanel: null, dashboard: getTestDashboard(), - cleanAndRemoveMany: jest.fn, }; expect(() => render()).not.toThrow(); }); diff --git a/public/app/features/dashboard/dashgrid/DashboardGrid.tsx b/public/app/features/dashboard/dashgrid/DashboardGrid.tsx index 2f273302107..b24841fe679 100644 --- a/public/app/features/dashboard/dashgrid/DashboardGrid.tsx +++ b/public/app/features/dashboard/dashgrid/DashboardGrid.tsx @@ -1,13 +1,11 @@ import classNames from 'classnames'; import React, { PureComponent, CSSProperties } from 'react'; import ReactGridLayout, { ItemCallback } from 'react-grid-layout'; -import { connect, ConnectedProps } from 'react-redux'; import AutoSizer from 'react-virtualized-auto-sizer'; import { Subscription } from 'rxjs'; import { config } from '@grafana/runtime'; import { GRID_CELL_HEIGHT, GRID_CELL_VMARGIN, GRID_COLUMN_COUNT } from 'app/core/constants'; -import { cleanAndRemoveMany } from 'app/features/panel/state/actions'; import { DashboardPanelsChangedEvent } from 'app/types/events'; import { AddPanelWidget } from '../components/AddPanelWidget'; @@ -17,7 +15,7 @@ import { GridPos } from '../state/PanelModel'; import { DashboardPanel } from './DashboardPanel'; -export interface OwnProps { +export interface Props { dashboard: DashboardModel; editPanel: PanelModel | null; viewPanel: PanelModel | null; @@ -27,15 +25,7 @@ export interface State { isLayoutInitialized: boolean; } -const mapDispatchToProps = { - cleanAndRemoveMany, -}; - -const connector = connect(null, mapDispatchToProps); - -export type Props = OwnProps & ConnectedProps; - -export class DashboardGridUnconnected extends PureComponent { +export class DashboardGrid extends PureComponent { private panelMap: { [key: string]: PanelModel } = {}; private eventSubs = new Subscription(); private windowHeight = 1200; @@ -59,7 +49,6 @@ export class DashboardGridUnconnected extends PureComponent { componentWillUnmount() { this.eventSubs.unsubscribe(); - this.props.cleanAndRemoveMany(Object.keys(this.panelMap)); } buildLayout() { @@ -323,5 +312,3 @@ function translateGridHeightToScreenHeight(gridHeight: number): number { } GrafanaGridItem.displayName = 'GridItemWithDimensions'; - -export const DashboardGrid = connector(DashboardGridUnconnected); diff --git a/public/app/features/dashboard/dashgrid/PanelChromeAngular.tsx b/public/app/features/dashboard/dashgrid/PanelChromeAngular.tsx index 7f967d603b1..e1c76ff94cf 100644 --- a/public/app/features/dashboard/dashgrid/PanelChromeAngular.tsx +++ b/public/app/features/dashboard/dashgrid/PanelChromeAngular.tsx @@ -102,6 +102,9 @@ export class PanelChromeAngularUnconnected extends PureComponent { componentWillUnmount() { this.subs.unsubscribe(); + if (this.props.angularComponent) { + this.props.angularComponent?.destroy(); + } } componentDidUpdate(prevProps: Props, prevState: State) { diff --git a/public/app/features/dashboard/state/actions.ts b/public/app/features/dashboard/state/actions.ts index ff718ddf6a9..1b089434123 100644 --- a/public/app/features/dashboard/state/actions.ts +++ b/public/app/features/dashboard/state/actions.ts @@ -3,6 +3,7 @@ import { getBackendSrv } from '@grafana/runtime'; import { notifyApp } from 'app/core/actions'; import { createSuccessNotification } from 'app/core/copy/appNotification'; import { dashboardWatcher } from 'app/features/live/dashboard/dashboardWatcher'; +import { removeAllPanels } from 'app/features/panel/state/reducers'; import { updateTimeZoneForSession, updateWeekStartForSession } from 'app/features/profile/state/reducers'; import { DashboardAcl, DashboardAclUpdateDTO, NewDashboardAclItem, PermissionLevel, ThunkResult } from 'app/types'; @@ -125,6 +126,7 @@ export const cleanUpDashboardAndVariables = (): ThunkResult => (dispatch, getTimeSrv().stopAutoRefresh(); dispatch(cleanUpDashboard()); + dispatch(removeAllPanels()); dashboardWatcher.leave(); }; diff --git a/public/app/features/panel/state/actions.ts b/public/app/features/panel/state/actions.ts index ef12bc3f275..e440554fc25 100644 --- a/public/app/features/panel/state/actions.ts +++ b/public/app/features/panel/state/actions.ts @@ -8,13 +8,7 @@ import { loadPanelPlugin } from 'app/features/plugins/admin/state/actions'; import { ThunkResult } from 'app/types'; import { PanelOptionsChangedEvent, PanelQueriesChangedEvent } from 'app/types/events'; -import { - changePanelKey, - cleanUpAngularComponent, - panelModelAndPluginReady, - removePanel, - removePanels, -} from './reducers'; +import { changePanelKey, panelModelAndPluginReady, removePanel } from './reducers'; export function initPanelState(panel: PanelModel): ThunkResult { return async (dispatch, getStore) => { @@ -45,23 +39,11 @@ export function initPanelState(panel: PanelModel): ThunkResult { } export function cleanUpPanelState(panelKey: string): ThunkResult { - return (dispatch, getStore) => { - const store = getStore().panels; - cleanUpAngularComponent(store[panelKey]); + return (dispatch) => { dispatch(removePanel({ key: panelKey })); }; } -export function cleanAndRemoveMany(panelKeys: string[]): ThunkResult { - return (dispatch, getStore) => { - const store = getStore().panels; - for (const key of panelKeys) { - cleanUpAngularComponent(store[key]); - } - dispatch(removePanels({ keys: panelKeys })); - }; -} - export interface ChangePanelPluginAndOptionsArgs { panel: PanelModel; pluginId: string; @@ -89,8 +71,6 @@ export function changePanelPlugin({ plugin = await dispatch(loadPanelPlugin(pluginId)); } - let cleanUpKey = panel.key; - if (panel.type !== pluginId) { panel.changePlugin(plugin); } @@ -110,7 +90,7 @@ export function changePanelPlugin({ panel.generateNewKey(); - dispatch(panelModelAndPluginReady({ key: panel.key, plugin, cleanUpKey })); + dispatch(panelModelAndPluginReady({ key: panel.key, plugin })); }; } @@ -139,12 +119,10 @@ export function changeToLibraryPanel(panel: PanelModel, libraryPanel: LibraryEle plugin = await dispatch(loadPanelPlugin(newPluginId)); } - const oldKey = panel.key; - panel.pluginLoaded(plugin); panel.generateNewKey(); - await dispatch(panelModelAndPluginReady({ key: panel.key, plugin, cleanUpKey: oldKey })); + await dispatch(panelModelAndPluginReady({ key: panel.key, plugin })); } else { // Even if the plugin is the same, we want to change the key // to force a rerender diff --git a/public/app/features/panel/state/reducers.ts b/public/app/features/panel/state/reducers.ts index 7c61f4f46cc..262b7430af3 100644 --- a/public/app/features/panel/state/reducers.ts +++ b/public/app/features/panel/state/reducers.ts @@ -1,4 +1,4 @@ -import { createSlice, Draft, PayloadAction } from '@reduxjs/toolkit'; +import { createSlice, PayloadAction } from '@reduxjs/toolkit'; import { PanelPlugin } from '@grafana/data'; import { AngularComponent } from '@grafana/runtime'; @@ -18,11 +18,6 @@ const panelsSlice = createSlice({ initialState, reducers: { panelModelAndPluginReady: (state, action: PayloadAction) => { - if (action.payload.cleanUpKey) { - cleanUpAngularComponent(state[action.payload.cleanUpKey]); - delete state[action.payload.cleanUpKey]; - } - state[action.payload.key] = { plugin: action.payload.plugin, }; @@ -34,33 +29,22 @@ const panelsSlice = createSlice({ removePanel: (state, action: PayloadAction<{ key: string }>) => { delete state[action.payload.key]; }, - removePanels: (state, action: PayloadAction<{ keys: string[] }>) => { - for (const key of action.payload.keys) { - delete state[key]; - } + removeAllPanels: (state) => { + Object.keys(state).forEach((key) => delete state[key]); }, setPanelInstanceState: (state, action: PayloadAction) => { state[action.payload.key].instanceState = action.payload.value; }, setPanelAngularComponent: (state, action: PayloadAction) => { const panelState = state[action.payload.key]; - cleanUpAngularComponent(panelState); panelState.angularComponent = action.payload.angularComponent; }, }, }); -export function cleanUpAngularComponent(panelState?: Draft) { - if (panelState?.angularComponent) { - panelState.angularComponent.destroy(); - } -} - export interface PanelModelAndPluginReadyPayload { key: string; plugin: PanelPlugin; - /** Used to cleanup previous state when we change key (used when switching panel plugin) */ - cleanUpKey?: string; } export interface SetPanelAngularComponentPayload { @@ -79,7 +63,7 @@ export const { setPanelInstanceState, changePanelKey, removePanel, - removePanels, + removeAllPanels, } = panelsSlice.actions; export const panelsReducer = panelsSlice.reducer;