Dashboard: Refactor panel cleanup (#47323) (#48134)

(cherry picked from commit f10047b708)

.betterer.results

@@ -203,7 +203,7 @@ exports[`no enzyme tests`] = {
     "public/app/features/dashboard/components/ShareModal/ShareLink.test.tsx:2357087833": [
       [0, 35, 13, "RegExp match", "2409514259"]
     ],
-    "public/app/features/dashboard/dashgrid/DashboardGrid.test.tsx:3341831113": [
+    "public/app/features/dashboard/dashgrid/DashboardGrid.test.tsx:2723773538": [
       [0, 35, 13, "RegExp match", "2409514259"]
     ],
     "public/app/features/dashboard/dashgrid/PanelHeader/PanelHeaderCorner.test.tsx:2851646279": [

public/app/features/dashboard/components/PanelEditor/PanelEditor.tsx

@@ -252,7 +252,6 @@ export class PanelEditorUnconnected extends PureComponent<Props> {
                       lazy={false}
                       width={panelSize.width}
                       height={panelSize.height}
-                      skipStateCleanUp={true}
                     />
                   </div>
                 </div>

public/app/features/dashboard/components/PanelEditor/state/actions.test.ts

@@ -1,4 +1,4 @@
-import { cleanUpPanelState, panelModelAndPluginReady } from 'app/features/panel/state/reducers';
+import { panelModelAndPluginReady, removePanel } from 'app/features/panel/state/reducers';
 import { getPanelPlugin } from 'app/features/plugins/__mocks__/pluginMocks';
 
 import { thunkTester } from '../../../../../../test/core/thunk/thunkTester';
@@ -50,6 +50,7 @@ describe('panelEditor actions', () => {
       };
 
       const dispatchedActions = await thunkTester({
+        panels: {},
         panelEditor: state,
         dashboard: {
           getModel: () => dashboard,
@@ -59,7 +60,7 @@ describe('panelEditor actions', () => {
         .whenThunkIsDispatched();
 
       expect(dispatchedActions.length).toBe(2);
-      expect(dispatchedActions[0].type).toBe(cleanUpPanelState.type);
+      expect(dispatchedActions[0].type).toBe(removePanel.type);
       expect(dispatchedActions[1].type).toBe(closeEditor.type);
       expect(sourcePanel.getOptions()).toEqual({ prop: true });
       expect(sourcePanel.id).toEqual(12);
@@ -86,6 +87,7 @@ describe('panelEditor actions', () => {
 
       const dispatchedActions = await thunkTester({
         panelEditor: state,
+        panels: {},
         dashboard: {
           getModel: () => dashboard,
         },
@@ -121,6 +123,7 @@ describe('panelEditor actions', () => {
 
       const dispatchedActions = await thunkTester({
         panelEditor: state,
+        panels: {},
         dashboard: {
           getModel: () => dashboard,
         },

public/app/features/dashboard/components/PanelEditor/state/actions.ts

@@ -1,8 +1,8 @@
 import { pick } from 'lodash';
 
 import store from 'app/core/store';
-import { initPanelState } from 'app/features/panel/state/actions';
-import { cleanUpPanelState, panelModelAndPluginReady } from 'app/features/panel/state/reducers';
+import { cleanUpPanelState, initPanelState } from 'app/features/panel/state/actions';
+import { panelModelAndPluginReady } from 'app/features/panel/state/reducers';
 import { ThunkResult } from 'app/types';
 
 import { DashboardModel, PanelModel } from '../../../state';
@@ -66,9 +66,10 @@ export function updateDuplicateLibraryPanels(
       panel.configRev++;
 
       if (pluginChanged) {
+        const cleanUpKey = panel.key;
         panel.generateNewKey();
 
-        dispatch(panelModelAndPluginReady({ key: panel.key, plugin: panel.plugin! }));
+        dispatch(panelModelAndPluginReady({ key: panel.key, plugin: panel.plugin!, cleanUpKey }));
       }
 
       // Resend last query result on source panel query runner
@@ -128,9 +129,10 @@ export function exitPanelEditor(): ThunkResult<void> {
       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! }));
+        await dispatch(panelModelAndPluginReady({ key: sourcePanel.key, plugin: panel.plugin!, cleanUpKey }));
       }
 
       // Resend last query result on source panel query runner
@@ -141,7 +143,7 @@ export function exitPanelEditor(): ThunkResult<void> {
       }, 20);
     }
 
-    dispatch(cleanUpPanelState({ key: panel.key }));
+    dispatch(cleanUpPanelState(panel.key));
     dispatch(closeEditor());
   };
 }

public/app/features/dashboard/dashgrid/DashboardGrid.test.tsx

@@ -3,7 +3,7 @@ import React from 'react';
 
 import { DashboardModel } from '../state';
 
-import { DashboardGrid, Props } from './DashboardGrid';
+import { DashboardGridUnconnected as DashboardGrid, Props } from './DashboardGrid';
 
 jest.mock('app/features/dashboard/dashgrid/LazyLoader', () => {
   const LazyLoader: React.FC = ({ children }) => {
@@ -69,6 +69,7 @@ function dashboardGridScenario(description: string, scenarioFn: (ctx: ScenarioCo
         editPanel: null,
         viewPanel: null,
         dashboard: getTestDashboard(),
+        cleanAndRemoveMany: jest.fn,
       },
       setProps: (props: Partial<Props>) => {
         Object.assign(ctx.props, props);

public/app/features/dashboard/dashgrid/DashboardGrid.tsx

@@ -1,11 +1,13 @@
 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';
@@ -15,7 +17,7 @@ import { GridPos } from '../state/PanelModel';
 
 import { DashboardPanel } from './DashboardPanel';
 
-export interface Props {
+export interface OwnProps {
   dashboard: DashboardModel;
   editPanel: PanelModel | null;
   viewPanel: PanelModel | null;
@@ -25,7 +27,15 @@ export interface State {
   isLayoutInitialized: boolean;
 }
 
-export class DashboardGrid extends PureComponent<Props, State> {
+const mapDispatchToProps = {
+  cleanAndRemoveMany,
+};
+
+const connector = connect(null, mapDispatchToProps);
+
+export type Props = OwnProps & ConnectedProps<typeof connector>;
+
+export class DashboardGridUnconnected extends PureComponent<Props, State> {
   private panelMap: { [key: string]: PanelModel } = {};
   private eventSubs = new Subscription();
   private windowHeight = 1200;
@@ -49,6 +59,7 @@ export class DashboardGrid extends PureComponent<Props, State> {
 
   componentWillUnmount() {
     this.eventSubs.unsubscribe();
+    this.props.cleanAndRemoveMany(Object.keys(this.panelMap));
   }
 
   buildLayout() {
@@ -313,3 +324,5 @@ function translateGridHeightToScreenHeight(gridHeight: number): number {
 }
 
 GrafanaGridItem.displayName = 'GridItemWithDimensions';
+
+export const DashboardGrid = connector(DashboardGridUnconnected);

public/app/features/dashboard/dashgrid/DashboardPanel.tsx

@@ -4,7 +4,7 @@ import { connect, ConnectedProps } from 'react-redux';
 import { StoreState } from 'app/types';
 
 import { initPanelState } from '../../panel/state/actions';
-import { cleanUpPanelState, setPanelInstanceState } from '../../panel/state/reducers';
+import { setPanelInstanceState } from '../../panel/state/reducers';
 import { DashboardModel, PanelModel } from '../state';
 
 import { LazyLoader } from './LazyLoader';
@@ -19,7 +19,6 @@ export interface OwnProps {
   isViewing: boolean;
   width: number;
   height: number;
-  skipStateCleanUp?: boolean;
   lazy?: boolean;
 }
 
@@ -37,7 +36,6 @@ const mapStateToProps = (state: StoreState, props: OwnProps) => {
 
 const mapDispatchToProps = {
   initPanelState,
-  cleanUpPanelState,
   setPanelInstanceState,
 };
 
@@ -50,8 +48,6 @@ export class DashboardPanelUnconnected extends PureComponent<Props> {
     lazy: true,
   };
 
-  specialPanels: { [key: string]: Function } = {};
-
   componentDidMount() {
     this.props.panel.isInView = !this.props.lazy;
     if (!this.props.lazy) {
@@ -59,13 +55,6 @@ export class DashboardPanelUnconnected extends PureComponent<Props> {
     }
   }
 
-  componentWillUnmount() {
-    // Most of the time an unmount should result in cleanup but in PanelEdit it should not
-    if (!this.props.skipStateCleanUp) {
-      this.props.cleanUpPanelState({ key: this.props.stateKey });
-    }
-  }
-
   onInstanceStateChange = (value: any) => {
     this.props.setPanelInstanceState({ key: this.props.stateKey, value });
   };

public/app/features/dashboard/utils/panel.ts

@@ -1,5 +1,7 @@
 import { isString as _isString } from 'lodash';
 
+// Store
+
 import { TimeRange, AppEvents, rangeUtil, dateMath, PanelModel as IPanelModel } from '@grafana/data';
 import { getTemplateSrv } from '@grafana/runtime';
 import appEvents from 'app/core/app_events';
@@ -11,6 +13,8 @@ import { DashboardModel } from 'app/features/dashboard/state/DashboardModel';
 import { PanelModel } from 'app/features/dashboard/state/PanelModel';
 import { AddLibraryPanelModal } from 'app/features/library-panels/components/AddLibraryPanelModal/AddLibraryPanelModal';
 import { UnlinkModal } from 'app/features/library-panels/components/UnlinkModal/UnlinkModal';
+import { cleanUpPanelState } from 'app/features/panel/state/actions';
+import { dispatch } from 'app/store/store';
 
 import { ShowConfirmModalEvent, ShowModalReactEvent } from '../../../types/events';
 
@@ -38,6 +42,7 @@ export const removePanel = (dashboard: DashboardModel, panel: PanelModel, ask: b
   }
 
   dashboard.removePanel(panel);
+  dispatch(cleanUpPanelState(panel.key));
 };
 
 export const duplicatePanel = (dashboard: DashboardModel, panel: PanelModel) => {

public/app/features/panel/state/actions.ts

@@ -8,7 +8,13 @@ import { loadPanelPlugin } from 'app/features/plugins/admin/state/actions';
 import { ThunkResult } from 'app/types';
 import { PanelOptionsChangedEvent, PanelQueriesChangedEvent } from 'app/types/events';
 
-import { changePanelKey, panelModelAndPluginReady } from './reducers';
+import {
+  changePanelKey,
+  cleanUpAngularComponent,
+  panelModelAndPluginReady,
+  removePanel,
+  removePanels,
+} from './reducers';
 
 export function initPanelState(panel: PanelModel): ThunkResult<void> {
   return async (dispatch, getStore) => {
@@ -32,6 +38,24 @@ export function initPanelState(panel: PanelModel): ThunkResult<void> {
   };
 }
 
+export function cleanUpPanelState(panelKey: string): ThunkResult<void> {
+  return (dispatch, getStore) => {
+    const store = getStore().panels;
+    cleanUpAngularComponent(store[panelKey]);
+    dispatch(removePanel({ key: panelKey }));
+  };
+}
+
+export function cleanAndRemoveMany(panelKeys: string[]): ThunkResult<void> {
+  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;

public/app/features/panel/state/reducers.ts

@@ -31,10 +31,14 @@ const panelsSlice = createSlice({
       state[action.payload.newKey] = state[action.payload.oldKey];
       delete state[action.payload.oldKey];
     },
-    cleanUpPanelState: (state, action: PayloadAction<{ key: string }>) => {
-      cleanUpAngularComponent(state[action.payload.key]);
+    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];
+      }
+    },
     setPanelInstanceState: (state, action: PayloadAction<SetPanelInstanceStatePayload>) => {
       state[action.payload.key].instanceState = action.payload.value;
     },
@@ -46,7 +50,7 @@ const panelsSlice = createSlice({
   },
 });
 
-function cleanUpAngularComponent(panelState?: Draft<PanelState>) {
+export function cleanUpAngularComponent(panelState?: Draft<PanelState>) {
   if (panelState?.angularComponent) {
     panelState.angularComponent.destroy();
   }
@@ -73,8 +77,9 @@ export const {
   panelModelAndPluginReady,
   setPanelAngularComponent,
   setPanelInstanceState,
-  cleanUpPanelState,
   changePanelKey,
+  removePanel,
+  removePanels,
 } = panelsSlice.actions;
 
 export const panelsReducer = panelsSlice.reducer;