carbon-design-system / carbon

A design system built by IBM
https://www.carbondesignsystem.com
Apache License 2.0
7.75k stars 1.8k forks source link

[Bug]: Performance Issue with Filterable Multiselect in Carbon React #17028

Closed lharrison13 closed 1 month ago

lharrison13 commented 2 months ago

Package

@carbon/react

Browser

No response

Package version

1.60.2

React version

16.14.0

Description

After switching from carbon v10 to carbon v11 we noticed a significant performance decrease on a page with multiple filterable multi selects that contain large amounts of items (in the 1000s). We noticed that in carbon v10 you sort and filter only if the dropdown is open but in v11 you are sorting and filtering on every re-render since it is done in the functional component's body. If i have 1000 items in the dropdown you will be sorting all of them every re-render without a filter.

Carbon v11 (sort and filter on every re-render) https://github.com/carbon-design-system/carbon/blob/351525d94b31a35760ab7d7e57747c[…]ages/react/src/components/MultiSelect/FilterableMultiSelect.tsx

Carbon v10 (sort and filter only if the dropdown is open) https://github.com/carbon-design-system/carbon/blob/25d31a6cc11ee3a252cb6ae2715ada2da5ed5b38/packages/react/src/components/MultiSelect/FilterableMultiSelect.js#L592

Note: This is a separate issue from the performance of searching on filterable multi selects with hundreds of items.

Temporary Fix: If you are experiencing this issue you can simply add the following prop to you filterable multi select

<FilterableMultiSelect
sortItems={(items) => items}
/>

This will essentially disable the filtering of the multi select items which you could perform yourself separately.

Reproduction/example

https://stackblitz.com/edit/github-dcqkrj?file=src%2FApp.jsx

Steps to reproduce

See stack blitz:

If you click the increment counter you will notice it takes a long time to re-render. If you remove sorting by uncommenting out the sort prop you will see it properly re-render.

Suggested Severity

Severity 3 = User can complete task, and/or has a workaround within the user experience of a given component.

Application/PAL

No response

Code of Conduct

tay1orjones commented 2 months ago

Thanks for opening this and the additional thoughts in the initial slack discussion

We could put it behind isOpen again, but it might be even better to cover all the bases by memoizing it with useMemo.

lharrison13 commented 2 months ago

I have been playing around with this locally and it seems to work when I memoize sortedItems. I ran into a tiny little issue with the itemToString and compareItems props. If you declare them as arrow functions in the props directly like this:

return(
<FilterableMultiSelect
        id="carbon-multiselect-example-3"
        titleText="Multiselect title"
        helperText="This is helper text"
        items={items}
        itemToString={(item) => (item ? item.text : '')}  // <---- This line
        selectionFeedback="top-after-reopen"
        {...args}
      />)

It will create a brand new function every re-render which useMemo will recognize as a dependency change and will re-calculate. So you have to declare these function using useCallback like so:

const itemToString = useCallback((item) => (item ? item.text : ''), []); // <--- this will stop it from being re-created each re-render
return(
<FilterableMultiSelect
        id="carbon-multiselect-example-3"
        titleText="Multiselect title"
        helperText="This is helper text"
        items={items}
        itemToString={itemToString}  // <-- passed in here
        selectionFeedback="top-after-reopen"
        {...args}
      />)