Explore: Improve Explore performance but removing unnecessary re-renders (#29752)

* Fix Explore and Logs rerenders

* Create ReturnToDashboardButton

* Add test for new component

* Remove queryKeys change on keystroke

* Move rendering conditions to Button

* Remove redundant logic

public/app/features/explore/ExploreToolbar.tsx

@@ -6,28 +6,23 @@ import classNames from 'classnames';
 import { css } from 'emotion';
 
 import { ExploreId, ExploreItemState } from 'app/types/explore';
-import { Icon, IconButton, LegacyForms, SetInterval, Tooltip } from '@grafana/ui';
-import { DataQuery, DataSourceInstanceSettings, RawTimeRange, TimeRange, TimeZone } from '@grafana/data';
+import { Icon, IconButton, SetInterval, Tooltip } from '@grafana/ui';
+import { DataSourceInstanceSettings, RawTimeRange, TimeRange, TimeZone } from '@grafana/data';
 import { DataSourcePicker } from 'app/core/components/Select/DataSourcePicker';
 import { StoreState } from 'app/types/store';
 import { createAndCopyShortLink } from 'app/core/utils/shortLinks';
 import { changeDatasource } from './state/datasource';
 import { splitClose, splitOpen } from './state/main';
 import { syncTimes, changeRefreshInterval } from './state/time';
-import { updateLocation } from 'app/core/actions';
 import { getTimeZone } from '../profile/state/selectors';
 import { updateTimeZoneForSession } from '../profile/state/reducers';
-import { getDashboardSrv } from '../dashboard/services/DashboardSrv';
-import kbn from '../../core/utils/kbn';
 import { ExploreTimeControls } from './ExploreTimeControls';
 import { LiveTailButton } from './LiveTailButton';
 import { ResponsiveButton } from './ResponsiveButton';
 import { RunButton } from './RunButton';
 import { LiveTailControls } from './useLiveTailControls';
-import { setDashboardQueriesToUpdateOnLoad } from '../dashboard/state/reducers';
 import { cancelQueries, clearQueries, runQueries } from './state/query';
-
-const { ButtonSelect } = LegacyForms;
+import ReturnToDashboardButton from './ReturnToDashboardButton';
 
 const getStyles = memoizeOne(() => {
   return {
@@ -56,8 +51,6 @@ interface StateProps {
   hasLiveOption: boolean;
   isLive: boolean;
   isPaused: boolean;
-  originPanelId?: number | null;
-  queries: DataQuery[];
   datasourceLoading?: boolean | null;
   containerWidth: number;
   datasourceName?: string;
@@ -72,8 +65,6 @@ interface DispatchProps {
   split: typeof splitOpen;
   syncTimes: typeof syncTimes;
   changeRefreshInterval: typeof changeRefreshInterval;
-  updateLocation: typeof updateLocation;
-  setDashboardQueriesToUpdateOnLoad: typeof setDashboardQueriesToUpdateOnLoad;
   onChangeTimeZone: typeof updateTimeZoneForSession;
 }
 
@@ -106,40 +97,6 @@ export class UnConnectedExploreToolbar extends PureComponent<Props> {
     syncTimes(exploreId);
   };
 
-  returnToPanel = async ({ withChanges = false } = {}) => {
-    const { originPanelId, queries } = this.props;
-
-    const dashboardSrv = getDashboardSrv();
-    const dash = dashboardSrv.getCurrent();
-    const titleSlug = kbn.slugifyForUrl(dash.title);
-
-    if (withChanges) {
-      this.props.setDashboardQueriesToUpdateOnLoad({
-        panelId: originPanelId!,
-        queries: this.cleanQueries(queries),
-      });
-    }
-
-    const query: any = {};
-
-    if (withChanges || dash.panelInEdit) {
-      query.editPanel = originPanelId;
-    } else if (dash.panelInView) {
-      query.viewPanel = originPanelId;
-    }
-
-    this.props.updateLocation({ path: `/d/${dash.uid}/:${titleSlug}`, query });
-  };
-
-  // Remove explore specific parameters from queries
-  private cleanQueries(queries: DataQuery[]) {
-    return queries.map((query: DataQuery & { context?: string }) => {
-      delete query.context;
-      delete query.key;
-      return query;
-    });
-  }
-
   render() {
     const {
       datasourceMissing,
@@ -156,18 +113,11 @@ export class UnConnectedExploreToolbar extends PureComponent<Props> {
       hasLiveOption,
       isLive,
       isPaused,
-      originPanelId,
       containerWidth,
       onChangeTimeZone,
     } = this.props;
 
     const styles = getStyles();
-    const originDashboardIsEditable = originPanelId && Number.isInteger(originPanelId);
-    const panelReturnClasses = classNames('btn', 'navbar-button', {
-      'btn--radius-right-0': originDashboardIsEditable,
-      'navbar-button navbar-button--border-right-0': originDashboardIsEditable,
-    });
-
     const showSmallDataSourcePicker = (splitted ? containerWidth < 700 : containerWidth < 800) || false;
     const showSmallTimePicker = splitted || containerWidth < 1210;
 
@@ -213,25 +163,7 @@ export class UnConnectedExploreToolbar extends PureComponent<Props> {
                 </div>
               </div>
             ) : null}
-
-            {originPanelId && Number.isInteger(originPanelId) && !splitted && (
-              <div className="explore-toolbar-content-item">
-                <Tooltip content={'Return to panel'} placement="bottom">
-                  <button className={panelReturnClasses} onClick={() => this.returnToPanel()}>
-                    <Icon name="arrow-left" />
-                  </button>
-                </Tooltip>
-                {originDashboardIsEditable && (
-                  <ButtonSelect
-                    className="navbar-button--attached btn--radius-left-0$"
-                    options={[{ label: 'Return to panel with changes', value: '' }]}
-                    onChange={() => this.returnToPanel({ withChanges: true })}
-                    maxMenuHeight={380}
-                  />
-                )}
-              </div>
-            )}
-
+            <ReturnToDashboardButton exploreId={exploreId} />
             {exploreId === 'left' && !splitted ? (
               <div className="explore-toolbar-content-item explore-icon-align">
                 <ResponsiveButton
@@ -330,8 +262,6 @@ const mapStateToProps = (state: StoreState, { exploreId }: OwnProps): StateProps
     loading,
     isLive,
     isPaused,
-    originPanelId,
-    queries,
     containerWidth,
   } = exploreItem;
 
@@ -348,8 +278,6 @@ const mapStateToProps = (state: StoreState, { exploreId }: OwnProps): StateProps
     hasLiveOption,
     isLive,
     isPaused,
-    originPanelId,
-    queries,
     syncedTimes,
     containerWidth,
   };
@@ -357,7 +285,6 @@ const mapStateToProps = (state: StoreState, { exploreId }: OwnProps): StateProps
 
 const mapDispatchToProps: DispatchProps = {
   changeDatasource,
-  updateLocation,
   changeRefreshInterval,
   clearAll: clearQueries,
   cancelQueries,
@@ -365,7 +292,6 @@ const mapDispatchToProps: DispatchProps = {
   closeSplit: splitClose,
   split: splitOpen,
   syncTimes,
-  setDashboardQueriesToUpdateOnLoad,
   onChangeTimeZone: updateTimeZoneForSession,
 };
 

public/app/features/explore/ReturnToDashboardButton.test.tsx

@@ -0,0 +1,41 @@
+import React, { ComponentProps } from 'react';
+import { render, screen, fireEvent } from '@testing-library/react';
+import { UnconnectedReturnToDashboardButton as ReturnToDashboardButton } from './ReturnToDashboardButton';
+import { ExploreId } from 'app/types/explore';
+
+const createProps = (propsOverride?: Partial<ComponentProps<typeof ReturnToDashboardButton>>) => {
+  const defaultProps = {
+    originPanelId: 1,
+    splitted: false,
+    exploreId: ExploreId.left,
+    queries: [],
+    updateLocation: jest.fn(),
+    setDashboardQueriesToUpdateOnLoad: jest.fn(),
+  };
+
+  return Object.assign(defaultProps, propsOverride) as ComponentProps<typeof ReturnToDashboardButton>;
+};
+
+describe('ReturnToDashboardButton', () => {
+  it('should render 2 buttons if originPanelId is provided', () => {
+    render(<ReturnToDashboardButton {...createProps()} />);
+    expect(screen.getAllByTestId(/returnButton/i)).toHaveLength(2);
+  });
+  it('should not render any button if originPanelId is not provided', () => {
+    render(<ReturnToDashboardButton {...createProps({ originPanelId: undefined })} />);
+    expect(screen.queryByTestId(/returnButton/i)).toBeNull();
+  });
+  it('should not render any button if split view', () => {
+    render(<ReturnToDashboardButton {...createProps({ splitted: true })} />);
+    expect(screen.queryByTestId(/returnButton/i)).toBeNull();
+  });
+  it('should show option to return to dashboard with changes', () => {
+    render(<ReturnToDashboardButton {...createProps()} />);
+    const returnWithChangesButton = screen.getByTestId('returnButtonWithChanges');
+    const selectButton = returnWithChangesButton.querySelector('.select-button');
+    if (selectButton) {
+      fireEvent.click(selectButton);
+    }
+    expect(screen.getAllByText('Return to panel with changes')).toHaveLength(1);
+  });
+});

public/app/features/explore/ReturnToDashboardButton.tsx

@@ -0,0 +1,118 @@
+import React, { FC } from 'react';
+import classNames from 'classnames';
+import { connect } from 'react-redux';
+import { hot } from 'react-hot-loader';
+import { Icon, Tooltip, LegacyForms } from '@grafana/ui';
+import { DataQuery } from '@grafana/data';
+
+import kbn from '../../core/utils/kbn';
+import { getDashboardSrv } from '../dashboard/services/DashboardSrv';
+import { StoreState } from 'app/types';
+import { ExploreId } from 'app/types/explore';
+import { updateLocation } from 'app/core/actions';
+import { setDashboardQueriesToUpdateOnLoad } from '../dashboard/state/reducers';
+
+const { ButtonSelect } = LegacyForms;
+
+interface Props {
+  exploreId: ExploreId;
+  splitted: boolean;
+  queries: DataQuery[];
+  originPanelId?: number | null;
+  updateLocation: typeof updateLocation;
+  setDashboardQueriesToUpdateOnLoad: typeof setDashboardQueriesToUpdateOnLoad;
+}
+
+export const UnconnectedReturnToDashboardButton: FC<Props> = ({
+  originPanelId,
+  updateLocation,
+  setDashboardQueriesToUpdateOnLoad,
+  queries,
+  splitted,
+}) => {
+  const withOriginId = originPanelId && Number.isInteger(originPanelId);
+
+  // If in split mode, or no origin id, escape early and return null
+  if (splitted || !withOriginId) {
+    return null;
+  }
+
+  const panelReturnClasses = classNames('btn', 'navbar-button', {
+    'btn--radius-right-0': withOriginId,
+    'navbar-button navbar-button--border-right-0': withOriginId,
+  });
+
+  const cleanQueries = (queries: DataQuery[]) => {
+    return queries.map((query: DataQuery & { context?: string }) => {
+      delete query.context;
+      delete query.key;
+      return query;
+    });
+  };
+
+  const returnToPanel = async ({ withChanges = false } = {}) => {
+    const dashboardSrv = getDashboardSrv();
+    const dash = dashboardSrv.getCurrent();
+    const titleSlug = kbn.slugifyForUrl(dash.title);
+
+    if (withChanges) {
+      setDashboardQueriesToUpdateOnLoad({
+        panelId: originPanelId!,
+        queries: cleanQueries(queries),
+      });
+    }
+
+    const query: any = {};
+
+    if (withChanges || dash.panelInEdit) {
+      query.editPanel = originPanelId;
+    } else if (dash.panelInView) {
+      query.viewPanel = originPanelId;
+    }
+
+    updateLocation({ path: `/d/${dash.uid}/:${titleSlug}`, query });
+  };
+
+  return (
+    <div className="explore-toolbar-content-item">
+      <Tooltip content={'Return to panel'} placement="bottom">
+        <button
+          data-testid="returnButton"
+          title={'Return to panel'}
+          className={panelReturnClasses}
+          onClick={() => returnToPanel()}
+        >
+          <Icon name="arrow-left" />
+        </button>
+      </Tooltip>
+      <div data-testid="returnButtonWithChanges">
+        <ButtonSelect
+          className="navbar-button--attached btn--radius-left-0$"
+          options={[{ label: 'Return to panel with changes', value: '' }]}
+          onChange={() => returnToPanel({ withChanges: true })}
+          maxMenuHeight={380}
+        />
+      </div>
+    </div>
+  );
+};
+
+function mapStateToProps(state: StoreState, { exploreId }: { exploreId: ExploreId }) {
+  const explore = state.explore;
+  const splitted = state.explore.split;
+  const { datasourceInstance, queries, originPanelId } = explore[exploreId];
+
+  return {
+    exploreId,
+    datasourceInstance,
+    queries,
+    originPanelId,
+    splitted,
+  };
+}
+
+const mapDispatchToProps = {
+  updateLocation,
+  setDashboardQueriesToUpdateOnLoad,
+};
+export default hot(module)(connect(mapStateToProps, mapDispatchToProps)(UnconnectedReturnToDashboardButton));

public/app/features/explore/state/explorePane.ts

@@ -1,5 +1,5 @@
 import { AnyAction } from 'redux';
-
+import { isEqual } from 'lodash';
 import { getQueryKeys } from 'app/core/utils/explore';
 import { ExploreId, ExploreItemState } from 'app/types/explore';
 import { queryReducer } from './query';
@@ -258,8 +258,14 @@ export const paneReducer = (state: ExploreItemState = makeExplorePaneState(), ac
   }
 
   if (highlightLogsExpressionAction.match(action)) {
-    const { expressions } = action.payload;
-    return { ...state, logsHighlighterExpressions: expressions };
+    const { expressions: newExpressions } = action.payload;
+    const { logsHighlighterExpressions: currentExpressions } = state;
+
+    return {
+      ...state,
+      // Prevents re-renders. As logsHighlighterExpressions [] comes from datasource, we cannot control if it returns new array or not.
+      logsHighlighterExpressions: isEqual(newExpressions, currentExpressions) ? currentExpressions : newExpressions,
+    };
   }
 
   if (changeDedupStrategyAction.match(action)) {

public/app/features/explore/state/query.ts

@@ -477,7 +477,6 @@ export const queryReducer = (state: ExploreItemState, action: AnyAction): Explor
     return {
       ...state,
       queries: nextQueries,
-      queryKeys: getQueryKeys(nextQueries, state.datasourceInstance),
     };
   }