Review feedback.

Signed-off-by: Eric Peterson <ericpeterson@spotify.com>

plugins/search/src/components/SearchFilter/SearchFilter.Autocomplete.test.tsx

@@ -14,8 +14,7 @@
  * limitations under the License.
  */
 
-import { ApiProvider } from '@backstage/core-app-api';
-import { TestApiRegistry } from '@backstage/test-utils';
+import { TestApiProvider } from '@backstage/test-utils';
 import { screen, render, waitFor, within } from '@testing-library/react';
 import userEvent from '@testing-library/user-event';
 import React from 'react';
@@ -35,7 +34,6 @@ const SearchContextFilterSpy = ({ name }: { name: string }) => {
 
 describe('SearchFilter.Autocomplete', () => {
   const query = jest.fn().mockResolvedValue({});
-  const mockApis = TestApiRegistry.from([searchApiRef, { query }]);
   const emptySearchContext = {
     term: '',
     types: [],
@@ -47,11 +45,11 @@ describe('SearchFilter.Autocomplete', () => {
 
   it('renders as expected', async () => {
     render(
-      <ApiProvider apis={mockApis}>
+      <TestApiProvider apis={[[searchApiRef, { query }]]}>
         <SearchContextProvider>
           <SearchFilter.Autocomplete name={name} values={values} />
         </SearchContextProvider>
-      </ApiProvider>,
+      </TestApiProvider>,
     );
 
     const autocomplete = screen.getByRole('combobox');
@@ -68,14 +66,11 @@ describe('SearchFilter.Autocomplete', () => {
 
   it('renders as expected with async values', async () => {
     render(
-      <ApiProvider apis={mockApis}>
+      <TestApiProvider apis={[[searchApiRef, { query }]]}>
         <SearchContextProvider>
-          <SearchFilter.Autocomplete
-            name={name}
-            asyncValues={async () => values}
-          />
+          <SearchFilter.Autocomplete name={name} values={async () => values} />
         </SearchContextProvider>
-      </ApiProvider>,
+      </TestApiProvider>,
     );
 
     const autocomplete = screen.getByRole('combobox');
@@ -92,7 +87,7 @@ describe('SearchFilter.Autocomplete', () => {
 
   it('does not affect unrelated filter state', async () => {
     render(
-      <ApiProvider apis={mockApis}>
+      <TestApiProvider apis={[[searchApiRef, { query }]]}>
         <SearchContextProvider
           initialState={{
             ...emptySearchContext,
@@ -103,7 +98,7 @@ describe('SearchFilter.Autocomplete', () => {
           <SearchContextFilterSpy name={name} />
           <SearchContextFilterSpy name="unrelated" />
         </SearchContextProvider>
-      </ApiProvider>,
+      </TestApiProvider>,
     );
 
     // The spy should show the initial value.
@@ -136,7 +131,7 @@ describe('SearchFilter.Autocomplete', () => {
   describe('single', () => {
     it('renders as expected with defaultValue', async () => {
       render(
-        <ApiProvider apis={mockApis}>
+        <TestApiProvider apis={[[searchApiRef, { query }]]}>
           <SearchContextProvider>
             <SearchFilter.Autocomplete
               name={name}
@@ -145,7 +140,7 @@ describe('SearchFilter.Autocomplete', () => {
             />
             <SearchContextFilterSpy name={name} />
           </SearchContextProvider>
-        </ApiProvider>,
+        </TestApiProvider>,
       );
 
       const autocomplete = screen.getByRole('combobox');
@@ -161,7 +156,7 @@ describe('SearchFilter.Autocomplete', () => {
 
     it('renders as expected with initial context', async () => {
       render(
-        <ApiProvider apis={mockApis}>
+        <TestApiProvider apis={[[searchApiRef, { query }]]}>
           <SearchContextProvider
             initialState={{
               ...emptySearchContext,
@@ -171,7 +166,7 @@ describe('SearchFilter.Autocomplete', () => {
             <SearchFilter.Autocomplete name={name} values={values} />
             <SearchContextFilterSpy name={name} />
           </SearchContextProvider>
-        </ApiProvider>,
+        </TestApiProvider>,
       );
 
       const autocomplete = screen.getByRole('combobox');
@@ -187,12 +182,12 @@ describe('SearchFilter.Autocomplete', () => {
 
     it('sets filter state when selecting a value', async () => {
       render(
-        <ApiProvider apis={mockApis}>
+        <TestApiProvider apis={[[searchApiRef, { query }]]}>
           <SearchContextProvider>
             <SearchFilter.Autocomplete name={name} values={values} />
             <SearchContextFilterSpy name={name} />
           </SearchContextProvider>
-        </ApiProvider>,
+        </TestApiProvider>,
       );
 
       // Select the first option in the autocomplete.
@@ -225,7 +220,7 @@ describe('SearchFilter.Autocomplete', () => {
   describe('multiple', () => {
     it('renders as expected with defaultValue', async () => {
       render(
-        <ApiProvider apis={mockApis}>
+        <TestApiProvider apis={[[searchApiRef, { query }]]}>
           <SearchContextProvider>
             <SearchFilter.Autocomplete
               multiple
@@ -235,7 +230,7 @@ describe('SearchFilter.Autocomplete', () => {
             />
             <SearchContextFilterSpy name={name} />
           </SearchContextProvider>
-        </ApiProvider>,
+        </TestApiProvider>,
       );
 
       await waitFor(() => {
@@ -249,7 +244,7 @@ describe('SearchFilter.Autocomplete', () => {
 
     it('renders as expected with initial context', async () => {
       render(
-        <ApiProvider apis={mockApis}>
+        <TestApiProvider apis={[[searchApiRef, { query }]]}>
           <SearchContextProvider
             initialState={{
               ...emptySearchContext,
@@ -259,7 +254,7 @@ describe('SearchFilter.Autocomplete', () => {
             <SearchFilter.Autocomplete multiple name={name} values={values} />
             <SearchContextFilterSpy name={name} />
           </SearchContextProvider>
-        </ApiProvider>,
+        </TestApiProvider>,
       );
 
       await waitFor(() => {
@@ -273,7 +268,7 @@ describe('SearchFilter.Autocomplete', () => {
 
     it('respects tag limit configuration', async () => {
       render(
-        <ApiProvider apis={mockApis}>
+        <TestApiProvider apis={[[searchApiRef, { query }]]}>
           <SearchContextProvider>
             <SearchFilter.Autocomplete
               multiple
@@ -282,7 +277,7 @@ describe('SearchFilter.Autocomplete', () => {
               values={values}
             />
           </SearchContextProvider>
-        </ApiProvider>,
+        </TestApiProvider>,
       );
 
       const autocomplete = screen.getByRole('combobox');
@@ -322,12 +317,12 @@ describe('SearchFilter.Autocomplete', () => {
 
     it('sets filter state when selecting a value', async () => {
       render(
-        <ApiProvider apis={mockApis}>
+        <TestApiProvider apis={[[searchApiRef, { query }]]}>
           <SearchContextProvider>
             <SearchFilter.Autocomplete multiple name={name} values={values} />
             <SearchContextFilterSpy name={name} />
           </SearchContextProvider>
-        </ApiProvider>,
+        </TestApiProvider>,
       );
 
       const autocomplete = screen.getByRole('combobox');

plugins/search/src/components/SearchFilter/SearchFilter.Autocomplete.tsx

@@ -29,29 +29,34 @@ import { SearchFilterComponentProps } from './SearchFilter';
  * @public
  */
 export type SearchAutocompleteFilterProps = SearchFilterComponentProps & {
-  multiple?: boolean;
+  filterSelectedOptions?: boolean;
   limitTags?: number;
+  multiple?: boolean;
 };
 
 export const AutocompleteFilter = (props: SearchAutocompleteFilterProps) => {
   const {
-    asyncValues,
-    asyncDebounce,
     className,
     defaultValue,
-    multiple,
     name,
     values: givenValues,
+    valuesDebounceMs,
     label,
+    filterSelectedOptions,
     limitTags,
+    multiple,
   } = props;
   const [inputValue, setInputValue] = useState<string>('');
   useDefaultFilterValue(name, defaultValue);
+  const asyncValues =
+    typeof givenValues === 'function' ? givenValues : undefined;
+  const defaultValues =
+    typeof givenValues === 'function' ? undefined : givenValues;
   const { value: values, loading } = useAsyncFilterValues(
     asyncValues,
     inputValue,
-    givenValues,
-    asyncDebounce,
+    defaultValues,
+    valuesDebounceMs,
   );
   const { filters, setFilters } = useSearch();
   const filterValue =
@@ -94,12 +99,13 @@ export const AutocompleteFilter = (props: SearchAutocompleteFilterProps) => {
 
   return (
     <Autocomplete
+      filterSelectedOptions={filterSelectedOptions}
+      limitTags={limitTags}
       multiple={multiple}
       className={className}
       id={`${multiple ? 'multi-' : ''}select-filter-${name}--select`}
       options={values || []}
       loading={loading}
-      limitTags={limitTags}
       value={filterValue}
       onChange={handleChange}
       onInputChange={(_, newValue) => setInputValue(newValue)}

plugins/search/src/components/SearchFilter/SearchFilter.stories.tsx

@@ -50,7 +50,8 @@ export const SelectFilter = () => {
   return (
     <Paper style={{ padding: 10 }}>
       <SearchFilter.Select
-        name="Search Select Filter"
+        label="Search Select Filter"
+        name="select_filter"
         values={['value1', 'value2']}
       />
     </Paper>
@@ -61,8 +62,9 @@ export const AsyncSelectFilter = () => {
   return (
     <Paper style={{ padding: 10 }}>
       <SearchFilter.Select
-        name="Asynchronous Values"
-        asyncValues={async () => {
+        label="Asynchronous Values"
+        name="async_values"
+        values={async () => {
           const response = await fetch('https://swapi.dev/api/planets');
           const json: { results: Array<{ name: string }> } =
             await response.json();
@@ -79,7 +81,7 @@ export const Autocomplete = () => {
       <SearchFilter.Autocomplete
         name="autocomplete"
         label="Single-Select Autocomplete Filter"
-        values={['abba', 'cadaver']}
+        values={['value1', 'value2']}
       />
     </Paper>
   );
@@ -92,7 +94,7 @@ export const MultiSelectAutocomplete = () => {
         multiple
         name="autocomplete"
         label="Multi-Select Autocomplete Filter"
-        values={['abba', 'cadaver']}
+        values={['value1', 'value2']}
       />
     </Paper>
   );
@@ -105,10 +107,12 @@ export const AsyncMultiSelectAutocomplete = () => {
         multiple
         name="starwarsPerson"
         label="Starwars Character"
-        asyncValues={async partial => {
+        values={async partial => {
           if (partial === '') return [];
           const response = await fetch(
-            `https://swapi.dev/api/people?search=${partial}`,
+            `https://swapi.dev/api/people?search=${encodeURIComponent(
+              partial,
+            )}`,
           );
           const json: { results: Array<{ name: string }> } =
             await response.json();

plugins/search/src/components/SearchFilter/SearchFilter.test.tsx

@@ -215,7 +215,7 @@ describe('SearchFilter', () => {
           <SearchFilter.Select
             label={label}
             name={name}
-            asyncValues={async () => values}
+            values={async () => values}
           />
         </SearchContextProvider>,
       );

plugins/search/src/components/SearchFilter/SearchFilter.tsx

@@ -46,18 +46,19 @@ export type SearchFilterComponentProps = {
   className?: string;
   name: string;
   label?: string;
-  values?: string[];
   /**
-   * An async function to return a list of values to be used in the filter. In
-   * the autocomplete filter, the last input value is provided as an input to
-   * allow values to be filtered. This function is debounced and values cached.
+   * Either an array of values directly, or an async function to return a list
+   * of values to be used in the filter. In the autocomplete filter, the last
+   * input value is provided as an input to allow values to be filtered. This
+   * function is debounced and values cached.
    */
-  asyncValues?: (partial: string) => Promise<string[]>;
-  /**
-   * Debounce time (ms) used by the asyncValues callback. Defaults to 250ms.
-   */
-  asyncDebounce?: number;
+  values?: string[] | ((partial: string) => Promise<string[]>);
   defaultValue?: string[] | string | null;
+  /**
+   * Debounce time in milliseconds, used when values is an async callback.
+   * Defaults to 250ms.
+   */
+  valuesDebounceMs?: number;
 };
 
 /**
@@ -69,10 +70,27 @@ export type SearchFilterWrapperProps = SearchFilterComponentProps & {
 };
 
 const CheckboxFilter = (props: SearchFilterComponentProps) => {
-  const { className, defaultValue, label, name, values = [] } = props;
+  const {
+    className,
+    defaultValue,
+    label,
+    name,
+    values: givenValues = [],
+    valuesDebounceMs,
+  } = props;
   const classes = useStyles();
   const { filters, setFilters } = useSearch();
   useDefaultFilterValue(name, defaultValue);
+  const asyncValues =
+    typeof givenValues === 'function' ? givenValues : undefined;
+  const defaultValues =
+    typeof givenValues === 'function' ? undefined : givenValues;
+  const { value: values = [], loading } = useAsyncFilterValues(
+    asyncValues,
+    '',
+    defaultValues,
+    valuesDebounceMs,
+  );
 
   const handleChange = (e: ChangeEvent<HTMLInputElement>) => {
     const {
@@ -90,6 +108,7 @@ const CheckboxFilter = (props: SearchFilterComponentProps) => {
   return (
     <FormControl
       className={className}
+      disabled={loading}
       fullWidth
       data-testid="search-checkboxfilter-next"
     >
@@ -117,21 +136,24 @@ const CheckboxFilter = (props: SearchFilterComponentProps) => {
 
 const SelectFilter = (props: SearchFilterComponentProps) => {
   const {
-    asyncValues,
-    asyncDebounce,
     className,
     defaultValue,
     label,
     name,
     values: givenValues,
+    valuesDebounceMs,
   } = props;
   const classes = useStyles();
   useDefaultFilterValue(name, defaultValue);
+  const asyncValues =
+    typeof givenValues === 'function' ? givenValues : undefined;
+  const defaultValues =
+    typeof givenValues === 'function' ? undefined : givenValues;
   const { value: values = [], loading } = useAsyncFilterValues(
     asyncValues,
     '',
-    givenValues,
-    asyncDebounce,
+    defaultValues,
+    valuesDebounceMs,
   );
   const { filters, setFilters } = useSearch();
 

plugins/search/src/components/SearchFilter/hooks.ts

@@ -29,7 +29,7 @@ export const useAsyncFilterValues = (
   defaultValues: string[] = [],
   debounce: number = 250,
 ) => {
-  const valuesMemo = useRef<Record<string, string[]>>({});
+  const valuesMemo = useRef<Record<string, string[] | Promise<string[]>>>({});
   const definiteFn = fn || (() => Promise.resolve([]));
 
   const [state, callback] = useAsyncFn(definiteFn, [inputValue], {
@@ -42,8 +42,10 @@ export const useAsyncFilterValues = (
       // Performance optimization: only invoke the callback once per inputValue
       // for the lifetime of the hook/component.
       if (valuesMemo.current[inputValue] === undefined) {
-        callback(inputValue).then(values => {
+        valuesMemo.current[inputValue] = callback(inputValue).then(values => {
+          // Overrite the value for future immediate returns.
           valuesMemo.current[inputValue] = values;
+          return values;
         });
       }
     },
@@ -59,11 +61,12 @@ export const useAsyncFilterValues = (
     };
   }
 
-  // Immediately return a memoized value if it is set.
-  if (valuesMemo.current[inputValue] !== undefined) {
+  // Immediately return a memoized value if it is set (and not a promise).
+  const possibleValue = valuesMemo.current[inputValue];
+  if (Array.isArray(possibleValue)) {
     return {
       loading: false,
-      value: valuesMemo.current[inputValue],
+      value: possibleValue,
     };
   }
 
@@ -80,11 +83,7 @@ export const useDefaultFilterValue = (
   const { setFilters } = useSearch();
 
   useEffect(() => {
-    const defaultIsEmpty = !defaultValue;
-    const defaultIsEmptyArray =
-      Array.isArray(defaultValue) && defaultValue.length === 0;
-
-    if (!defaultIsEmpty && !defaultIsEmptyArray) {
+    if (defaultValue && [defaultValue].flat().length > 0) {
       setFilters(prevFilters => ({
         ...prevFilters,
         [name]: defaultValue,