[GitHub] [superset] michael-s-molina commented on a diff in pull request #20690: chore: Remove unecessary code from async and sync select components

[GitBox <gi...@apache.org>]

michael-s-molina commented on code in PR #20690:
URL: https://github.com/apache/superset/pull/20690#discussion_r920332830


##########
superset-frontend/src/components/Select/Select.tsx:
##########
@@ -322,25 +286,19 @@ const Select = (
   }: SelectProps,
   ref: RefObject<SelectRef>,
 ) => {
-  const isAsync = typeof options === 'function';
   const isSingleMode = mode === 'single';
-  const shouldShowSearch = isAsync || allowNewOptions ? true : showSearch;
+  const shouldShowSearch = allowNewOptions ? true : showSearch;
   const [selectValue, setSelectValue] = useState(value);
   const [inputValue, setInputValue] = useState('');
   const [isLoading, setIsLoading] = useState(loading);
   const [error, setError] = useState('');
   const [isDropdownVisible, setIsDropdownVisible] = useState(false);
-  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>());

Review Comment:
   You can remove `fetchedQueries` and all its references.



##########
superset-frontend/src/components/Select/Select.tsx:
##########
@@ -148,13 +126,6 @@ export interface SelectProps extends PickedSelectProps {
    * False by default.
    */
   invertSelection?: boolean;
-  /**
-   * It fires a request against the server only after
-   * searching.
-   * Works in async mode only (See the options property).
-   * Undefined by default.
-   */
-  fetchOnlyOnSearch?: boolean;

Review Comment:
   Can you remove `onError` prop and the associated error component, `internalOnError`, and error state?



##########
superset-frontend/src/components/Select/Select.tsx:
##########
@@ -358,10 +316,9 @@ const Select = (
   const sortComparatorForNoSearch = useCallback(

Review Comment:
   You can remove `sortComparatorForNoSearch` and just use `sortSelectedFirst` now that the code does not have another comparison.



##########
superset-frontend/src/components/Select/Select.tsx:
##########
@@ -77,12 +74,6 @@ export type OptionsTypePage = {
   totalCount: number;
 };
 
-export type OptionsPagePromise = (
-  search: string,
-  page: number,
-  pageSize: number,
-) => Promise<OptionsTypePage>;
-
 export type SelectRef = HTMLInputElement & { clearCache: () => void };

Review Comment:
   We should remove the `SelectRef` as well because there's no more cache. Just use `HTMLInputElement` as the ref type.
   
   We should also remove `clearCache` and `useImperativeHandle` below.



##########
superset-frontend/src/components/Select/Select.tsx:
##########
@@ -133,13 +117,7 @@ export interface SelectProps extends PickedSelectProps {
    * The options can also be async, a promise that returns
    * an array of options.
    */
-  options: OptionsType | OptionsPagePromise;

Review Comment:
   Can you remove `OptionsTypePage` declaration as well?



##########
superset-frontend/src/components/Select/Select.tsx:
##########
@@ -358,10 +316,9 @@ const Select = (
   const sortComparatorForNoSearch = useCallback(
     (a: AntdLabeledValue, b: AntdLabeledValue) =>
       sortSelectedFirst(a, b) ||
-      // Only apply the custom sorter in async mode because we should
-      // preserve the options order as much as possible.
-      (isAsync ? sortComparator(a, b, '') : 0),
-    [isAsync, sortComparator, sortSelectedFirst],
+      // we should preserve the options order as much as possible.
+      0,
+    [sortComparator, sortSelectedFirst],
   );
 
   const initialOptions = useMemo(

Review Comment:
   We should remove `internalOnError` below.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org