[useAutocomplete] groupedOptions type definition seems wrong

[ZachCMP]

Hey, I think I found a mistake in the type definitions for useAutocomplete. Happy to take a crack at fixing it, but I wanted to put an issue up first to make sure it wasn't like that for a reason.

---

• The issue is present in the latest release.
• I have searched the issues of this repository and believe that this is not a duplicate.

Current Behavior 😯

I'm getting these type errors when using groupedOptions from the useAutocomplete hook, when using the actual code in the Autocomplete component that consumes groupedOptions.

TS2339: Property 'key' does not exist on type 'T'.
TS2339: Property 'group' does not exist on type 'T'.
TS2339: Property 'options' does not exist on type 'T'.
TS7006: Parameter 'option2' implicitly has an 'any' type.
TS7006: Parameter 'index2' implicitly has an 'any' type.
TS2339: Property 'index' does not exist on type 'T'.

The type mapping indicates that groupedOptions is T[], but the code in useAutocomplete seems to actually return a different type if groupBy is defined. Here's the relevant code from useAutocomplete:

  let groupedOptions = filteredOptions;
  if (groupBy) {
    // used to keep track of key and indexes in the result array
    const indexBy = new Map();
    let warn = false;

    groupedOptions = filteredOptions.reduce((acc, option, index) => {
      const group = groupBy(option);

      if (acc.length > 0 && acc[acc.length - 1].group === group) {
        acc[acc.length - 1].options.push(option);
      } else {
        if (process.env.NODE_ENV !== 'production') {
          if (indexBy.get(group) && !warn) {
            console.warn(
              `Material-UI: The options provided combined with the \`groupBy\` method of ${componentName} returns duplicated headers.`,
              'You can solve the issue by sorting the options with the output of `groupBy`.',
            );
            warn = true;
          }
          indexBy.set(group, true);
        }

        acc.push({
          key: index,
          index,
          group,
          options: [option],
        });
      }

      return acc;
    }, []);
  }

And the relevant type definition in useAutocomplete.d.ts:

export default function useAutocomplete<
  T,
  Multiple extends boolean | undefined = undefined,
  DisableClearable extends boolean | undefined = undefined,
  FreeSolo extends boolean | undefined = undefined
>(
  props: UseAutocompleteProps<T, Multiple, DisableClearable, FreeSolo>
): {
  ...
  groupedOptions: T[];
};

Expected Behavior 🤔

I shouldn't get type errors here. Ideally the typing should be T[] if groupBy isn't defined, and something like { key: number, index: number, group: string, options: T[] } if it is defined.

Steps to Reproduce 🕹

TS playground was having trouble importing @material-ui/lab, so I just used codesandbox, which was closer to the errors I'm actually getting in my editor.

https://codesandbox.io/s/material-ui-issue-forked-ky88t?file=/src/Demo.tsx

Steps:

1. Observe type errors

Context 🔦

I'm working on a custom autocomplete component that allows more granular control around the behavior of the freeSolo aspect. The stock Autocomplete behavior for that doesn't work for my purposes here, so I'm building something custom to allow the addition of actions as additional options, which would be in their own group. These TS errors appeared when copying some pieces of the stock Autocomplete component into my codebase. Since it's a custom component, I can cast the types manually, so it's not holding me up now; But the type definition seems obviously wrong, no?

Your Environment 🌎

Tech	Version
Material-UI	v4.11.1
React	v17.0.1
Browser	N/A
TypeScript	v4.0.3

[oliviertassinari]

@ZachCMP Thanks for the detailed report. The types look definitely wrong. Would this be enough?

diff --git a/packages/material-ui/src/useAutocomplete/useAutocomplete.d.ts b/packages/material-ui/src/useAutocomplete/useAutocomplete.d.ts
index cec10d8e08..8017cc3996 100644
--- a/packages/material-ui/src/useAutocomplete/useAutocomplete.d.ts
+++ b/packages/material-ui/src/useAutocomplete/useAutocomplete.d.ts
@@ -271,6 +271,14 @@ export type AutocompleteChangeReason =
 export interface AutocompleteChangeDetails<T = string> {
   option: T;
 }
+
+export interface AutocompleteGroupedOption<T = string> {
+  key: number;
+  index: number;
+  group: string;
+  options: T[];
+}
+
 export type AutocompleteCloseReason =
   | 'toggleInput'
   | 'escape'
@@ -311,5 +319,5 @@ export default function useAutocomplete<
   anchorEl: null | HTMLElement;
   setAnchorEl: () => void;
   focusedTag: number;
-  groupedOptions: T[];
+  groupedOptions: AutocompleteGroupedOption<T>[];
 };

[ZachCMP]

@oliviertassinari Thanks for the quick response! Looking at the useAutocomplete.js code, it looks like the returned value for groupedOptions would actually be T[] | AutocompleteGroupedOption<T>[].

That would remove the type errors, but I think it would still require you to type guard when you implement it. It seems like the switch is the presence of a groupBy prop, so I'm thinking the best solution would be to determine the type based on the presence of that prop. Something like:

interface AutocompleteWithNormalOptions<T> {
  groupedOptions: T[]
  groupBy: (option: T) => string
}

interface AutocompleteWithGroupedOptions<T> {
  groupedOptions: AutocompleteGroupedOption<T>[]
  groupBy?: undefined
}

I'm not sure exactly how you'd go about weaving that into the rest of the props here. I've mostly used types and inline typing, so my thought would be something like:

type Props<...args> = UseAutocompleteProps<...args> & (AutocompleteWithNormalOptions<T> | AutocompleteWithGroupedOptions<T>)

But I know that there's a few different ways to do that kind of thing, and I'm not sure which one is most appropriate here. What do you think?

[ZachCMP]

Sorry for the double post, but it occurs to me that another option would be to output filteredOptions from useAutocomplete, and then groupedOptions would always be AutocompleteGroupedOption<T>[]. One big group if no groupBy, otherwise grouped as per normal. Then the types would be:

groupedOptions: AutocompleteGroupedOption<T>[]
filteredOptions: T[]

That might be more in line with what the variable names indicate, and prevents having to do a type check. However, it would also require some slight tweaking of the Autocomplete component. And unfortunately I'm pretty certain that would be a breaking change, so probably never mind 🤦‍♂️.

[oliviertassinari]

Looking at the useAutocomplete.js code, it looks like the returned value for groupedOptions would actually be T[] | AutocompleteGroupedOption[].

@ZachCMP A right, I had forgotten about this. I would be leaning for the "quick win" for a start (the union type), at least we would export AutocompleteGroupedOption. Would it be good enough in your case?

[ZachCMP]

@oliviertassinari Yeah, I think a union type of T[] | AutocompleteGroupedOption<T>[] for groupedOptions would be good enough. Let me know if there's anything I can do to help. Also happy to give it a shot and submit a PR. Whatever's best for you.

[oliviertassinari]

@ZachCMP If you could open a pull request, that would definitely help :)