Skip to content

Commit 551856e

Browse files
Maxim Mordasovjoeyorlando
Maxim Mordasov
andauthored
add debounce for GSelect and RemoteSelect (#2466)
# What this PR does Fix performance Issue in GSelect component when searching ## Which issue(s) this PR fixes #1628 ## Checklist - [ ] Unit, integration, and e2e (if applicable) tests updated - [ ] Documentation added (or `pr:no public docs` PR label added if not required) - [ ] `CHANGELOG.md` updated (or `pr:no changelog` PR label added if not required) --------- Co-authored-by: Joey Orlando <[email protected]> Co-authored-by: Joey Orlando <[email protected]>
1 parent bd49f21 commit 551856e

File tree

5 files changed

+19
-33
lines changed

5 files changed

+19
-33
lines changed

integration-tests/utils/forms.ts

+1-8
Original file line numberDiff line numberDiff line change
@@ -88,14 +88,7 @@ const chooseDropdownValue = async ({ page, value, optionExactMatch = true }: Sel
8888

8989
export const selectDropdownValue = async (args: SelectDropdownValueArgs): Promise<Locator> => {
9090
const selectElement = await openSelect(args);
91-
92-
/**
93-
* use the select search to filter down the options
94-
* TODO: get rid of the slice when we fix the GSelect component..
95-
* without slicing this would fire off an API request for every key-stroke
96-
*/
97-
await selectElement.type(args.value.slice(0, 5));
98-
91+
await selectElement.type(args.value);
9992
await chooseDropdownValue(args);
10093

10194
return selectElement;

playwright.config.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ const config: PlaywrightTestConfig = {
3838
* to flaky tests.. let's just retry failed tests. If the same test fails 3 times, you know something must be up
3939
*/
4040
retries: !!process.env.CI ? 3 : 0,
41-
workers: !!process.env.CI ? 2 : 1,
41+
workers: 3,
4242
/* Reporter to use. See https://playwright.dev/docs/test-reporters */
4343
reporter: 'html',
4444
/* Shared settings for all the projects below. See https://playwright.dev/docs/api/class-testoptions. */

src/containers/GSelect/GSelect.tsx

+6-13
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import { get, isNil } from 'lodash-es';
77
import { observer } from 'mobx-react';
88

99
import { useStore } from 'state/useStore';
10+
import { useDebouncedCallback } from 'utils/hooks';
1011

1112
import styles from './GSelect.module.scss';
1213

@@ -89,30 +90,22 @@ const GSelect = observer((props: GSelectProps) => {
8990
[model, onChange]
9091
);
9192

92-
/**
93-
* without debouncing this function when search is available
94-
* we risk hammering the API endpoint for every single key stroke
95-
* some context on 250ms as the choice here - https://stackoverflow.com/a/44755058/3902555
96-
*/
97-
const loadOptions = (query: string) => {
98-
return model.updateItems(query).then(() => {
93+
const loadOptions = useDebouncedCallback((query: string, cb) => {
94+
model.updateItems(query).then(() => {
9995
const searchResult = model.getSearchResult(query);
10096
let items = Array.isArray(searchResult.results) ? searchResult.results : searchResult;
10197
if (filterOptions) {
10298
items = items.filter((opt: any) => filterOptions(opt[valueField]));
10399
}
104-
105-
return items.map((item: any) => ({
100+
const options = items.map((item: any) => ({
106101
value: item[valueField],
107102
label: get(item, displayField),
108103
imgUrl: item.avatar_url,
109104
description: getDescription && getDescription(item),
110105
}));
106+
cb(options);
111107
});
112-
};
113-
114-
// TODO: why doesn't this work properly?
115-
// const loadOptions = debounce(_loadOptions, showSearch ? 250 : 0);
108+
}, 250);
116109

117110
const values = isMulti
118111
? (value ? (value as string[]) : [])

src/containers/RemoteSelect/RemoteSelect.tsx

+7-10
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,12 @@
1-
import React, { useCallback, useEffect, useMemo, useReducer, useState } from 'react';
1+
import React, { useCallback, useMemo, useReducer, useState } from 'react';
22

33
import { SelectableValue } from '@grafana/data';
44
import { AsyncMultiSelect, AsyncSelect } from '@grafana/ui';
55
import { inject, observer } from 'mobx-react';
66

77
import { makeRequest, isNetworkError } from 'network';
88
import { UserAction, generateMissingPermissionMessage } from 'utils/authorization';
9+
import { useDebouncedCallback } from 'utils/hooks';
910

1011
interface RemoteSelectProps {
1112
autoFocus?: boolean;
@@ -67,24 +68,20 @@ const RemoteSelect = inject('store')(
6768

6869
const [options, setOptions] = useReducer(mergeOptions, []);
6970

70-
const loadOptionsCallback = useCallback(async (query?: string): Promise<SelectableValue[]> => {
71+
const loadOptionsCallback = useDebouncedCallback(async (query: string, cb) => {
7172
try {
7273
const data = await makeRequest(href, { params: { search: query } });
7374
const options = getOptions(data.results || data);
7475
setOptions(options);
7576

76-
return options;
77+
cb(options);
7778
} catch (e) {
7879
if (isNetworkError(e) && e.response.status === 403 && requiredUserAction) {
7980
setNoOptionsMessage(generateMissingPermissionMessage(requiredUserAction));
8081
}
81-
return [];
82+
cb([]);
8283
}
83-
}, []);
84-
85-
useEffect(() => {
86-
loadOptionsCallback();
87-
}, []);
84+
}, 250);
8885

8986
const onChangeCallback = useCallback(
9087
(option) => {
@@ -127,7 +124,7 @@ const RemoteSelect = inject('store')(
127124
isSearchable={showSearch}
128125
value={value}
129126
onChange={onChangeCallback}
130-
defaultOptions={options}
127+
defaultOptions
131128
loadOptions={loadOptionsCallback}
132129
getOptionLabel={getOptionLabel}
133130
noOptionsMessage={noOptionsMessage}

src/models/grafana_team/grafana_team.ts

+4-1
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import { action, observable } from 'mobx';
22

33
import BaseStore from 'models/base_store';
44
import { GrafanaTeam } from 'models/grafana_team/grafana_team.types';
5+
import { makeRequest } from 'network';
56
import { RootStore } from 'state';
67

78
export class GrafanaTeamStore extends BaseStore {
@@ -29,7 +30,9 @@ export class GrafanaTeamStore extends BaseStore {
2930

3031
@action
3132
async updateItems(query = '') {
32-
const result = await this.getAll();
33+
const result = await makeRequest(`${this.path}`, {
34+
params: { search: query },
35+
});
3336

3437
this.items = {
3538
...this.items,

0 commit comments

Comments
 (0)