Downshift: [useSelect] memoizing getter functions return error

[jorgemoya]

• downshift version: 5.4.2

Relevant code or config

https://codesandbox.io/s/useselect-usage-d4jpm?file=/index.js

What you did:

This example is a simplified version of my code. I'm memoizing part of my component to prevent excessive rendering. I pass in a reference object to my getToggleButtons getter function.

What happened:
Before 5.4.x I believe it worked fine but now I keep getting the downshift: You forgot to call the getToggleButtonProps getter function on your component / element. error message.

Problem description:

I believe the issue is because getToggleButtons is never called again because renderToggle is memoized. Or maybe the issue is that getToggleButtons is expected to always be called? Did it work before because the reference of the getToggleButtons function was always different and thus it would rerender anyway? Theoretically, I want the toggle button to only rerender if a prop has changed.

I get the same problem when using useCombobox but for my getMenuProps getter function when it lives under a memo.

Suggested solution:

One quick solution is to stop memoizing since it will always rerender and call getToggleButtons again and again (even when it may no be necessary). Is this the expected thing to do?

[silviuaavram]

Yeah ... we should improve this, it's a use case that conflicts with the memoization.

I still want these errors, people complaining in https://github.com/downshift-js/downshift/pull/1053 that they do not render the menu and get the error, but they should render the menu even when it's closed. That's the accessible way.

All things considered, I am in favour in having memoization, as it improves performance. But I also want these warnings, so let's see how we can stop throwing them when the functions are memoized.

[rnnyrk]

Hmm I think my issue is related to what you mention in your comment @silviuaavram. I keep getting the error downshift: You forgot to call the getMenuProps getter function on your component / element. for my MultiSelect and ComboSelect inputs.

This is for example my ComboBox in which I render getMenuProps() as soon as isOpen is true. But as I understand, we should try to always render the getMenuProps()? I've tried that, but the behaviour is not as I would like to see it then..

Any recommendations?

export const ComboSelect: React.FC<i.ComboSelectType> = ({
  name, label, register, error, size, items, onChange, defaultSelected,
  noFieldset, searchInputItems, resetCombo, onFilterItems, onAdd, ...props
}) => {
  const [inputItems, setInputItems] = React.useState<i.SelectItemsType[]>(items);
  const formMethods = useFormContext();

  const filterItems = (items: i.SelectItemsType[], inputValue?: string) => {
    if (onFilterItems && typeof inputValue !== 'undefined') {
      onFilterItems(inputValue);
      return items;
    } else {
      const filteredItems = items?.filter((item) => {
        if (!inputValue) return false;
        return item.value.toLowerCase().includes(inputValue.toLowerCase());
      });

      return inputValue ? filteredItems : items;
    }
  };

  const setFormValues = (name: string, value: string | undefined) => {
    formMethods.setValue(name, value);
    formMethods.trigger(name);
  };

  const {
    isOpen,
    getToggleButtonProps,
    getMenuProps,
    getInputProps,
    getComboboxProps,
    highlightedIndex,
    getItemProps,
    selectItem,
  } = useCombobox({
    items: filterItems(inputItems),
    defaultSelectedItem: defaultSelected,
    defaultInputValue: defaultSelected?.value,
    onInputValueChange: ({ inputValue }) => {
      setInputItems(filterItems(items, inputValue));

      if (!inputValue) {
        selectItem(undefined);
        setFormValues(name, undefined);
      }
    },
    itemToString: (item: i.SelectItemsType) => {
      return item?.value || '';
    },
    onSelectedItemChange: (changes) => {
      if (formMethods && !onChange && changes?.selectedItem) {
        setFormValues(name, changes.selectedItem.key);
      }
    },
  });

  React.useEffect(() => {
    setInputItems(items);
  }, [items]);

  return (
    <Fieldset {...{ name, label, error, size, noFieldset }}>
      <DownshiftWrapper>
        <SelectWrapper
          {...getToggleButtonProps()}
          {...getComboboxProps()}
        >
          <StyledSelectInput
            {...getInputProps()}
            {...{ name }}
            id={name}
            error={!!error}
            {...props}
          />
          <IconWrapper>
            <SearchIcon />
          </IconWrapper>
        </SelectWrapper>

        {isOpen && (
          inputItems.length > 0 && (
            <SelectList {...getMenuProps()}>
              {inputItems.map((item: i.SelectItemsType, index: number) => {
                return (
                  <SelectItem
                    style={
                      highlightedIndex === index ? {
                        color: theme.colors.blue,
                        backgroundColor: theme.colors.blue.off,
                      } : {}
                    }
                    key={item.key}
                    {...getItemProps({
                      item,
                      index,
                    })}
                  >
                    {item.value}
                  </SelectItem>
                );
              })}
            </SelectList>
          )
        )}
      </DownshiftWrapper>
    </Fieldset>
  );
};

[elramus]

Same issue here. downshift: You forgot to call the getMenuProps getter function on your component / element. @rnnyrk are you using Next.js by any chance? I am for this project, and yesterday I updated from 9.3 to 9.4 and suddenly this stopped working and I started seeing this message.

[rnnyrk]

@elramus No the project I'm working on is not serverside rendered, so I don't think the issue is related to NextJS. Think it simply has to do with not rendering the getMenuProps element when isOpen is false. But like I said; the behaviour is not the same when I do that

[silviuaavram]

I am suggesting to change:

        {isOpen && (
          inputItems.length > 0 && (
            <SelectList {...getMenuProps()}>
               {inputItems.map((item: i.SelectItemsType, index: number) => {

to

      <SelectList {...getMenuProps()}>
        {isOpen &&
          inputItems.length > 0 &&
            inputItems.map((item: i.SelectItemsType, index: number) => {

[elramus]

Thanks @silviuaavram, that cleared up my error. The Next.js thing was unrelated.

[silviuaavram]

Yw! @elramus @rnnyrk this is not a hack to get it working. It's what ARIA specifies in the combobox pattern. Even if you have the menu open or closed, the <ul> or whatever it is that you use as menu container needs to be present in the DOM.

[silviuaavram]

:tada: This issue has been resolved in version 5.4.6 :tada:

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot :package::rocket:

[nicgirault]

I found this issue while wondering why I get this message You forgot to call the getMenuProps getter function on your component / element. It successfully makes me improve the accessibility of my component but the message is misleading because I didn't forget to call the function. However I was not displaying the menu when closed. Message suggestion: getMenuProps should be called (even when menu is closed)