[Autocomplete] Add index to renderOption's AutocompleteRenderOptionState

[CowDotDev]

• I have followed (at least) the PR section of the contributing guide.

The purpose of this PR is to simply expose the index that renderListOption intakes, coming from the AutcompleteListbox groupedOptions.map rendering. This allows a developer to have an easy, and more performant, way to reference the current option's index inside the renderOption callback method.

The use-case that brought up this issue was during my attempts at adding virtualization to our Autocomplete components. Our autocompletes that need virtualization may already be using renderOptions, or they may simply be using getOptionLabel. Either way, in order to create a single "VirtualizedAutocomplete" wrapper that can accept varying render sources as well as support lists where the rendered options have varying heights we need to be able to reference the index of the option being rendered.

Example Without this PR's Change - Inline Comments Denoting What Would Change

I am omitting the code that defines GetVirtualListbox that handles the dynamic listHeight/itemSize as it does not change with this PR.

function VirtualAutocomplete<
  T,
  Multiple extends boolean = undefined,
  DisableClearable extends boolean = undefined,
  FreeSolo extends boolean = undefined,
>(autocompleteProps: AutocompleteProps<T, Multiple, DisableClearable, FreeSolo>): JSX.Element {
  const virtualListboxRef = useRef<VariableSizeList>(null);
  const itemSizes = useRef<Record<string, number>>({});
  const stringifiedOptions = autocompleteProps.options.map((o) => JSON.stringify(o));

  const getItemSize = (index) => {
    return itemSizes.current[index] || 48;
  };

  const setItemSize = (index, size) => {
    itemSizes.current = { ...itemSizes.current, [index]: size };
    virtualListboxRef.current.resetAfterIndex(0, true);
  };

  const getOptionIndex = (option) => {
    const flatOption = JSON.stringify(option);
    return stringifiedOptions.findIndex((o) => o === flatOption);
  };

  return (
    <VirtualListboxContext.Provider value={{ getItemSize, setItemSize, virtualListboxRef }}>
      <Autocomplete
        {...autocompleteProps}
        renderOption={(props, option, state) => {
          // Everything surrounding this getOptionIndex will be removed and replaced with state.index.
          const index = getOptionIndex(option);
          return typeof autocompleteProps.renderOption === 'function' ? (
            <TrackedOption
              key={`trackedOption-${index}`}
              optionIndex={index}
              renderType="renderOption"
            >
              {autocompleteProps.renderOption(props, option, state)}
            </TrackedOption>
          ) : (
            <TrackedOption
              key={`trackedOption-${index}`}
              optionIndex={index}
              renderType="getOptionLabel"
              renderProps={props}
            >
              {autocompleteProps.getOptionLabel(option)}
            </TrackedOption>
          );
        }}
        ListboxComponent={GetVirtualListbox()}
      />
    </VirtualListboxContext.Provider>
  );
}

While my use-case is specific to Virtualization, and there are other ways to may be more performant to index all the options, I believe this addition is generic enough that other developers will find it useful and comes at no cost as the index was already being supplied to the method that calls renderOption.

[mui-bot]

	Messages
📖	Netlify deploy preview: https://deploy-preview-35578--material-ui.netlify.app/

Details of bundle changes

Generated by 🚫 dangerJS against 26f59be

[ZeeshanTamboli]

Won't option.id work for you? You can even get the option's index with data-option-index attribute (props['data-option-index']).

[CowDotDev]

@ZeeshanTamboli
No, option.id does not give me the index of the option being rendered. It only gives me an identifier, so I would still have to loop through the options array to find the index. This is also under the assumption our options array is always an array of objects with an id property, which isn't the case when we are using a simplified options array of strings.

(Initially misunderstood your prop option, updated response) I would have to look at that, but definitely could work for grabbing the index if it is on the props object from renderOption. This option feels a little brittle/dirty to be honest though. It seems pretty standard to expose the index when exposing a callback method when working with an array, without having to dig through props meant for the DOM.

[CowDotDev]

@ZeeshanTamboli - I reviewed your suggestion about props['data-option-index'] and it definitely does work as an alternative.

That being said I do think explicitly exposing the index as part of renderOption's state arg is a beneficial addition considering the API docs specify props as props: The props to apply on the li element.. For functional information, I don't believe we should be reaching into a DOM specific object that is loosely typed (the data-option-index is not explicitly a property on props, as it is typed as HTMLAttributes<HTMLLIElement>); for the same reason inputValue and selected live in state and not props.

[ZeeshanTamboli]

@CowDotDev Your reasoning sounds good to me. It's fine from my side. The code looks good. However, I would still like other team member to take the final decision. @michaldudak Can you take a look?

[michaldudak]

Yeah, I agree that getting data from a DOM attribute doesn't feel great. Let's include the index as you proposed. Thanks for the PR!