Unpredictable behavior with MultiSelectCombobox

[jalexy12]

Hey all,

Writing because I had a couple of questions about the Multiselect Combobox.

We tend to use quite a few of these in Segment, and I've found it behaves a bit unpredictably and is reliant on stable references to not behave incorrectly.

I created a Codesandbox here: https://codesandbox.io/s/bold-shadow-9rstqc?file=/src/index.tsx

As you can see here, and in the examples for the Combobox in the docs - it's required that the filteredItems (and I believe the selected items callback handler?) are required to be stable references, typically with useMemo.

A couple of things about this:

1. In the doc examples this is fairly simple to execute because the data is static and is defined outside of the component, creating a stable reference. In a realistic scenario, the data is likely fetched from an external data source asynchronously, loaded later, and passed down as a prop. This makes it much easier to create an unstable reference, especially after passing the data through to other components, filtering/modifying the data, etc.
2. From the old react docs (as far as I know - useMemo hasn't changed in this regard):

You may rely on useMemo as a performance optimization, not as a semantic guarantee. In the future, React may choose to “forget” some previously memoized values and recalculate them on next render, e.g. to free memory for offscreen components. Write your code so that it still works without useMemo — and then add it to optimize performance.

It feels a bit unsafe to depend on the above for the component to work correctly
3. It creates a lot of cruft and pointless code, defensive code, opens space for bugs. Ideally, its best to avoid over-memoizing things or making components depend on memoization because you quickly run into a chain of memoizing X, Y, and Z because A depends on them being memoized. It also leads to less than ideal solutions like JSON.stringifying a list of objects in the memo dependency array.

My questions: Have others encountered similar issues? Is there a reason that it's this way, and is there a plan to change it in the future? I'm feeling like I and my team have run into a couple issues already with this component in the migration from evergreen to paste.

Thanks!

[TwilioPasteBot]

Disclaimer: This is a very experimental bot using OpenAI's GPT-4. The answers may not be correct, a human will review the answer and update it if necessary.

---

Hello!

Thank you for your detailed questions and observations about the Multiselect Combobox.

The useMemo hook is indeed used in the examples to ensure that the filteredItems reference remains stable. This is important because the Combobox components are designed to be highly performant and avoid unnecessary re-renders.

However, you're correct that in real-world scenarios, data is often fetched asynchronously and passed down as props, which can lead to unstable references.

Regarding your concerns about relying on useMemo as a semantic guarantee, it's important to note that the Paste design system's Combobox components are built on top of the Downshift library. This library makes certain assumptions about the stability of the props it receives. Therefore, while React's documentation does state that useMemo should not be relied upon as a semantic guarantee, in this specific context, it's used to align with the requirements of the underlying library.

As for your third point, we understand that overuse of memoization can lead to unnecessary complexity and potential performance issues. However, in this case, the memoization is necessary to ensure the correct functioning of the Combobox components.

To your questions:

1. Yes, others have encountered similar issues, especially when dealing with asynchronously fetched data.
2. The reason for this design choice is primarily due to the underlying Downshift library's assumptions about prop stability.
3. As for future changes, the Paste design system is always evolving, and feedback like yours is invaluable in guiding that evolution.

Here's a simplified example of how you might handle asynchronously fetched data with the Multiselect Combobox:

import {MultiselectCombobox} from '@twilio-paste/core/combobox';
import React, { useState, useEffect, useMemo } from 'react';

const MyComponent = () => {
  const [items, setItems] = useState([]);
  const [inputValue, setInputValue] = useState('');
  
  useEffect(() => {
    // Fetch data here and update items
    // For example:
    fetchItems().then(fetchedItems => setItems(fetchedItems));
  }, []);

  const getFilteredItems = (inputValue, items) => {
    const lowerCasedInputValue = inputValue.toLowerCase();
    return items.filter(item => item.toLowerCase().includes(lowerCasedInputValue));
  };

  const filteredItems = useMemo(() => getFilteredItems(inputValue, items), [inputValue, items]);

  return (
    <MultiselectCombobox
      labelText="

---

Here are some similar discussions:

- [Performance issues with Combobox and large data sets.](https://github.com/twilio-labs/paste/discussions/841) (updated: 2023-05-19T17:39:20Z, similarity score: 0.86)
- [Combobox component strange behaviour](https://github.com/twilio-labs/paste/discussions/2716) (updated: 2022-10-10T22:19:33Z, similarity score: 0.86)
- [Multiselect Combobox issue](https://github.com/twilio-labs/paste/discussions/2994) (updated: 2023-09-15T16:48:19Z, similarity score: 0.86)

[TheSisb]

Hi @jalexy12,

This is the first time it has been brought up, so it could possibly be an issue or something to improve, or at least document better. Your codesandbox shows two working examples, so I'm unable to see the repro. Any chance you can implement the repro?

[jalexy12]

Just to be clearer, the first example (the first combobox on the page) doesn't work when unselecting items.

Try selecting an item, and then unselecting - it will add an additional pill with the item in it instead of unselecting in the unmemoized version.

[nkrantz]

Hey @jalexy12 - looks like this reply got buried. I'll take a look today.

[SiTaggart]

Hi @jalexy12

Took a look at this today. There is a decent amount of usage across the company, and no one has raised this as a possible concern so far.

We don't really do anything with the items passed to the component, we basically just directly pass it on to Downshift which is the third part underneath the implementation. There's not a bunch we can do with that regard, you're just using Downshift as it's intended.

As @TheSisb mentions, there may need to be some improved docs to call this out, but to reiterate, it's widely used across Twilio, Console in particular, without any issues to date.

Thanks for pointing it out, though, we'll keep our ears open to see if any trends occur.