chore: Improves the Select component to avoid additional queries when…

… all values have been loaded (#16712) * chore: Improves the Select component to avoid additional queries when all values have been loaded * Handles the logic in handlePaginateFetch and removes the use effect
apache · Sep 15, 2021 · 86290cc · 86290cc
1 parent 00ca21e
commit 86290cc
Show file tree

Hide file tree

Showing 2 changed files with 52 additions and 9 deletions.
diff --git 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(<Select {...defaultProps} options={mock} pageSize={pageSize} />);
+  await open();
+  expect(mock).toHaveBeenCalledTimes(1);
+  await type(search);
+  expect(await findSelectOption(search)).toBeInTheDocument();
+  expect(mock).toHaveBeenCalledTimes(1);
+});
+
+test('async - fires a new request if all values have not been fetched', async () => {
+  const mock = jest.fn(loadOptions);
+  const search = 'George';
+  const pageSize = OPTIONS.length / 2;
+  render(<Select {...defaultProps} options={mock} pageSize={pageSize} />);
+  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
@@ -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<string, number>());
   const mappedMode = isSingleMode
     ? undefined
@@ -333,25 +334,35 @@ const Select = ({
     });
 
   const handleData = (data: OptionsType) => {
+    let mergedData: OptionsType = [];
     if (data && Array.isArray(data) && data.length) {
       const dataValues = new Set();
       data.forEach(option =>
         dataValues.add(String(option.value).toLocaleLowerCase()),
       );
 
       // 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(
     () => (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) {
@@ -364,17 +375,24 @@ 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(() => {
           setIsLoading(false);
           setIsTyping(false);
         });
     },
-    [options],
+    [allValuesLoaded, fetchOnlyOnSearch, options],
   );
 
   const handleOnSearch = useMemo(
@@ -493,6 +511,7 @@ const Select = ({
     setSelectOptions(
       options && Array.isArray(options) ? options : EMPTY_OPTIONS,
     );
+    setAllValuesLoaded(false);
   }, [options]);
 
   useEffect(() => {