From 347daead71ea6d9e0b7c38ce34b2a0e709c21750 Mon Sep 17 00:00:00 2001 From: "Michael S. Molina" Date: Wed, 15 Sep 2021 11:40:25 -0300 Subject: [PATCH 1/2] chore: Improves the Select component to avoid additional queries when all values have been loaded --- .../src/components/Select/Select.test.tsx | 24 +++++++++++++++++++ .../src/components/Select/Select.tsx | 15 +++++++++++- 2 files changed, 38 insertions(+), 1 deletion(-) diff --git a/superset-frontend/src/components/Select/Select.test.tsx b/superset-frontend/src/components/Select/Select.test.tsx index b9deaf76f81eb..3756d4a5d412e 100644 --- a/superset-frontend/src/components/Select/Select.test.tsx +++ b/superset-frontend/src/components/Select/Select.test.tsx @@ -605,6 +605,30 @@ test('async - does not fire a new request for the same search input', async () = expect(loadOptions).toHaveBeenCalledTimes(1); }); +test('async - does not fire a new request if all values have been fetched', async () => { + const mock = jest.fn(loadOptions); + const search = 'George'; + const pageSize = OPTIONS.length; + render(); + await open(); + expect(mock).toHaveBeenCalledTimes(1); + await type(search); + expect(await findSelectOption(search)).toBeInTheDocument(); + expect(mock).toHaveBeenCalledTimes(2); +}); + /* TODO: Add tests that require scroll interaction. Needs further investigation. - Fetches more data when scrolling and more data is available diff --git a/superset-frontend/src/components/Select/Select.tsx b/superset-frontend/src/components/Select/Select.tsx index 68d79e4a599c8..09447b85214cb 100644 --- a/superset-frontend/src/components/Select/Select.tsx +++ b/superset-frontend/src/components/Select/Select.tsx @@ -210,6 +210,7 @@ const Select = ({ const [page, setPage] = useState(0); const [totalCount, setTotalCount] = useState(0); const [loadingEnabled, setLoadingEnabled] = useState(!lazyLoading); + const [allValuesLoaded, setAllValuesLoaded] = useState(false); const fetchedQueries = useRef(new Map()); const mappedMode = isSingleMode ? undefined @@ -352,6 +353,11 @@ const Select = ({ const handlePaginatedFetch = useMemo( () => (value: string, page: number, pageSize: number) => { + if (allValuesLoaded) { + setIsLoading(false); + setIsTyping(false); + return; + } const key = `${value};${page};${pageSize}`; const cachedCount = fetchedQueries.current.get(key); if (cachedCount) { @@ -374,7 +380,7 @@ const Select = ({ setIsTyping(false); }); }, - [options], + [allValuesLoaded, options], ); const handleOnSearch = useMemo( @@ -493,6 +499,7 @@ const Select = ({ setSelectOptions( options && Array.isArray(options) ? options : EMPTY_OPTIONS, ); + setAllValuesLoaded(false); }, [options]); useEffect(() => { @@ -557,6 +564,12 @@ const Select = ({ } }, [isLoading, loading]); + useEffect(() => { + if (!fetchOnlyOnSearch && searchedValue === '' && totalCount > 0) { + setAllValuesLoaded(selectOptions.length >= totalCount); + } + }, [fetchOnlyOnSearch, searchedValue, selectOptions.length, totalCount]); + return ( {header} From 183f452be6bc35f24cfdff18139bb2ffc3a66a9e Mon Sep 17 00:00:00 2001 From: "Michael S. Molina" Date: Wed, 15 Sep 2021 14:29:43 -0300 Subject: [PATCH 2/2] Handles the logic in handlePaginateFetch and removes the use effect --- .../src/components/Select/Select.tsx | 36 +++++++++++-------- 1 file changed, 21 insertions(+), 15 deletions(-) diff --git a/superset-frontend/src/components/Select/Select.tsx b/superset-frontend/src/components/Select/Select.tsx index 09447b85214cb..e70527438115d 100644 --- a/superset-frontend/src/components/Select/Select.tsx +++ b/superset-frontend/src/components/Select/Select.tsx @@ -334,6 +334,7 @@ const Select = ({ }); const handleData = (data: OptionsType) => { + let mergedData: OptionsType = []; if (data && Array.isArray(data) && data.length) { const dataValues = new Set(); data.forEach(option => @@ -341,14 +342,18 @@ const Select = ({ ); // merges with existing and creates unique options - setSelectOptions(prevOptions => [ - ...prevOptions.filter( - previousOption => - !dataValues.has(String(previousOption.value).toLocaleLowerCase()), - ), - ...data, - ]); + setSelectOptions(prevOptions => { + mergedData = [ + ...prevOptions.filter( + previousOption => + !dataValues.has(String(previousOption.value).toLocaleLowerCase()), + ), + ...data, + ]; + return mergedData; + }); } + return mergedData; }; const handlePaginatedFetch = useMemo( @@ -370,9 +375,16 @@ const Select = ({ const fetchOptions = options as OptionsPagePromise; fetchOptions(value, page, pageSize) .then(({ data, totalCount }: OptionsTypePage) => { - handleData(data); + const mergedData = handleData(data); fetchedQueries.current.set(key, totalCount); setTotalCount(totalCount); + if ( + !fetchOnlyOnSearch && + value === '' && + mergedData.length >= totalCount + ) { + setAllValuesLoaded(true); + } }) .catch(onError) .finally(() => { @@ -380,7 +392,7 @@ const Select = ({ setIsTyping(false); }); }, - [allValuesLoaded, options], + [allValuesLoaded, fetchOnlyOnSearch, options], ); const handleOnSearch = useMemo( @@ -564,12 +576,6 @@ const Select = ({ } }, [isLoading, loading]); - useEffect(() => { - if (!fetchOnlyOnSearch && searchedValue === '' && totalCount > 0) { - setAllValuesLoaded(selectOptions.length >= totalCount); - } - }, [fetchOnlyOnSearch, searchedValue, selectOptions.length, totalCount]); - return ( {header}