From b951b6b6bd9bea9136475492b28b7bdea9e9a1cf Mon Sep 17 00:00:00 2001 From: Maxim Mordasov Date: Tue, 11 Jul 2023 16:51:22 +0300 Subject: [PATCH] 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 https://github.com/grafana/oncall/issues/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 Co-authored-by: Joey Orlando --- CHANGELOG.md | 7 +++++++ .../integration-tests/utils/forms.ts | 9 +-------- grafana-plugin/playwright.config.ts | 2 +- .../src/containers/GSelect/GSelect.tsx | 19 ++++++------------- .../containers/RemoteSelect/RemoteSelect.tsx | 17 +++++++---------- .../src/models/grafana_team/grafana_team.ts | 5 ++++- 6 files changed, 26 insertions(+), 33 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e6fd2a9d..9316eade 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,13 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## Unreleased + +### Fixed + +- Add debounce on Select UI components to avoid making API search requests on each key-down event by + @maskin25 ([#2466](https://github.com/grafana/oncall/pull/2466)) + ## v1.3.8 (2023-07-11) ### Added diff --git a/grafana-plugin/integration-tests/utils/forms.ts b/grafana-plugin/integration-tests/utils/forms.ts index 30fb8cdd..fb00b9dd 100644 --- a/grafana-plugin/integration-tests/utils/forms.ts +++ b/grafana-plugin/integration-tests/utils/forms.ts @@ -88,14 +88,7 @@ const chooseDropdownValue = async ({ page, value, optionExactMatch = true }: Sel export const selectDropdownValue = async (args: SelectDropdownValueArgs): Promise => { const selectElement = await openSelect(args); - - /** - * use the select search to filter down the options - * TODO: get rid of the slice when we fix the GSelect component.. - * without slicing this would fire off an API request for every key-stroke - */ - await selectElement.type(args.value.slice(0, 5)); - + await selectElement.type(args.value); await chooseDropdownValue(args); return selectElement; diff --git a/grafana-plugin/playwright.config.ts b/grafana-plugin/playwright.config.ts index bab2ea80..424a03e3 100644 --- a/grafana-plugin/playwright.config.ts +++ b/grafana-plugin/playwright.config.ts @@ -38,7 +38,7 @@ const config: PlaywrightTestConfig = { * to flaky tests.. let's just retry failed tests. If the same test fails 3 times, you know something must be up */ retries: !!process.env.CI ? 3 : 0, - workers: !!process.env.CI ? 2 : 1, + workers: 3, /* Reporter to use. See https://playwright.dev/docs/test-reporters */ reporter: 'html', /* Shared settings for all the projects below. See https://playwright.dev/docs/api/class-testoptions. */ diff --git a/grafana-plugin/src/containers/GSelect/GSelect.tsx b/grafana-plugin/src/containers/GSelect/GSelect.tsx index 16fb6db1..38c3e351 100644 --- a/grafana-plugin/src/containers/GSelect/GSelect.tsx +++ b/grafana-plugin/src/containers/GSelect/GSelect.tsx @@ -7,6 +7,7 @@ import { get, isNil } from 'lodash-es'; import { observer } from 'mobx-react'; import { useStore } from 'state/useStore'; +import { useDebouncedCallback } from 'utils/hooks'; import styles from './GSelect.module.scss'; @@ -89,30 +90,22 @@ const GSelect = observer((props: GSelectProps) => { [model, onChange] ); - /** - * without debouncing this function when search is available - * we risk hammering the API endpoint for every single key stroke - * some context on 250ms as the choice here - https://stackoverflow.com/a/44755058/3902555 - */ - const loadOptions = (query: string) => { - return model.updateItems(query).then(() => { + const loadOptions = useDebouncedCallback((query: string, cb) => { + model.updateItems(query).then(() => { const searchResult = model.getSearchResult(query); let items = Array.isArray(searchResult.results) ? searchResult.results : searchResult; if (filterOptions) { items = items.filter((opt: any) => filterOptions(opt[valueField])); } - - return items.map((item: any) => ({ + const options = items.map((item: any) => ({ value: item[valueField], label: get(item, displayField), imgUrl: item.avatar_url, description: getDescription && getDescription(item), })); + cb(options); }); - }; - - // TODO: why doesn't this work properly? - // const loadOptions = debounce(_loadOptions, showSearch ? 250 : 0); + }, 250); const values = isMulti ? (value ? (value as string[]) : []) diff --git a/grafana-plugin/src/containers/RemoteSelect/RemoteSelect.tsx b/grafana-plugin/src/containers/RemoteSelect/RemoteSelect.tsx index 91b17d19..b9572040 100644 --- a/grafana-plugin/src/containers/RemoteSelect/RemoteSelect.tsx +++ b/grafana-plugin/src/containers/RemoteSelect/RemoteSelect.tsx @@ -1,4 +1,4 @@ -import React, { useCallback, useEffect, useMemo, useReducer, useState } from 'react'; +import React, { useCallback, useMemo, useReducer, useState } from 'react'; import { SelectableValue } from '@grafana/data'; import { AsyncMultiSelect, AsyncSelect } from '@grafana/ui'; @@ -6,6 +6,7 @@ import { inject, observer } from 'mobx-react'; import { makeRequest, isNetworkError } from 'network'; import { UserAction, generateMissingPermissionMessage } from 'utils/authorization'; +import { useDebouncedCallback } from 'utils/hooks'; interface RemoteSelectProps { autoFocus?: boolean; @@ -67,24 +68,20 @@ const RemoteSelect = inject('store')( const [options, setOptions] = useReducer(mergeOptions, []); - const loadOptionsCallback = useCallback(async (query?: string): Promise => { + const loadOptionsCallback = useDebouncedCallback(async (query: string, cb) => { try { const data = await makeRequest(href, { params: { search: query } }); const options = getOptions(data.results || data); setOptions(options); - return options; + cb(options); } catch (e) { if (isNetworkError(e) && e.response.status === 403 && requiredUserAction) { setNoOptionsMessage(generateMissingPermissionMessage(requiredUserAction)); } - return []; + cb([]); } - }, []); - - useEffect(() => { - loadOptionsCallback(); - }, []); + }, 250); const onChangeCallback = useCallback( (option) => { @@ -127,7 +124,7 @@ const RemoteSelect = inject('store')( isSearchable={showSearch} value={value} onChange={onChangeCallback} - defaultOptions={options} + defaultOptions loadOptions={loadOptionsCallback} getOptionLabel={getOptionLabel} noOptionsMessage={noOptionsMessage} diff --git a/grafana-plugin/src/models/grafana_team/grafana_team.ts b/grafana-plugin/src/models/grafana_team/grafana_team.ts index 243d3af2..8ca9a7b1 100644 --- a/grafana-plugin/src/models/grafana_team/grafana_team.ts +++ b/grafana-plugin/src/models/grafana_team/grafana_team.ts @@ -2,6 +2,7 @@ import { action, observable } from 'mobx'; import BaseStore from 'models/base_store'; import { GrafanaTeam } from 'models/grafana_team/grafana_team.types'; +import { makeRequest } from 'network'; import { RootStore } from 'state'; export class GrafanaTeamStore extends BaseStore { @@ -29,7 +30,9 @@ export class GrafanaTeamStore extends BaseStore { @action async updateItems(query = '') { - const result = await this.getAll(); + const result = await makeRequest(`${this.path}`, { + params: { search: query }, + }); this.items = { ...this.items,