Skip to content

Commit c8b8a0a

Browse files
dej611kibanamachine
dej611
and
kibanamachine
authored
[Lens] Avoid unnecessary data fetching on dimension flyout open (#82957)
Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
1 parent 01b1710 commit c8b8a0a

File tree

6 files changed

+129
-63
lines changed

6 files changed

+129
-63
lines changed

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

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
/*
2+
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
3+
* or more contributor license agreements. Licensed under the Elastic License;
4+
* you may not use this file except in compliance with the Elastic License.
5+
*/
6+
7+
import { useRef } from 'react';
8+
import useDebounce from 'react-use/lib/useDebounce';
9+
10+
export const useDebounceWithOptions = (
11+
fn: Function,
12+
{ skipFirstRender }: { skipFirstRender: boolean } = { skipFirstRender: false },
13+
ms?: number | undefined,
14+
deps?: React.DependencyList | undefined
15+
) => {
16+
const isFirstRender = useRef(true);
17+
const newDeps = [...(deps || []), isFirstRender];
18+
19+
return useDebounce(
20+
() => {
21+
if (skipFirstRender && isFirstRender.current) {
22+
isFirstRender.current = false;
23+
return;
24+
}
25+
return fn();
26+
},
27+
ms,
28+
newDeps
29+
);
30+
};

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

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@ import './advanced_editor.scss';
88

99
import React, { useState, MouseEventHandler } from 'react';
1010
import { i18n } from '@kbn/i18n';
11-
import useDebounce from 'react-use/lib/useDebounce';
1211
import {
1312
EuiFlexGroup,
1413
EuiFlexItem,
@@ -31,6 +30,7 @@ import {
3130
DraggableBucketContainer,
3231
LabelInput,
3332
} from '../shared_components';
33+
import { useDebounceWithOptions } from '../helpers';
3434

3535
const generateId = htmlIdGenerator();
3636

@@ -208,12 +208,13 @@ export const AdvancedRangeEditor = ({
208208

209209
const lastIndex = localRanges.length - 1;
210210

211-
// Update locally all the time, but bounce the parents prop function
212-
// to aviod too many requests
213-
useDebounce(
211+
// Update locally all the time, but bounce the parents prop function to aviod too many requests
212+
// Avoid to trigger on first render
213+
useDebounceWithOptions(
214214
() => {
215215
setRanges(localRanges.map(({ id, ...rest }) => ({ ...rest })));
216216
},
217+
{ skipFirstRender: true },
217218
TYPING_DEBOUNCE_TIME,
218219
[localRanges]
219220
);

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

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@
66

77
import React, { useEffect, useState } from 'react';
88
import { i18n } from '@kbn/i18n';
9-
import useDebounce from 'react-use/lib/useDebounce';
109
import {
1110
EuiButtonEmpty,
1211
EuiFormRow,
@@ -21,6 +20,7 @@ import { IFieldFormat } from 'src/plugins/data/public';
2120
import { RangeColumnParams, UpdateParamsFnType, MODES_TYPES } from './ranges';
2221
import { AdvancedRangeEditor } from './advanced_editor';
2322
import { TYPING_DEBOUNCE_TIME, MODES, MIN_HISTOGRAM_BARS } from './constants';
23+
import { useDebounceWithOptions } from '../helpers';
2424

2525
const BaseRangeEditor = ({
2626
maxBars,
@@ -37,10 +37,11 @@ const BaseRangeEditor = ({
3737
}) => {
3838
const [maxBarsValue, setMaxBarsValue] = useState(String(maxBars));
3939

40-
useDebounce(
40+
useDebounceWithOptions(
4141
() => {
4242
onMaxBarsChange(Number(maxBarsValue));
4343
},
44+
{ skipFirstRender: true },
4445
TYPING_DEBOUNCE_TIME,
4546
[maxBarsValue]
4647
);
@@ -151,13 +152,14 @@ export const RangeEditor = ({
151152
}) => {
152153
const [isAdvancedEditor, toggleAdvancedEditor] = useState(params.type === MODES.Range);
153154

154-
// if the maxBars in the params is set to auto refresh it with the default value
155-
// only on bootstrap
155+
// if the maxBars in the params is set to auto refresh it with the default value only on bootstrap
156156
useEffect(() => {
157-
if (params.maxBars !== maxBars) {
158-
setParam('maxBars', maxBars);
157+
if (!isAdvancedEditor) {
158+
if (params.maxBars !== maxBars) {
159+
setParam('maxBars', maxBars);
160+
}
159161
}
160-
}, [maxBars, params.maxBars, setParam]);
162+
}, [maxBars, params.maxBars, setParam, isAdvancedEditor]);
161163

162164
if (isAdvancedEditor) {
163165
return (

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

Lines changed: 69 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -284,7 +284,11 @@ describe('ranges', () => {
284284
/>
285285
);
286286

287+
// There's a useEffect in the component that updates the value on bootstrap
288+
// because there's a debouncer, wait a bit before calling onChange
287289
act(() => {
290+
jest.advanceTimersByTime(TYPING_DEBOUNCE_TIME * 4);
291+
288292
instance.find(EuiRange).prop('onChange')!(
289293
{
290294
currentTarget: {
@@ -293,26 +297,27 @@ describe('ranges', () => {
293297
} as React.ChangeEvent<HTMLInputElement>,
294298
true
295299
);
300+
296301
jest.advanceTimersByTime(TYPING_DEBOUNCE_TIME * 4);
302+
});
297303

298-
expect(setStateSpy).toHaveBeenCalledWith({
299-
...state,
300-
layers: {
301-
first: {
302-
...state.layers.first,
303-
columns: {
304-
...state.layers.first.columns,
305-
col1: {
306-
...state.layers.first.columns.col1,
307-
params: {
308-
...state.layers.first.columns.col1.params,
309-
maxBars: MAX_HISTOGRAM_VALUE,
310-
},
304+
expect(setStateSpy).toHaveBeenCalledWith({
305+
...state,
306+
layers: {
307+
first: {
308+
...state.layers.first,
309+
columns: {
310+
...state.layers.first.columns,
311+
col1: {
312+
...state.layers.first.columns.col1,
313+
params: {
314+
...state.layers.first.columns.col1.params,
315+
maxBars: MAX_HISTOGRAM_VALUE,
311316
},
312317
},
313318
},
314319
},
315-
});
320+
},
316321
});
317322
});
318323

@@ -330,59 +335,65 @@ describe('ranges', () => {
330335
/>
331336
);
332337

338+
// There's a useEffect in the component that updates the value on bootstrap
339+
// because there's a debouncer, wait a bit before calling onChange
333340
act(() => {
341+
jest.advanceTimersByTime(TYPING_DEBOUNCE_TIME * 4);
334342
// minus button
335343
instance
336344
.find('[data-test-subj="lns-indexPattern-range-maxBars-minus"]')
337345
.find('button')
338346
.prop('onClick')!({} as ReactMouseEvent);
339347
jest.advanceTimersByTime(TYPING_DEBOUNCE_TIME * 4);
348+
});
340349

341-
expect(setStateSpy).toHaveBeenCalledWith({
342-
...state,
343-
layers: {
344-
first: {
345-
...state.layers.first,
346-
columns: {
347-
...state.layers.first.columns,
348-
col1: {
349-
...state.layers.first.columns.col1,
350-
params: {
351-
...state.layers.first.columns.col1.params,
352-
maxBars: GRANULARITY_DEFAULT_VALUE - GRANULARITY_STEP,
353-
},
350+
expect(setStateSpy).toHaveBeenCalledWith({
351+
...state,
352+
layers: {
353+
first: {
354+
...state.layers.first,
355+
columns: {
356+
...state.layers.first.columns,
357+
col1: {
358+
...state.layers.first.columns.col1,
359+
params: {
360+
...state.layers.first.columns.col1.params,
361+
maxBars: GRANULARITY_DEFAULT_VALUE - GRANULARITY_STEP,
354362
},
355363
},
356364
},
357365
},
358-
});
366+
},
367+
});
359368

369+
act(() => {
360370
// plus button
361371
instance
362372
.find('[data-test-subj="lns-indexPattern-range-maxBars-plus"]')
363373
.find('button')
364374
.prop('onClick')!({} as ReactMouseEvent);
365375
jest.advanceTimersByTime(TYPING_DEBOUNCE_TIME * 4);
376+
});
366377

367-
expect(setStateSpy).toHaveBeenCalledWith({
368-
...state,
369-
layers: {
370-
first: {
371-
...state.layers.first,
372-
columns: {
373-
...state.layers.first.columns,
374-
col1: {
375-
...state.layers.first.columns.col1,
376-
params: {
377-
...state.layers.first.columns.col1.params,
378-
maxBars: GRANULARITY_DEFAULT_VALUE,
379-
},
378+
expect(setStateSpy).toHaveBeenCalledWith({
379+
...state,
380+
layers: {
381+
first: {
382+
...state.layers.first,
383+
columns: {
384+
...state.layers.first.columns,
385+
col1: {
386+
...state.layers.first.columns.col1,
387+
params: {
388+
...state.layers.first.columns.col1.params,
389+
maxBars: GRANULARITY_DEFAULT_VALUE,
380390
},
381391
},
382392
},
383393
},
384-
});
394+
},
385395
});
396+
// });
386397
});
387398
});
388399

@@ -749,6 +760,22 @@ describe('ranges', () => {
749760
);
750761
});
751762

763+
it('should not update the state on mount', () => {
764+
const setStateSpy = jest.fn();
765+
766+
mount(
767+
<InlineOptions
768+
{...defaultOptions}
769+
state={state}
770+
setState={setStateSpy}
771+
columnId="col1"
772+
currentColumn={state.layers.first.columns.col1 as RangeIndexPatternColumn}
773+
layerId="first"
774+
/>
775+
);
776+
expect(setStateSpy.mock.calls.length).toBe(0);
777+
});
778+
752779
it('should not reset formatters when switching between custom ranges and auto histogram', () => {
753780
const setStateSpy = jest.fn();
754781
// now set a format on the range operation

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

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,13 @@ describe('ValuesRangeInput', () => {
2020
expect(instance.find(EuiRange).prop('value')).toEqual('5');
2121
});
2222

23+
it('should not run onChange function on mount', () => {
24+
const onChangeSpy = jest.fn();
25+
shallow(<ValuesRangeInput value={5} onChange={onChangeSpy} />);
26+
27+
expect(onChangeSpy.mock.calls.length).toBe(0);
28+
});
29+
2330
it('should run onChange function on update', () => {
2431
const onChangeSpy = jest.fn();
2532
const instance = shallow(<ValuesRangeInput value={5} onChange={onChangeSpy} />);
@@ -30,11 +37,10 @@ describe('ValuesRangeInput', () => {
3037
);
3138
});
3239
expect(instance.find(EuiRange).prop('value')).toEqual('7');
33-
// useDebounce runs on initialization and on change
34-
expect(onChangeSpy.mock.calls.length).toBe(2);
35-
expect(onChangeSpy.mock.calls[0][0]).toBe(5);
36-
expect(onChangeSpy.mock.calls[1][0]).toBe(7);
40+
expect(onChangeSpy.mock.calls.length).toBe(1);
41+
expect(onChangeSpy.mock.calls[0][0]).toBe(7);
3742
});
43+
3844
it('should not run onChange function on update when value is out of 1-100 range', () => {
3945
const onChangeSpy = jest.fn();
4046
const instance = shallow(<ValuesRangeInput value={5} onChange={onChangeSpy} />);
@@ -46,9 +52,7 @@ describe('ValuesRangeInput', () => {
4652
});
4753
instance.update();
4854
expect(instance.find(EuiRange).prop('value')).toEqual('107');
49-
// useDebounce only runs on initialization
50-
expect(onChangeSpy.mock.calls.length).toBe(2);
51-
expect(onChangeSpy.mock.calls[0][0]).toBe(5);
52-
expect(onChangeSpy.mock.calls[1][0]).toBe(100);
55+
expect(onChangeSpy.mock.calls.length).toBe(1);
56+
expect(onChangeSpy.mock.calls[0][0]).toBe(100);
5357
});
5458
});

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

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,9 @@
55
*/
66

77
import React, { useState } from 'react';
8-
import useDebounce from 'react-use/lib/useDebounce';
98
import { i18n } from '@kbn/i18n';
109
import { EuiRange } from '@elastic/eui';
10+
import { useDebounceWithOptions } from '../helpers';
1111

1212
export const ValuesRangeInput = ({
1313
value,
@@ -20,14 +20,16 @@ export const ValuesRangeInput = ({
2020
const MAX_NUMBER_OF_VALUES = 100;
2121

2222
const [inputValue, setInputValue] = useState(String(value));
23-
useDebounce(
23+
24+
useDebounceWithOptions(
2425
() => {
2526
if (inputValue === '') {
2627
return;
2728
}
2829
const inputNumber = Number(inputValue);
2930
onChange(Math.min(MAX_NUMBER_OF_VALUES, Math.max(inputNumber, MIN_NUMBER_OF_VALUES)));
3031
},
32+
{ skipFirstRender: true },
3133
256,
3234
[inputValue]
3335
);

0 commit comments

Comments
 (0)