[7.x] [Lens] Avoid unnecessary data fetching on dimension flyout open (…

…#82957) (#83538)

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>

x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/helpers.tsx

@@ -0,0 +1,30 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License;
+ * you may not use this file except in compliance with the Elastic License.
+ */
+
+import { useRef } from 'react';
+import useDebounce from 'react-use/lib/useDebounce';
+
+export const useDebounceWithOptions = (
+  fn: Function,
+  { skipFirstRender }: { skipFirstRender: boolean } = { skipFirstRender: false },
+  ms?: number | undefined,
+  deps?: React.DependencyList | undefined
+) => {
+  const isFirstRender = useRef(true);
+  const newDeps = [...(deps || []), isFirstRender];
+
+  return useDebounce(
+    () => {
+      if (skipFirstRender && isFirstRender.current) {
+        isFirstRender.current = false;
+        return;
+      }
+      return fn();
+    },
+    ms,
+    newDeps
+  );
+};

x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/ranges/advanced_editor.tsx

@@ -8,7 +8,6 @@ import './advanced_editor.scss';
 
 import React, { useState, MouseEventHandler } from 'react';
 import { i18n } from '@kbn/i18n';
-import useDebounce from 'react-use/lib/useDebounce';
 import {
   EuiFlexGroup,
   EuiFlexItem,
@@ -31,6 +30,7 @@ import {
   DraggableBucketContainer,
   LabelInput,
 } from '../shared_components';
+import { useDebounceWithOptions } from '../helpers';
 
 const generateId = htmlIdGenerator();
 
@@ -208,12 +208,13 @@ export const AdvancedRangeEditor = ({
 
   const lastIndex = localRanges.length - 1;
 
-  // Update locally all the time, but bounce the parents prop function
-  // to aviod too many requests
-  useDebounce(
+  // Update locally all the time, but bounce the parents prop function to aviod too many requests
+  // Avoid to trigger on first render
+  useDebounceWithOptions(
     () => {
       setRanges(localRanges.map(({ id, ...rest }) => ({ ...rest })));
     },
+    { skipFirstRender: true },
     TYPING_DEBOUNCE_TIME,
     [localRanges]
   );

x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/ranges/range_editor.tsx

@@ -6,7 +6,6 @@
 
 import React, { useEffect, useState } from 'react';
 import { i18n } from '@kbn/i18n';
-import useDebounce from 'react-use/lib/useDebounce';
 import {
   EuiButtonEmpty,
   EuiFormRow,
@@ -21,6 +20,7 @@ import { IFieldFormat } from 'src/plugins/data/public';
 import { RangeColumnParams, UpdateParamsFnType, MODES_TYPES } from './ranges';
 import { AdvancedRangeEditor } from './advanced_editor';
 import { TYPING_DEBOUNCE_TIME, MODES, MIN_HISTOGRAM_BARS } from './constants';
+import { useDebounceWithOptions } from '../helpers';
 
 const BaseRangeEditor = ({
   maxBars,
@@ -37,10 +37,11 @@ const BaseRangeEditor = ({
 }) => {
   const [maxBarsValue, setMaxBarsValue] = useState(String(maxBars));
 
-  useDebounce(
+  useDebounceWithOptions(
     () => {
       onMaxBarsChange(Number(maxBarsValue));
     },
+    { skipFirstRender: true },
     TYPING_DEBOUNCE_TIME,
     [maxBarsValue]
   );
@@ -151,13 +152,14 @@ export const RangeEditor = ({
 }) => {
   const [isAdvancedEditor, toggleAdvancedEditor] = useState(params.type === MODES.Range);
 
-  // if the maxBars in the params is set to auto refresh it with the default value
-  // only on bootstrap
+  // if the maxBars in the params is set to auto refresh it with the default value only on bootstrap
   useEffect(() => {
-    if (params.maxBars !== maxBars) {
-      setParam('maxBars', maxBars);
+    if (!isAdvancedEditor) {
+      if (params.maxBars !== maxBars) {
+        setParam('maxBars', maxBars);
+      }
     }
-  }, [maxBars, params.maxBars, setParam]);
+  }, [maxBars, params.maxBars, setParam, isAdvancedEditor]);
 
   if (isAdvancedEditor) {
     return (

x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/ranges/ranges.test.tsx

@@ -284,7 +284,11 @@ describe('ranges', () => {
           />
         );
 
+        // There's a useEffect in the component that updates the value on bootstrap
+        // because there's a debouncer, wait a bit before calling onChange
         act(() => {
+          jest.advanceTimersByTime(TYPING_DEBOUNCE_TIME * 4);
+
           instance.find(EuiRange).prop('onChange')!(
             {
               currentTarget: {
@@ -293,26 +297,27 @@ describe('ranges', () => {
             } as React.ChangeEvent<HTMLInputElement>,
             true
           );
+
           jest.advanceTimersByTime(TYPING_DEBOUNCE_TIME * 4);
+        });
 
-          expect(setStateSpy).toHaveBeenCalledWith({
-            ...state,
-            layers: {
-              first: {
-                ...state.layers.first,
-                columns: {
-                  ...state.layers.first.columns,
-                  col1: {
-                    ...state.layers.first.columns.col1,
-                    params: {
-                      ...state.layers.first.columns.col1.params,
-                      maxBars: MAX_HISTOGRAM_VALUE,
-                    },
+        expect(setStateSpy).toHaveBeenCalledWith({
+          ...state,
+          layers: {
+            first: {
+              ...state.layers.first,
+              columns: {
+                ...state.layers.first.columns,
+                col1: {
+                  ...state.layers.first.columns.col1,
+                  params: {
+                    ...state.layers.first.columns.col1.params,
+                    maxBars: MAX_HISTOGRAM_VALUE,
                   },
                 },
               },
             },
-          });
+          },
         });
       });
 
@@ -330,59 +335,65 @@ describe('ranges', () => {
           />
         );
 
+        // There's a useEffect in the component that updates the value on bootstrap
+        // because there's a debouncer, wait a bit before calling onChange
         act(() => {
+          jest.advanceTimersByTime(TYPING_DEBOUNCE_TIME * 4);
           // minus button
           instance
             .find('[data-test-subj="lns-indexPattern-range-maxBars-minus"]')
             .find('button')
             .prop('onClick')!({} as ReactMouseEvent);
           jest.advanceTimersByTime(TYPING_DEBOUNCE_TIME * 4);
+        });
 
-          expect(setStateSpy).toHaveBeenCalledWith({
-            ...state,
-            layers: {
-              first: {
-                ...state.layers.first,
-                columns: {
-                  ...state.layers.first.columns,
-                  col1: {
-                    ...state.layers.first.columns.col1,
-                    params: {
-                      ...state.layers.first.columns.col1.params,
-                      maxBars: GRANULARITY_DEFAULT_VALUE - GRANULARITY_STEP,
-                    },
+        expect(setStateSpy).toHaveBeenCalledWith({
+          ...state,
+          layers: {
+            first: {
+              ...state.layers.first,
+              columns: {
+                ...state.layers.first.columns,
+                col1: {
+                  ...state.layers.first.columns.col1,
+                  params: {
+                    ...state.layers.first.columns.col1.params,
+                    maxBars: GRANULARITY_DEFAULT_VALUE - GRANULARITY_STEP,
                   },
                 },
               },
             },
-          });
+          },
+        });
 
+        act(() => {
           // plus button
           instance
             .find('[data-test-subj="lns-indexPattern-range-maxBars-plus"]')
             .find('button')
             .prop('onClick')!({} as ReactMouseEvent);
           jest.advanceTimersByTime(TYPING_DEBOUNCE_TIME * 4);
+        });
 
-          expect(setStateSpy).toHaveBeenCalledWith({
-            ...state,
-            layers: {
-              first: {
-                ...state.layers.first,
-                columns: {
-                  ...state.layers.first.columns,
-                  col1: {
-                    ...state.layers.first.columns.col1,
-                    params: {
-                      ...state.layers.first.columns.col1.params,
-                      maxBars: GRANULARITY_DEFAULT_VALUE,
-                    },
+        expect(setStateSpy).toHaveBeenCalledWith({
+          ...state,
+          layers: {
+            first: {
+              ...state.layers.first,
+              columns: {
+                ...state.layers.first.columns,
+                col1: {
+                  ...state.layers.first.columns.col1,
+                  params: {
+                    ...state.layers.first.columns.col1.params,
+                    maxBars: GRANULARITY_DEFAULT_VALUE,
                   },
                 },
               },
             },
-          });
+          },
         });
+        // });
       });
     });
 
@@ -749,6 +760,22 @@ describe('ranges', () => {
         );
       });
 
+      it('should not update the state on mount', () => {
+        const setStateSpy = jest.fn();
+
+        mount(
+          <InlineOptions
+            {...defaultOptions}
+            state={state}
+            setState={setStateSpy}
+            columnId="col1"
+            currentColumn={state.layers.first.columns.col1 as RangeIndexPatternColumn}
+            layerId="first"
+          />
+        );
+        expect(setStateSpy.mock.calls.length).toBe(0);
+      });
+
       it('should not reset formatters when switching between custom ranges and auto histogram', () => {
         const setStateSpy = jest.fn();
         // now set a format on the range operation

x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/terms/values_range_input.test.tsx

@@ -20,6 +20,13 @@ describe('ValuesRangeInput', () => {
     expect(instance.find(EuiRange).prop('value')).toEqual('5');
   });
 
+  it('should not run onChange function on mount', () => {
+    const onChangeSpy = jest.fn();
+    shallow(<ValuesRangeInput value={5} onChange={onChangeSpy} />);
+
+    expect(onChangeSpy.mock.calls.length).toBe(0);
+  });
+
   it('should run onChange function on update', () => {
     const onChangeSpy = jest.fn();
     const instance = shallow(<ValuesRangeInput value={5} onChange={onChangeSpy} />);
@@ -30,11 +37,10 @@ describe('ValuesRangeInput', () => {
       );
     });
     expect(instance.find(EuiRange).prop('value')).toEqual('7');
-    // useDebounce runs on initialization and on change
-    expect(onChangeSpy.mock.calls.length).toBe(2);
-    expect(onChangeSpy.mock.calls[0][0]).toBe(5);
-    expect(onChangeSpy.mock.calls[1][0]).toBe(7);
+    expect(onChangeSpy.mock.calls.length).toBe(1);
+    expect(onChangeSpy.mock.calls[0][0]).toBe(7);
   });
+
   it('should not run onChange function on update when value is out of 1-100 range', () => {
     const onChangeSpy = jest.fn();
     const instance = shallow(<ValuesRangeInput value={5} onChange={onChangeSpy} />);
@@ -46,9 +52,7 @@ describe('ValuesRangeInput', () => {
     });
     instance.update();
     expect(instance.find(EuiRange).prop('value')).toEqual('107');
-    // useDebounce only runs on initialization
-    expect(onChangeSpy.mock.calls.length).toBe(2);
-    expect(onChangeSpy.mock.calls[0][0]).toBe(5);
-    expect(onChangeSpy.mock.calls[1][0]).toBe(100);
+    expect(onChangeSpy.mock.calls.length).toBe(1);
+    expect(onChangeSpy.mock.calls[0][0]).toBe(100);
   });
 });

x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/terms/values_range_input.tsx

@@ -5,9 +5,9 @@
  */
 
 import React, { useState } from 'react';
-import useDebounce from 'react-use/lib/useDebounce';
 import { i18n } from '@kbn/i18n';
 import { EuiRange } from '@elastic/eui';
+import { useDebounceWithOptions } from '../helpers';
 
 export const ValuesRangeInput = ({
   value,
@@ -20,14 +20,16 @@ export const ValuesRangeInput = ({
   const MAX_NUMBER_OF_VALUES = 100;
 
   const [inputValue, setInputValue] = useState(String(value));
-  useDebounce(
+
+  useDebounceWithOptions(
     () => {
       if (inputValue === '') {
         return;
       }
       const inputNumber = Number(inputValue);
       onChange(Math.min(MAX_NUMBER_OF_VALUES, Math.max(inputNumber, MIN_NUMBER_OF_VALUES)));
     },
+    { skipFirstRender: true },
     256,
     [inputValue]
   );