React: refactor away from unsafe lifecycle methods (#21291)

* refactor aliasBy and PopoverCtrl components

* refactor Aggregations component

* refactor MetricSelect component

* refactor UserProvider

* popoverCtr: remove redundant logic

* simplified the MetricsSelect a bit.

* skipping testing of internal component logic.

* changed to componentWillMount.

* changed elapsed time to a functional component.

* rewrote the tests.

* fixed missing test title.

* fixed a tiny issue with elapsed time.

* rename of field.

Co-authored-by: Marcus Andersson <marcus.andersson@grafana.com>

packages/grafana-ui/src/components/Tooltip/PopoverController.tsx

@@ -48,63 +48,27 @@ interface Props {
 }
 
 interface State {
-  placement: PopperJS.Placement;
   show: boolean;
 }
 
 class PopoverController extends React.Component<Props, State> {
   private hideTimeout: any;
-
+  state = { show: false };
-  constructor(props: Props) {
-    super(props);
-
-    this.state = {
-      placement: this.props.placement || 'auto',
-      show: false,
-    };
-  }
-
-  UNSAFE_componentWillReceiveProps(nextProps: Props) {
-    if (nextProps.placement && nextProps.placement !== this.state.placement) {
-      this.setState((prevState: State) => {
-        return {
-          ...prevState,
-          placement: nextProps.placement || 'auto',
-        };
-      });
-    }
-  }
 
   showPopper = () => {
-    if (this.hideTimeout) {
     clearTimeout(this.hideTimeout);
-    }
+    this.setState({ show: true });
-
-    this.setState(prevState => ({
-      ...prevState,
-      show: true,
-    }));
   };
 
   hidePopper = () => {
-    if (this.props.hideAfter !== 0) {
     this.hideTimeout = setTimeout(() => {
-        this.setState(prevState => ({
+      this.setState({ show: false });
-          ...prevState,
-          show: false,
-        }));
     }, this.props.hideAfter);
-      return;
-    }
-    this.setState(prevState => ({
-      ...prevState,
-      show: false,
-    }));
   };
 
   render() {
-    const { children, content } = this.props;
+    const { children, content, placement = 'auto' } = this.props;
-    const { show, placement } = this.state;
+    const { show } = this.state;
 
     return children(this.showPopper, this.hidePopper, {
       show,

public/app/core/components/Select/MetricSelect.test.tsx

@@ -0,0 +1,48 @@
+import React from 'react';
+import { shallow } from 'enzyme';
+import { MetricSelect } from './MetricSelect';
+import { LegacyForms } from '@grafana/ui';
+const { Select } = LegacyForms;
+
+describe('MetricSelect', () => {
+  describe('When receive props', () => {
+    it('should pass correct set of props to Select component', () => {
+      const props: any = {
+        placeholder: 'Select Reducer',
+        className: 'width-15',
+        options: [],
+        variables: [],
+      };
+      const wrapper = shallow(<MetricSelect {...props} />);
+      expect(wrapper.find(Select).props()).toMatchObject({
+        className: 'width-15',
+        isMulti: false,
+        isClearable: false,
+        backspaceRemovesValue: false,
+        isSearchable: true,
+        maxMenuHeight: 500,
+        placeholder: 'Select Reducer',
+      });
+    });
+    it('should pass callbacks correctly to the Select component', () => {
+      const spyOnChange = jest.fn();
+      const props: any = {
+        onChange: spyOnChange,
+        options: [],
+        variables: [],
+      };
+      const wrapper = shallow(<MetricSelect {...props} />);
+      wrapper
+        .find(Select)
+        .props()
+        .onChange({ value: 'foo' });
+      expect(
+        wrapper
+          .find(Select)
+          .props()
+          .noOptionsMessage()
+      ).toEqual('No options found');
+      expect(spyOnChange).toHaveBeenCalledWith('foo');
+    });
+  });
+});

public/app/core/components/Select/MetricSelect.tsx

@@ -1,4 +1,4 @@
-import React from 'react';
+import React, { useMemo, useCallback, FC } from 'react';
 import _ from 'lodash';
 
 import { LegacyForms } from '@grafana/ui';
@@ -16,75 +16,51 @@ export interface Props {
   variables?: Variable[];
 }
 
-interface State {
+export const MetricSelect: FC<Props> = props => {
-  options: Array<SelectableValue<string>>;
+  const { value, placeholder, className, isSearchable, onChange } = props;
-}
+  const options = useSelectOptions(props);
+  const selected = useSelectedOption(options, value);
+  const onChangeValue = useCallback((selectable: SelectableValue<string>) => onChange(selectable.value), [onChange]);
 
-export class MetricSelect extends React.Component<Props, State> {
-  static defaultProps: Partial<Props> = {
-    variables: [],
-    options: [],
-    isSearchable: true,
-  };
-
-  constructor(props: Props) {
-    super(props);
-    this.state = { options: [] };
-  }
-
-  componentDidMount() {
-    this.setState({ options: this.buildOptions(this.props) });
-  }
-
-  UNSAFE_componentWillReceiveProps(nextProps: Props) {
-    if (nextProps.options.length > 0 || nextProps.variables?.length) {
-      this.setState({ options: this.buildOptions(nextProps) });
-    }
-  }
-
-  shouldComponentUpdate(nextProps: Props) {
-    const nextOptions = this.buildOptions(nextProps);
-    return nextProps.value !== this.props.value || !_.isEqual(nextOptions, this.state.options);
-  }
-
-  buildOptions({ variables = [], options }: Props) {
-    return variables.length > 0 ? [this.getVariablesGroup(), ...options] : options;
-  }
-
-  getVariablesGroup() {
-    return {
-      label: 'Template Variables',
-      options: this.props.variables?.map(v => ({
-        label: `$${v.name}`,
-        value: `$${v.name}`,
-      })),
-    };
-  }
-
-  getSelectedOption() {
-    const { options } = this.state;
-    const allOptions = options.every(o => o.options) ? _.flatten(options.map(o => o.options)) : options;
-    return allOptions.find(option => option.value === this.props.value);
-  }
-
-  render() {
-    const { placeholder, className, isSearchable, onChange } = this.props;
-    const { options } = this.state;
-    const selectedOption = this.getSelectedOption();
   return (
     <Select
       className={className}
       isMulti={false}
       isClearable={false}
       backspaceRemovesValue={false}
-        onChange={item => onChange(item.value ?? '')}
+      onChange={onChangeValue}
       options={options}
       isSearchable={isSearchable}
       maxMenuHeight={500}
       placeholder={placeholder}
       noOptionsMessage={() => 'No options found'}
-        value={selectedOption}
+      value={selected}
     />
   );
+};
+
+const useSelectOptions = ({ variables = [], options }: Props): Array<SelectableValue<string>> => {
+  return useMemo(() => {
+    if (!Array.isArray(variables) || variables.length === 0) {
+      return options;
     }
-}
+
+    return [
+      {
+        label: 'Template Variables',
+        options: variables.map(({ name }) => ({
+          label: `$${name}`,
+          value: `$${name}`,
+        })),
+      },
+      ...options,
+    ];
+  }, [variables, options]);
+};
+
+const useSelectedOption = (options: Array<SelectableValue<string>>, value: string): SelectableValue<string> => {
+  return useMemo(() => {
+    const allOptions = options.every(o => o.options) ? _.flatten(options.map(o => o.options)) : options;
+    return allOptions.find(option => option.value === value);
+  }, [options, value]);
+};

public/app/features/explore/ElapsedTime.tsx

@@ -1,76 +1,22 @@
-import React, { PureComponent } from 'react';
+import React, { FC, useState, useEffect } from 'react';
-import { toDuration } from '@grafana/data';
+import { useInterval } from 'react-use';
+import { Time, TimeProps } from './Time';
 
 const INTERVAL = 150;
 
-export interface Props {
+export interface ElapsedTimeProps extends Omit<TimeProps, 'timeInMs'> {
-  time?: number;
   // Use this to reset the timer. Any value is allowed just need to be !== from the previous.
   // Keep in mind things like [] !== [] or {} !== {}.
   resetKey?: any;
-  className?: string;
-  humanize?: boolean;
 }
 
-export interface State {
+export const ElapsedTime: FC<ElapsedTimeProps> = ({ resetKey, humanize, className }) => {
-  elapsed: number;
+  const [elapsed, setElapsed] = useState(0); // the current value of elapsed
-}
 
-/**
+  // hook that will schedule a interval and then update the elapsed value on every tick.
- * Shows an incremental time ticker of elapsed time from some event.
+  useInterval(() => setElapsed(elapsed + INTERVAL), INTERVAL);
- */
+  // this effect will only be run when resetKey changes. This will reset the elapsed to 0.
-export default class ElapsedTime extends PureComponent<Props, State> {
+  useEffect(() => setElapsed(0), [resetKey]);
-  offset: number;
-  timer: number;
 
-  state = {
+  return <Time timeInMs={elapsed} className={className} humanize={humanize} />;
-    elapsed: 0,
+};
-  };
-
-  start() {
-    this.offset = Date.now();
-    this.timer = window.setInterval(this.tick, INTERVAL);
-  }
-
-  tick = () => {
-    const jetzt = Date.now();
-    const elapsed = jetzt - this.offset;
-    this.setState({ elapsed });
-  };
-
-  UNSAFE_componentWillReceiveProps(nextProps: Props) {
-    if (nextProps.time) {
-      clearInterval(this.timer);
-    } else if (this.props.time) {
-      this.start();
-    }
-
-    if (nextProps.resetKey !== this.props.resetKey) {
-      clearInterval(this.timer);
-      this.start();
-    }
-  }
-
-  componentDidMount() {
-    this.start();
-  }
-
-  componentWillUnmount() {
-    clearInterval(this.timer);
-  }
-
-  render() {
-    const { elapsed } = this.state;
-    const { className, time, humanize } = this.props;
-    const value = (time || elapsed) / 1000;
-    let displayValue = `${value.toFixed(1)}s`;
-    if (humanize) {
-      const duration = toDuration(elapsed);
-      const hours = duration.hours();
-      const minutes = duration.minutes();
-      const seconds = duration.seconds();
-      displayValue = hours ? `${hours}h ${minutes}m ${seconds}s` : minutes ? ` ${minutes}m ${seconds}s` : `${seconds}s`;
-    }
-    return <span className={`elapsed-time ${className}`}>{displayValue}</span>;
-  }
-}

public/app/features/explore/LiveLogs.tsx

@@ -5,7 +5,7 @@ import tinycolor from 'tinycolor2';
 import { Themeable, withTheme, getLogRowStyles, Icon } from '@grafana/ui';
 import { GrafanaTheme, LogRowModel, TimeZone, dateTimeFormat } from '@grafana/data';
 
-import ElapsedTime from './ElapsedTime';
+import { ElapsedTime } from './ElapsedTime';
 
 const getStyles = (theme: GrafanaTheme) => ({
   logsRowsLive: css`

public/app/features/explore/QueryStatus.tsx

@@ -1,6 +1,6 @@
 import React, { PureComponent } from 'react';
 
-import ElapsedTime from './ElapsedTime';
+import { ElapsedTime } from './ElapsedTime';
 import { PanelData, LoadingState } from '@grafana/data';
 
 function formatLatency(value: number) {

public/app/features/explore/Time.tsx

@@ -0,0 +1,35 @@
+import React, { FC } from 'react';
+import { toDuration } from '@grafana/data';
+
+export interface TimeProps {
+  timeInMs: number;
+  className?: string;
+  humanize?: boolean;
+}
+
+export const Time: FC<TimeProps> = ({ timeInMs, className, humanize }) => {
+  return <span className={`elapsed-time ${className}`}>{formatTime(timeInMs, humanize)}</span>;
+};
+
+const formatTime = (timeInMs: number, humanize = false): string => {
+  const inSeconds = timeInMs / 1000;
+
+  if (!humanize) {
+    return `${inSeconds.toFixed(1)}s`;
+  }
+
+  const duration = toDuration(inSeconds, 'seconds');
+  const hours = duration.hours();
+  const minutes = duration.minutes();
+  const seconds = duration.seconds();
+
+  if (hours) {
+    return `${hours}h ${minutes}m ${seconds}s`;
+  }
+
+  if (minutes) {
+    return `${minutes}m ${seconds}s`;
+  }
+
+  return `${seconds}s`;
+};

public/app/plugins/datasource/stackdriver/components/Aggregations.test.tsx

@@ -1,7 +1,8 @@
 import React from 'react';
 import renderer from 'react-test-renderer';
-import { Aggregations, Props } from './Aggregations';
 import { shallow } from 'enzyme';
+import { Segment } from '@grafana/ui';
+import { Aggregations, Props } from './Aggregations';
 import { ValueTypes, MetricKind } from '../constants';
 import { TemplateSrvStub } from 'test/specs/helpers';
 
@@ -20,39 +21,49 @@ const props: Props = {
 };
 
 describe('Aggregations', () => {
-  let wrapper: any;
   it('renders correctly', () => {
     const tree = renderer.create(<Aggregations {...props} />).toJSON();
     expect(tree).toMatchSnapshot();
   });
 
   describe('options', () => {
-    describe('when DOUBLE and DELTA is passed as props', () => {
+    describe('when DOUBLE and GAUGE is passed as props', () => {
-      beforeEach(() => {
+      const nextProps = {
-        const newProps = { ...props, metricDescriptor: { valueType: ValueTypes.DOUBLE, metricKind: MetricKind.GAUGE } };
+        ...props,
-        wrapper = shallow(<Aggregations {...newProps} />);
+        metricDescriptor: {
-      });
+          valueType: ValueTypes.DOUBLE,
-      it('', () => {
+          metricKind: MetricKind.GAUGE,
-        const options = wrapper.state().aggOptions;
+        },
-        expect(options.length).toEqual(11);
+      };
-        expect(options.map((o: any) => o.value)).toEqual(
+
+      it('should not have the reduce values', () => {
+        const wrapper = shallow(<Aggregations {...nextProps} />);
+        const { options } = wrapper.find(Segment).props() as any;
+        const [, aggGroup] = options;
+
+        expect(aggGroup.options.length).toEqual(11);
+        expect(aggGroup.options.map((o: any) => o.value)).toEqual(
           expect.not.arrayContaining(['REDUCE_COUNT_TRUE', 'REDUCE_COUNT_FALSE'])
         );
       });
     });
 
     describe('when MONEY and CUMULATIVE is passed as props', () => {
-      beforeEach(() => {
+      const nextProps = {
-        const newProps = {
         ...props,
-          metricDescriptor: { valueType: ValueTypes.MONEY, metricKind: MetricKind.CUMULATIVE },
+        metricDescriptor: {
+          valueType: ValueTypes.MONEY,
+          metricKind: MetricKind.CUMULATIVE,
+        },
       };
-        wrapper = shallow(<Aggregations {...newProps} />);
+
-      });
+      it('should have the reduce values', () => {
-      it('', () => {
+        const wrapper = shallow(<Aggregations {...nextProps} />);
-        const options = wrapper.state().aggOptions;
+        const { options } = wrapper.find(Segment).props() as any;
-        expect(options.length).toEqual(10);
+        const [, aggGroup] = options;
-        expect(options.map((o: any) => o.value)).toEqual(expect.arrayContaining(['REDUCE_NONE']));
+
+        expect(aggGroup.options.length).toEqual(10);
+        expect(aggGroup.options.map((o: any) => o.value)).toEqual(expect.arrayContaining(['REDUCE_NONE']));
       });
     });
   });

public/app/plugins/datasource/stackdriver/components/Aggregations.tsx

@@ -1,4 +1,4 @@
-import React from 'react';
+import React, { FC, useState, useMemo } from 'react';
 import _ from 'lodash';
 
 import { SelectableValue } from '@grafana/data';
@@ -18,60 +18,22 @@ export interface Props {
   templateVariableOptions: Array<SelectableValue<string>>;
 }
 
-export interface State {
+export const Aggregations: FC<Props> = props => {
-  aggOptions: any[];
+  const [displayAdvancedOptions, setDisplayAdvancedOptions] = useState(false);
-  displayAdvancedOptions: boolean;
+  const aggOptions = useAggregationOptionsByMetric(props);
-}
+  const selected = useSelectedFromOptions(aggOptions, props);
-
-export class Aggregations extends React.Component<Props, State> {
-  state: State = {
-    aggOptions: [],
-    displayAdvancedOptions: false,
-  };
-
-  componentDidMount() {
-    this.setAggOptions(this.props);
-  }
-
-  UNSAFE_componentWillReceiveProps(nextProps: Props) {
-    this.setAggOptions(nextProps);
-  }
-
-  setAggOptions({ metricDescriptor }: Props) {
-    let aggOptions: any[] = [];
-    if (metricDescriptor) {
-      aggOptions = getAggregationOptionsByMetric(
-        metricDescriptor.valueType as ValueTypes,
-        metricDescriptor.metricKind as MetricKind
-      ).map(a => ({
-        ...a,
-        label: a.text,
-      }));
-    }
-    this.setState({ aggOptions });
-  }
-
-  onToggleDisplayAdvanced = () => {
-    this.setState(state => ({
-      displayAdvancedOptions: !state.displayAdvancedOptions,
-    }));
-  };
-
-  render() {
-    const { displayAdvancedOptions, aggOptions } = this.state;
-    const { templateVariableOptions, onChange, crossSeriesReducer } = this.props;
 
   return (
     <>
       <div className="gf-form-inline">
         <label className="gf-form-label query-keyword width-9">Aggregation</label>
         <Segment
-            onChange={({ value }) => onChange(value)}
+          onChange={({ value }) => props.onChange(value)}
-            value={[...aggOptions, ...templateVariableOptions].find(s => s.value === crossSeriesReducer)}
+          value={selected}
           options={[
             {
               label: 'Template Variables',
-                options: templateVariableOptions,
+              options: props.templateVariableOptions,
             },
             {
               label: 'Aggregations',
@@ -83,7 +45,7 @@ export class Aggregations extends React.Component<Props, State> {
         ></Segment>
         <div className="gf-form gf-form--grow">
           <label className="gf-form-label gf-form-label--grow">
-              <a onClick={this.onToggleDisplayAdvanced}>
+            <a onClick={() => setDisplayAdvancedOptions(!displayAdvancedOptions)}>
               <>
                 <Icon name={displayAdvancedOptions ? 'angle-down' : 'angle-right'} /> Advanced Options
               </>
@@ -91,8 +53,30 @@ export class Aggregations extends React.Component<Props, State> {
           </label>
         </div>
       </div>
-        {this.props.children && this.props.children(this.state.displayAdvancedOptions)}
+      {props.children(displayAdvancedOptions)}
     </>
   );
+};
+
+const useAggregationOptionsByMetric = ({ metricDescriptor }: Props): Array<SelectableValue<string>> => {
+  return useMemo(() => {
+    if (!metricDescriptor) {
+      return [];
     }
-}
+
+    return getAggregationOptionsByMetric(
+      metricDescriptor.valueType as ValueTypes,
+      metricDescriptor.metricKind as MetricKind
+    ).map(a => ({
+      ...a,
+      label: a.text,
+    }));
+  }, [metricDescriptor?.metricKind, metricDescriptor?.valueType]);
+};
+
+const useSelectedFromOptions = (aggOptions: Array<SelectableValue<string>>, props: Props) => {
+  return useMemo(() => {
+    const allOptions = [...aggOptions, ...props.templateVariableOptions];
+    return allOptions.find(s => s.value === props.crossSeriesReducer);
+  }, [aggOptions, props.crossSeriesReducer, props.templateVariableOptions]);
+};