Query components unsafe lifecycle methods (#21163)

* Chore: refactor query components unsafe lifecycle methods

* Chore: remove redundant defaults

* Chore: check expression exist while filter targets in query

* Chore: fix broken test

* Chore: adding query filed tests

* pass normalized query to QueryFieldsEditor

Signed-off-by: Boyko Lalov <boiskila@gmail.com>

* Remove introduced strictNullCheck errors and correctly solve merged conficts

* Improve redability and fix strictNullChecks

Co-authored-by: Ivana <ivana.huckova@gmail.com>

packages/grafana-ui/src/components/QueryField/QueryField.test.tsx

@@ -49,4 +49,36 @@ describe('<QueryField />', () => {
     expect(onBlur.mock.calls.length).toBe(1);
     expect(onRun.mock.calls.length).toBe(0);
   });
+  describe('syntaxLoaded', () => {
+    it('should re-render the editor after syntax has fully loaded', () => {
+      const wrapper: any = shallow(<QueryField query="my query" portalOrigin="mock-origin" />);
+      const spyOnChange = jest.spyOn(wrapper.instance(), 'onChange').mockImplementation(jest.fn());
+      wrapper.instance().editor = { insertText: () => ({ deleteBackward: () => ({ value: 'fooo' }) }) };
+      wrapper.setProps({ syntaxLoaded: true });
+      expect(spyOnChange).toHaveBeenCalledWith('fooo', true);
+    });
+    it('should not re-render the editor if syntax is already loaded', () => {
+      const wrapper: any = shallow(<QueryField query="my query" portalOrigin="mock-origin" />);
+      const spyOnChange = jest.spyOn(wrapper.instance(), 'onChange').mockImplementation(jest.fn());
+      wrapper.setProps({ syntaxLoaded: true });
+      wrapper.instance().editor = {};
+      wrapper.setProps({ syntaxLoaded: true });
+      expect(spyOnChange).not.toBeCalled();
+    });
+    it('should not re-render the editor if editor itself is not defined', () => {
+      const wrapper: any = shallow(<QueryField query="my query" portalOrigin="mock-origin" />);
+      const spyOnChange = jest.spyOn(wrapper.instance(), 'onChange').mockImplementation(jest.fn());
+      wrapper.setProps({ syntaxLoaded: true });
+      expect(wrapper.instance().editor).toBeFalsy();
+      expect(spyOnChange).not.toBeCalled();
+    });
+    it('should not re-render the editor twice once syntax is fully lodaded', () => {
+      const wrapper: any = shallow(<QueryField query="my query" portalOrigin="mock-origin" />);
+      const spyOnChange = jest.spyOn(wrapper.instance(), 'onChange').mockImplementation(jest.fn());
+      wrapper.instance().editor = { insertText: () => ({ deleteBackward: () => ({ value: 'fooo' }) }) };
+      wrapper.setProps({ syntaxLoaded: true });
+      wrapper.setProps({ syntaxLoaded: true });
+      expect(spyOnChange).toBeCalledTimes(1);
+    });
+  });
 });

packages/grafana-ui/src/components/QueryField/QueryField.tsx

@@ -95,7 +95,13 @@ export class QueryField extends React.PureComponent<QueryFieldProps, QueryFieldS
   }
 
   componentDidUpdate(prevProps: QueryFieldProps, prevState: QueryFieldState) {
-    const { query, syntax } = this.props;
+    const { query, syntax, syntaxLoaded } = this.props;
+
+    if (!prevProps.syntaxLoaded && syntaxLoaded && this.editor) {
+      // Need a bogus edit to re-render the editor after syntax has fully loaded
+      const editor = this.editor.insertText(' ').deleteBackward(1);
+      this.onChange(editor.value, true);
+    }
     const { value } = this.state;
 
     // Handle two way binging between local state and outside prop.
@@ -108,18 +114,6 @@ export class QueryField extends React.PureComponent<QueryFieldProps, QueryFieldS
     }
   }
 
-  UNSAFE_componentWillReceiveProps(nextProps: QueryFieldProps) {
-    if (nextProps.syntaxLoaded && !this.props.syntaxLoaded) {
-      if (!this.editor) {
-        return;
-      }
-
-      // Need a bogus edit to re-render the editor after syntax has fully loaded
-      const editor = this.editor.insertText(' ').deleteBackward(1);
-      this.onChange(editor.value, true);
-    }
-  }
-
   /**
    * Update local state, propagate change upstream and optionally run the query afterwards.
    */

public/app/plugins/datasource/cloudwatch/components/QueryEditor.test.tsx

@@ -5,7 +5,7 @@ import { act } from 'react-dom/test-utils';
 import { DataSourceInstanceSettings } from '@grafana/data';
 import { TemplateSrv } from 'app/features/templating/template_srv';
 import { CustomVariable } from 'app/features/templating/all';
-import { Props, QueryEditor } from './QueryEditor';
+import { Props, QueryEditor, normalizeQuery } from './QueryEditor';
 import CloudWatchDatasource from '../datasource';
 
 const setup = () => {
@@ -86,26 +86,19 @@ describe('QueryEditor', () => {
       });
     });
 
-    it('should init props correctly', async () => {
-      // @ts-ignore strict null error TS2345: Argument of type '() => Promise<void>' is not assignable to parameter of type '() => void | undefined'.
-      await act(async () => {
-        const props = setup();
-        props.query.namespace = (null as unknown) as string;
-        props.query.metricName = (null as unknown) as string;
-        props.query.expression = (null as unknown) as string;
-        props.query.dimensions = (null as unknown) as { [key: string]: string | string[] };
-        props.query.region = (null as unknown) as string;
-        props.query.statistics = (null as unknown) as string[];
-        const wrapper = mount(<QueryEditor {...props} />);
-        const {
-          query: { namespace, region, metricName, dimensions, statistics, expression },
-        } = wrapper.props();
-        expect(namespace).toEqual('');
-        expect(metricName).toEqual('');
-        expect(expression).toEqual('');
-        expect(region).toEqual('default');
-        expect(statistics).toEqual(['Average']);
-        expect(dimensions).toEqual({});
+    it('should normalize query with default values', () => {
+      expect(normalizeQuery({ refId: '42' } as any)).toEqual({
+        namespace: '',
+        metricName: '',
+        expression: '',
+        dimensions: {},
+        region: 'default',
+        id: '',
+        alias: '',
+        statistics: ['Average'],
+        matchExact: true,
+        period: '',
+        refId: '42',
       });
     });
   });

public/app/plugins/datasource/cloudwatch/components/QueryEditor.tsx

@@ -1,6 +1,7 @@
 import React, { PureComponent, ChangeEvent } from 'react';
 import { ExploreQueryFieldProps } from '@grafana/data';
 import { Input, ValidationEvents, EventsWithValidation, Switch } from '@grafana/ui';
+import isEmpty from 'lodash/isEmpty';
 import { CloudWatchQuery } from '../types';
 import CloudWatchDatasource from '../datasource';
 import { QueryField, Alias, QueryFieldsEditor } from './';
@@ -20,51 +21,36 @@ const idValidationEvents: ValidationEvents = {
   ],
 };
 
+export const normalizeQuery = ({
+  namespace,
+  metricName,
+  expression,
+  dimensions,
+  region,
+  id,
+  alias,
+  statistics,
+  period,
+  ...rest
+}: CloudWatchQuery): CloudWatchQuery => {
+  const normalizedQuery = {
+    namespace: namespace || '',
+    metricName: metricName || '',
+    expression: expression || '',
+    dimensions: dimensions || {},
+    region: region || 'default',
+    id: id || '',
+    alias: alias || '',
+    statistics: isEmpty(statistics) ? ['Average'] : statistics,
+    period: period || '',
+    ...rest,
+  };
+  return !rest.hasOwnProperty('matchExact') ? { ...normalizedQuery, matchExact: true } : normalizedQuery;
+};
+
 export class QueryEditor extends PureComponent<Props, State> {
   state: State = { showMeta: false };
 
-  static getDerivedStateFromProps(props: Props, state: State) {
-    const { query } = props;
-
-    if (!query.namespace) {
-      query.namespace = '';
-    }
-
-    if (!query.metricName) {
-      query.metricName = '';
-    }
-
-    if (!query.expression) {
-      query.expression = '';
-    }
-
-    if (!query.dimensions) {
-      query.dimensions = {};
-    }
-
-    if (!query.region) {
-      query.region = 'default';
-    }
-
-    if (!query.id) {
-      query.id = '';
-    }
-
-    if (!query.alias) {
-      query.alias = '';
-    }
-
-    if (!query.statistics || !query.statistics.length) {
-      query.statistics = ['Average'];
-    }
-
-    if (!query.hasOwnProperty('matchExact')) {
-      query.matchExact = true;
-    }
-
-    return state;
-  }
-
   onChange(query: CloudWatchQuery) {
     const { onChange, onRunQuery } = this.props;
     onChange(query);
@@ -72,12 +58,13 @@ export class QueryEditor extends PureComponent<Props, State> {
   }
 
   render() {
-    const { data, query, onRunQuery } = this.props;
+    const { data, onRunQuery } = this.props;
     const { showMeta } = this.state;
+    const query = normalizeQuery(this.props.query);
     const metaDataExist = data && Object.values(data).length && data.state === 'Done';
     return (
       <>
-        <QueryFieldsEditor {...this.props}></QueryFieldsEditor>
+        <QueryFieldsEditor {...{ ...this.props, query }}></QueryFieldsEditor>
         {query.statistics.length <= 1 && (
           <div className="gf-form-inline">
             <div className="gf-form">
@@ -92,7 +79,7 @@ export class QueryEditor extends PureComponent<Props, State> {
                     this.onChange({ ...query, id: event.target.value })
                   }
                   validationEvents={idValidationEvents}
-                  value={query.id || ''}
+                  value={query.id}
                 />
               </QueryField>
             </div>
@@ -105,7 +92,7 @@ export class QueryEditor extends PureComponent<Props, State> {
                 <Input
                   className="gf-form-input"
                   onBlur={onRunQuery}
-                  value={query.expression || ''}
+                  value={query.expression}
                   onChange={(event: ChangeEvent<HTMLInputElement>) =>
                     this.onChange({ ...query, expression: event.target.value })
                   }
@@ -119,7 +106,7 @@ export class QueryEditor extends PureComponent<Props, State> {
             <QueryField label="Period" tooltip="Minimum interval between points in seconds">
               <Input
                 className="gf-form-input width-8"
-                value={query.period || ''}
+                value={query.period}
                 placeholder="auto"
                 onBlur={onRunQuery}
                 onChange={(event: ChangeEvent<HTMLInputElement>) =>
@@ -170,7 +157,7 @@ export class QueryEditor extends PureComponent<Props, State> {
                 </tr>
               </thead>
               <tbody>
-                {data.series[0].meta.gmdMeta.map(({ ID, Expression, Period }: any) => (
+                {data?.series[0]?.meta?.gmdMeta.map(({ ID, Expression, Period }: any) => (
                   <tr key={ID}>
                     <td>{ID}</td>
                     <td>{Expression}</td>

public/app/plugins/datasource/cloudwatch/datasource.ts

@@ -68,7 +68,7 @@ export default class CloudWatchDatasource extends DataSourceApi<CloudWatchQuery,
       return (
         (item.id !== '' || item.hide !== true) &&
         ((!!item.region && !!item.namespace && !!item.metricName && !_.isEmpty(item.statistics)) ||
-          item.expression.length > 0)
+          item.expression?.length > 0)
       );
     }).map(item => {
       item.region = this.replace(this.getActualRegion(item.region), options.scopedVars, true, 'region');
@@ -112,8 +112,8 @@ export default class CloudWatchDatasource extends DataSourceApi<CloudWatchQuery,
     }
 
     const request = {
-      from: options.range.from.valueOf().toString(),
-      to: options.range.to.valueOf().toString(),
+      from: options?.range?.from.valueOf().toString(),
+      to: options?.range?.to.valueOf().toString(),
       queries: queries,
     };