refactor: replace react-window usage for tanstack/virtual (#2034)

* refactor: migrate combobox list to tanstack virtual * refactor: migrate multiple combobox list to tanstack virtual * chore: remove react-window * fix: failing test due to waitfor timeout
getlago · Feb 19, 2025 · 0feb76a · 0feb76a
1 parent f0ca914
commit 0feb76a
Show file tree

Hide file tree

Showing 7 changed files with 162 additions and 157 deletions.
diff --git a/package.json b/package.json
@@ -59,7 +59,6 @@
     "@types/react": "18.2.38",
     "@types/react-dom": "18.2.17",
     "@types/react-router-dom": "5.3.3",
-    "@types/react-window": "1.8.5",
     "@types/sanitize-html": "2.9.5",
     "@vitejs/plugin-react-swc": "^3.7.2",
     "auto-changelog": "2.4.0",
@@ -123,7 +122,6 @@
     "react-ace": "^11.0.1",
     "react-dom": "18.2.0",
     "react-router-dom": "6.15.0",
-    "react-window": "1.8.11",
     "recharts": "^2.9.0",
     "sanitize-html": "2.12.1",
     "styled-components": "^6.1.13",

diff --git a/src/components/customers/__tests__/EditCustomerVatRateDialog.test.tsx b/src/components/customers/__tests__/EditCustomerVatRateDialog.test.tsx
@@ -1,4 +1,4 @@
-import { act, cleanup, screen, waitFor } from '@testing-library/react'
+import { act, cleanup, screen } from '@testing-library/react'
 import userEvent from '@testing-library/user-event'
 
 import { EditCustomerVatRateDialog } from '~/components/customers/EditCustomerVatRateDialog'
@@ -75,19 +75,19 @@ describe('EditCustomerVatRateDialog', () => {
   it('should propose to create a new tax if none exists and have permissions', async () => {
     await prepare()
 
-    await waitFor(() =>
-      userEvent.click(
+    userEvent
+      .click(
         screen
           .queryByTestId('edit-customer-vat-rate-dialog')
           ?.querySelector(
             `.${SEARCH_TAX_INPUT_FOR_CUSTOMER_CLASSNAME} .${MUI_INPUT_BASE_ROOT_CLASSNAME}`,
           ) as HTMLElement,
-      ),
-    )
-
-    expect(screen.queryByTestId('combobox-item-Create a tax_rate')).toBeInTheDocument()
-    expect(
-      screen.queryByTestId('combobox-item-Create a tax_rate')?.querySelector(`a`),
-    ).toHaveAttribute('href', CREATE_TAX_ROUTE)
+      )
+      .then(() => {
+        expect(screen.queryByTestId('combobox-item-Create a tax_rate')).toBeInTheDocument()
+        expect(
+          screen.queryByTestId('combobox-item-Create a tax_rate')?.querySelector(`a`),
+        ).toHaveAttribute('href', CREATE_TAX_ROUTE)
+      })
   })
 })
diff --git a/src/components/designSystem/__tests__/ButtonLink.test.tsx b/src/components/designSystem/__tests__/ButtonLink.test.tsx
@@ -1,4 +1,4 @@
-import { act, cleanup, screen, waitFor } from '@testing-library/react'
+import { act, cleanup, screen } from '@testing-library/react'
 import userEvent from '@testing-library/user-event'
 
 import { render } from '~/test-utils'
@@ -71,11 +71,9 @@ describe('ButtonLink', () => {
 
       expect(onContinueMock).not.toHaveBeenCalled()
 
-      await waitFor(() =>
-        userEvent.click(screen.queryByTestId('button-link-button') as HTMLElement),
-      )
-
-      expect(onContinueMock).toHaveBeenCalled()
+      userEvent.click(screen.queryByTestId('button-link-button') as HTMLElement).then(() => {
+        expect(onContinueMock).toHaveBeenCalled()
+      })
     })
 
     it('should not trigger the confirm action on click if button is disabled', async () => {

diff --git a/src/components/form/ComboBox/ComboBoxVirtualizedList.tsx b/src/components/form/ComboBox/ComboBoxVirtualizedList.tsx
@@ -1,5 +1,5 @@
+import { useVirtualizer } from '@tanstack/react-virtual'
 import { ReactElement, useEffect, useRef } from 'react'
-import { VariableSizeList } from 'react-window'
 
 import { ITEM_HEIGHT } from '~/styles'
 import { tw } from '~/styles/utils'
@@ -9,87 +9,90 @@ import { ComboBoxProps } from './types'
 export const GROUP_ITEM_KEY = 'combobox-group-by'
 export const GROUP_HEADER_HEIGHT = 44
 
-function useResetCache(itemCount: number) {
-  const ref = useRef<VariableSizeList>(null)
-
-  useEffect(() => {
-    if (ref.current) {
-      ref.current.resetAfterIndex(0, true)
-    }
-  }, [itemCount])
-  return ref
-}
-
 type ComboBoxVirtualizedListProps = {
   elements: ReactElement[]
 } & Pick<ComboBoxProps, 'value'>
 
-export const ComboBoxVirtualizedList = (props: ComboBoxVirtualizedListProps) => {
-  const { elements, value } = props
-
+export const ComboBoxVirtualizedList = ({ elements, value }: ComboBoxVirtualizedListProps) => {
   const itemCount = elements?.length
+  const parentRef = useRef<HTMLDivElement>(null)
 
   const getHeight = () => {
     const hasAnyGroupHeader = elements.some((el) => (el.key as string).includes(GROUP_ITEM_KEY))
     const hasDescription = elements.some(
       (el) => (el.props?.children?.props?.option?.description as string)?.length > 0,
     )
 
-    const itemheightDelta = hasDescription ? 8 : 4
+    const itemHeightDelta = hasDescription ? 8 : 4
 
     // recommended perf best practice
     if (itemCount > 5) {
-      return 5 * (ITEM_HEIGHT + itemheightDelta) + 4 // Add 4px for margins
+      return 5 * (ITEM_HEIGHT + itemHeightDelta) + 4 // Add 4px for margins
     } else if (itemCount <= 2 && hasAnyGroupHeader) {
       return itemCount * (ITEM_HEIGHT + 2) // Add 2px for margins
     } else if (itemCount <= 2 && hasDescription) {
-      return itemCount * (ITEM_HEIGHT + itemheightDelta) + 4 // Add 4px for margins
+      return itemCount * (ITEM_HEIGHT + itemHeightDelta) + 4 // Add 4px for margins
+    }
+    return itemCount * (ITEM_HEIGHT + itemHeightDelta) + 4 // Add 4px for margins
+  }
+
+  const getItemHeight = (index: number) => {
+    const element = elements[index]
+
+    if ((element.key as string)?.includes(GROUP_ITEM_KEY)) {
+      return GROUP_HEADER_HEIGHT + (index === 0 ? 2 : 6)
+    }
+
+    if (index === itemCount - 1) {
+      return ITEM_HEIGHT
     }
-    return itemCount * (ITEM_HEIGHT + itemheightDelta) + 4 // Add 4px for margins
+
+    return ITEM_HEIGHT + 8
   }
 
-  // reset the `VariableSizeList` cache if data gets updated
-  const gridRef = useResetCache(itemCount)
-
-  // when value gets updated, ensure we tell <VariableSizeList>
-  // to scroll to the selected option
-  useEffect(
-    () => {
-      if (gridRef && value && gridRef.current) {
-        const valueIndex = elements.findIndex(
-          (el) => el.props?.children?.props?.option?.value === value,
-        )
-
-        if (valueIndex) {
-          gridRef.current?.scrollToItem(valueIndex, 'smart')
-        }
-      }
-    },
+  const rowVirtualizer = useVirtualizer({
+    count: elements.length,
+    getScrollElement: () => parentRef.current,
+    estimateSize: getItemHeight,
+    overscan: 5,
+  })
+
+  useEffect(() => {
+    const index = elements.findIndex((el) => el.props?.children?.props?.option?.value === value)
+
+    if (index !== -1) {
+      rowVirtualizer.scrollToIndex(index, { align: 'start' })
+    }
     // eslint-disable-next-line react-hooks/exhaustive-deps
-    [value],
-  )
+  }, [value])
 
   return (
-    <VariableSizeList
-      className={tw({
-        'mb-1': itemCount > 1,
-      })}
-      itemData={elements}
-      height={getHeight()}
-      width="100%"
-      ref={gridRef}
-      innerElementType="div"
-      itemSize={(index) => {
-        return index === itemCount - 1
-          ? ITEM_HEIGHT
-          : ((elements[index].key as string) || '').includes(GROUP_ITEM_KEY)
-            ? GROUP_HEADER_HEIGHT + (index === 0 ? 2 : 6)
-            : ITEM_HEIGHT + 8
-      }}
-      overscanCount={5}
-      itemCount={itemCount}
+    <div
+      ref={parentRef}
+      className={tw({ 'mb-1': elements.length > 1 })}
+      style={{ height: getHeight(), width: '100%', overflow: 'auto' }}
     >
-      {({ style, index }) => <div style={style}>{elements[index]}</div>}
-    </VariableSizeList>
+      <div
+        className="relative w-full"
+        style={{
+          height: rowVirtualizer.getTotalSize(),
+        }}
+      >
+        {rowVirtualizer.getVirtualItems().map((virtualRow) => (
+          <div
+            key={virtualRow.key}
+            ref={rowVirtualizer.measureElement}
+            className="absolute left-0 top-0 w-full"
+            style={{
+              height: `${virtualRow.size}px`,
+              // Use the translate property for performance reasons
+              transform: `translateY(${virtualRow.start}px)`,
+            }}
+          >
+            {elements[virtualRow.index]}
+          </div>
+        ))}
+      </div>
+    </div>
   )
 }
diff --git a/src/components/form/ComboBox/ComboboxList.tsx b/src/components/form/ComboBox/ComboboxList.tsx
@@ -120,13 +120,7 @@ export const ComboboxList = forwardRef(
     }, [isGrouped, renderGroupHeader, children, propsToForward, virtualized])
 
     return (
-      <div
-        className={tw('relative max-h-[inherit] overflow-auto pb-0', {
-          'overflow-hidden': virtualized,
-        })}
-        ref={ref}
-        role="listbox"
-      >
+      <div className="relative max-h-[inherit] overflow-auto pb-0" ref={ref} role="listbox">
         {virtualized ? (
           <ComboBoxVirtualizedList value={value} elements={htmlItems as ReactElement[]} />
         ) : (

diff --git a/src/components/form/MultipleComboBox/MultipleComboBoxVirtualizedList.tsx b/src/components/form/MultipleComboBox/MultipleComboBoxVirtualizedList.tsx
@@ -1,5 +1,5 @@
+import { useVirtualizer } from '@tanstack/react-virtual'
 import { ReactElement, useEffect, useRef } from 'react'
-import { VariableSizeList } from 'react-window'
 
 import { ITEM_HEIGHT } from '~/styles'
 import { tw } from '~/styles/utils'
@@ -9,24 +9,16 @@ import { MultipleComboBoxProps } from './types'
 export const MULTIPLE_GROUP_ITEM_KEY = 'multiple-comboBox-group-by'
 export const GROUP_HEADER_HEIGHT = 44
 
-function useResetCache(itemCount: number) {
-  const ref = useRef<VariableSizeList>(null)
-
-  useEffect(() => {
-    if (ref.current) {
-      ref.current.resetAfterIndex(0, true)
-    }
-  }, [itemCount])
-  return ref
-}
-
 type MultipleComboBoxVirtualizedListProps = {
   elements: ReactElement[]
 } & Pick<MultipleComboBoxProps, 'value'>
 
-export const MultipleComboBoxVirtualizedList = (props: MultipleComboBoxVirtualizedListProps) => {
-  const { elements, value } = props
+export const MultipleComboBoxVirtualizedList = ({
+  elements,
+  value,
+}: MultipleComboBoxVirtualizedListProps) => {
   const itemCount = elements?.length
+  const parentRef = useRef<HTMLDivElement>(null)
 
   const getHeight = () => {
     const hasAnyGroupHeader = elements.some((el) =>
@@ -42,48 +34,63 @@ export const MultipleComboBoxVirtualizedList = (props: MultipleComboBoxVirtualiz
     return itemCount * (ITEM_HEIGHT + 8) + 4 // Add 4px for margins
   }
 
-  // reset the `VariableSizeList` cache if data gets updated
-  const gridRef = useResetCache(itemCount)
+  const getItemHeight = (index: number) => {
+    const element = elements[index]
+
+    if ((element.key as string)?.includes(MULTIPLE_GROUP_ITEM_KEY)) {
+      return GROUP_HEADER_HEIGHT + (index === 0 ? 2 : 6)
+    }
 
-  // when value gets updated, ensure we tell <VariableSizeList>
-  // to scroll to the selected option
-  useEffect(
-    () => {
-      if (gridRef && value && gridRef.current) {
-        const valueIndex = elements.findIndex(
-          (el) => el.props?.children?.props?.option?.value === value,
-        )
+    if (index === itemCount - 1) {
+      return ITEM_HEIGHT
+    }
 
-        if (valueIndex) {
-          gridRef.current?.scrollToItem(valueIndex, 'smart')
-        }
-      }
-    },
+    return ITEM_HEIGHT + 8
+  }
+
+  const rowVirtualizer = useVirtualizer({
+    count: elements.length,
+    getScrollElement: () => parentRef.current,
+    estimateSize: getItemHeight,
+    overscan: 5,
+  })
+
+  useEffect(() => {
+    const index = elements.findIndex((el) => el.props?.children?.props?.option?.value === value)
+
+    if (index !== -1) {
+      rowVirtualizer.scrollToIndex(index, { align: 'start' })
+    }
     // eslint-disable-next-line react-hooks/exhaustive-deps
-    [value],
-  )
+  }, [value])
 
   return (
-    <VariableSizeList
-      className={tw({
-        'mb-1': itemCount > 1,
-      })}
-      itemData={elements}
-      height={getHeight()}
-      width="100%"
-      ref={gridRef}
-      innerElementType="div"
-      itemSize={(index) => {
-        return index === itemCount - 1
-          ? ITEM_HEIGHT
-          : ((elements[index].key as string) || '').includes(MULTIPLE_GROUP_ITEM_KEY)
-            ? GROUP_HEADER_HEIGHT + (index === 0 ? 2 : 6)
-            : ITEM_HEIGHT + 8
-      }}
-      overscanCount={5}
-      itemCount={itemCount}
+    <div
+      ref={parentRef}
+      className={tw({ 'mb-1': elements.length > 1 })}
+      style={{ height: getHeight(), width: '100%', overflow: 'auto' }}
     >
-      {({ style, index }) => <div style={style}>{elements[index]}</div>}
-    </VariableSizeList>
+      <div
+        className="relative w-full"
+        style={{
+          height: rowVirtualizer.getTotalSize(),
+        }}
+      >
+        {rowVirtualizer.getVirtualItems().map((virtualRow) => (
+          <div
+            key={virtualRow.key}
+            ref={rowVirtualizer.measureElement}
+            className="absolute left-0 top-0 w-full"
+            style={{
+              height: `${virtualRow.size}px`,
+              // Use the translate property for performance reasons
+              transform: `translateY(${virtualRow.start}px)`,
+            }}
+          >
+            {elements[virtualRow.index]}
+          </div>
+        ))}
+      </div>
+    </div>
   )
 }