refactor(suite/components): improve typing and performance of the Vir…

…tualizedList (#16592) * refactor(suite): fix typing of Virtualized component and improve perf by memoizing VirtualizedList while mantaining type safety * refactor(suite): remove needless state from VirtualizedList * refactor(suite): leverage memo() for memoizing expensive items list in VirtualizedList component * feat(components): set up react testing
trezor · Jan 27, 2025 · 307c075 · 307c075
1 parent ba58aa5
commit 307c075
Show file tree

Hide file tree

Showing 9 changed files with 363 additions and 116 deletions.
diff --git a/packages/components/jest.config.js b/packages/components/jest.config.js
@@ -0,0 +1,7 @@
+const baseConfig = require('../../jest.config.base');
+
+module.exports = {
+    ...baseConfig,
+    setupFilesAfterEnv: ['<rootDir>/jest.setup.js'],
+    testEnvironment: 'jsdom',
+};
diff --git a/packages/components/jest.setup.js b/packages/components/jest.setup.js
@@ -0,0 +1 @@
+import '@testing-library/jest-dom';
diff --git a/packages/components/package.json b/packages/components/package.json
@@ -13,7 +13,9 @@
         "type-check": "yarn g:tsc --build tsconfig.json",
         "type-check:watch": "yarn type-check -- --watch",
         "storybook": "storybook dev -p 9003 -c .storybook",
-        "storybook-build": "storybook build -c .storybook -o .build-storybook"
+        "storybook-build": "storybook build -c .storybook -o .build-storybook",
+        "test:unit": "yarn g:jest",
+        "test-unit:watch": "yarn g:jest -o --watch"
     },
     "dependencies": {
         "@floating-ui/react": "^0.26.9",
@@ -22,6 +24,7 @@
         "@suite-common/suite-constants": "workspace:*",
         "@suite-common/suite-utils": "workspace:*",
         "@suite-common/validators": "workspace:*",
+        "@testing-library/jest-dom": "^6.6.3",
         "@trezor/asset-utils": "workspace:*",
         "@trezor/connect": "workspace:*",
         "@trezor/dom-utils": "workspace:*",
@@ -55,6 +58,7 @@
         "@storybook/react": "^7.6.13",
         "@storybook/react-webpack5": "^7.6.13",
         "@storybook/theming": "^7.6.13",
+        "@testing-library/react": "14.2.1",
         "@trezor/eslint": "workspace:*",
         "@types/react": "18.2.79",
         "@types/react-date-range": "^1.4.9",

diff --git a/packages/components/src/components/VirtualizedList/VirtualizedList.tsx b/packages/components/src/components/VirtualizedList/VirtualizedList.tsx
@@ -1,4 +1,4 @@
-import React, { useState, useEffect, useCallback, forwardRef, useRef } from 'react';
+import React, { useState, useEffect, useCallback, useRef, memo, forwardRef, useMemo } from 'react';
 
 import styled from 'styled-components';
 
@@ -48,133 +48,179 @@ const Item = styled.div`
     width: 100%;
 `;
 
-const calculateItemHeight = <T extends { height: number }>(item: T): number => item.height;
+type BaseItemProps = {
+    height: number;
+};
+
+const calculateItemHeight = <T extends BaseItemProps>(item: T): number => item.height;
+
+interface ListContainerProps<T extends BaseItemProps> {
+    listHeight: number | string;
+    listMinHeight: number | string;
+    totalHeight: number;
+    items: Array<T>;
+    itemHeights: Array<number>;
+    startIndex: number;
+    endIndex: number;
+    ref?: React.Ref<HTMLDivElement>; // NOTE: needs to be here due to typecasting due to forwardRef
+    renderItem: (item: T, index: number) => React.ReactNode;
+}
+
+function ListContainerComponent<T extends BaseItemProps>(
+    {
+        listHeight,
+        listMinHeight,
+        totalHeight,
+        items,
+        itemHeights,
+        startIndex,
+        endIndex,
+        renderItem,
+    }: ListContainerProps<T>,
+    ref: React.Ref<HTMLDivElement>,
+) {
+    return (
+        <Container ref={ref} $height={listHeight} $minHeight={listMinHeight}>
+            <Content style={{ height: `${totalHeight}px` }}>
+                {itemHeights.slice(startIndex, endIndex).map((height, index) => {
+                    const itemIndex = startIndex + index;
+                    const itemTop = itemHeights.slice(0, itemIndex).reduce((acc, h) => acc + h, 0);
+
+                    if (!items[itemIndex]) return null;
+
+                    return (
+                        <Item
+                            key={itemIndex}
+                            style={{
+                                top: `${itemTop}px`,
+                                height,
+                            }}
+                        >
+                            {renderItem(items[itemIndex], itemIndex)}
+                        </Item>
+                    );
+                })}
+            </Content>
+        </Container>
+    );
+}
 
-type VirtualizedListProps<T> = {
-    items: Array<T & { height: number }>;
+// NOTE: don't forget the forwardRef() because of passing the ref for useShadow()
+const ListContainer = memo(forwardRef(ListContainerComponent)) as typeof ListContainerComponent;
+
+type VirtualizedListProps<T extends BaseItemProps> = {
+    items: Array<T>;
     onScroll?: (e: Event) => void;
     onScrollEnd: () => void;
     listHeight: number | string;
     listMinHeight: number | string;
+    ref?: React.Ref<HTMLDivElement>;
     renderItem: (item: T, index: number) => React.ReactNode;
 };
 
-export const VirtualizedList = forwardRef<HTMLDivElement, VirtualizedListProps<any>>(
-    <T,>(
-        {
-            items: initialItems,
-            onScroll,
-            onScrollEnd,
-            listHeight,
-            listMinHeight,
-            renderItem,
-        }: VirtualizedListProps<T>,
-        ref: React.Ref<HTMLDivElement>,
-    ) => {
-        const newRef = useRef<HTMLDivElement>(null);
-        const containerRef = (ref as React.RefObject<HTMLDivElement>) || newRef;
-        const [items, setItems] = useState(initialItems);
-        const [startIndex, setStartIndex] = useState(0);
-        const [endIndex, setEndIndex] = useState(DEFAULT_VISIBLE_ITEMS_COUNT);
-        const [itemHeights, setItemHeights] = useState<number[]>([]);
-        const [totalHeight, setTotalHeight] = useState(0);
-        const debouncedOnScrollEnd = debounce(onScrollEnd, 1000);
-
-        const resetScroll = useCallback(() => {
-            if (!containerRef.current) return;
+export function VirtualizedListComponent<T extends BaseItemProps>(
+    {
+        items: initialItems,
+        onScroll,
+        onScrollEnd,
+        listHeight,
+        listMinHeight,
+        renderItem,
+    }: VirtualizedListProps<T>,
+    ref: React.Ref<HTMLDivElement>,
+) {
+    const newRef = useRef<HTMLDivElement>(null);
+    const containerRef = (ref as React.RefObject<HTMLDivElement>) || newRef;
+    const [items, setItems] = useState(initialItems);
+    const [startIndex, setStartIndex] = useState(0);
+    const [endIndex, setEndIndex] = useState(DEFAULT_VISIBLE_ITEMS_COUNT);
+    const debouncedOnScrollEnd = useMemo(() => debounce(onScrollEnd, 1000), [onScrollEnd]);
+
+    const resetScroll = useCallback(() => {
+        if (!containerRef.current) return;
+
+        containerRef.current.scrollTop = 0;
+    }, [containerRef]);
+
+    useEffect(() => {
+        if (isChanged(items, initialItems)) {
+            setItems(initialItems);
+            resetScroll();
+        }
+    }, [initialItems, items, resetScroll]);
 
-            containerRef.current.scrollTop = 0;
-        }, [containerRef]);
+    const itemHeights = useMemo(() => items.map(item => calculateItemHeight(item)), [items]);
+    const totalHeight = useMemo(
+        () => itemHeights.reduce((acc, height) => acc + height, 0),
+        [itemHeights],
+    );
 
-        useEffect(() => {
-            if (isChanged(items, initialItems)) {
-                setItems(initialItems);
-                resetScroll();
-            }
-        }, [initialItems, items, resetScroll]);
-
-        useEffect(() => {
-            const heights = items.map(item => calculateItemHeight(item));
-            setItemHeights(heights);
-            setTotalHeight(heights.reduce((acc, height) => acc + height, 0));
-        }, [items]);
-
-        const handleScroll = useCallback(
-            (e: Event) => {
-                if (!containerRef.current) return;
-                const { scrollTop } = containerRef.current;
-                let offset = 0;
-                let newStartIndex = 0;
-
-                for (let i = 0; i < itemHeights.length; i++) {
-                    if (offset + itemHeights[i] >= scrollTop) {
-                        newStartIndex = i;
-                        break;
-                    }
-                    offset += itemHeights[i];
+    const handleScroll = useCallback(
+        (e: Event) => {
+            if (!containerRef.current) return;
+            const { scrollTop } = containerRef.current;
+            let offset = 0;
+            let newStartIndex = 0;
+
+            for (let i = 0; i < itemHeights.length; i++) {
+                if (offset + itemHeights[i] >= scrollTop) {
+                    newStartIndex = i;
+                    break;
                 }
+                offset += itemHeights[i];
+            }
 
-                newStartIndex = Math.max(0, newStartIndex - BEFORE_AFTER_BUFFER_COUNT);
+            newStartIndex = Math.max(0, newStartIndex - BEFORE_AFTER_BUFFER_COUNT);
 
-                let newEndIndex = newStartIndex;
-                let visibleHeight = 0;
-                const containerHeight = containerRef.current.clientHeight;
+            let newEndIndex = newStartIndex;
+            let visibleHeight = 0;
+            const containerHeight = containerRef.current.clientHeight;
 
-                while (
-                    newEndIndex < items.length &&
-                    visibleHeight <
-                        containerHeight + BEFORE_AFTER_BUFFER_COUNT * ESTIMATED_ITEM_HEIGHT
-                ) {
-                    visibleHeight += itemHeights[newEndIndex];
-                    newEndIndex++;
-                }
-                newEndIndex = Math.min(items.length, newEndIndex + BEFORE_AFTER_BUFFER_COUNT);
+            while (
+                newEndIndex < items.length &&
+                visibleHeight < containerHeight + BEFORE_AFTER_BUFFER_COUNT * ESTIMATED_ITEM_HEIGHT
+            ) {
+                visibleHeight += itemHeights[newEndIndex];
+                newEndIndex++;
+            }
+            newEndIndex = Math.min(items.length, newEndIndex + BEFORE_AFTER_BUFFER_COUNT);
 
-                setStartIndex(newStartIndex);
-                setEndIndex(newEndIndex);
+            setStartIndex(newStartIndex);
+            setEndIndex(newEndIndex);
 
-                if (newEndIndex >= items.length - LOAD_MORE_BUFFER_COUNT) {
-                    debouncedOnScrollEnd();
-                }
-                onScroll?.(e);
-            },
-            [containerRef, debouncedOnScrollEnd, itemHeights, items.length, onScroll],
-        );
+            if (newEndIndex >= items.length - LOAD_MORE_BUFFER_COUNT) {
+                debouncedOnScrollEnd();
+            }
+            onScroll?.(e);
+        },
+        [containerRef, debouncedOnScrollEnd, itemHeights, items.length, onScroll],
+    );
 
-        useEffect(() => {
-            const container = containerRef.current;
-            if (container) {
-                container.addEventListener('scroll', handleScroll);
+    useEffect(() => {
+        const container = containerRef.current;
+        if (container) {
+            container.addEventListener('scroll', handleScroll);
 
-                return () => container.removeEventListener('scroll', handleScroll);
-            }
-        }, [containerRef, handleScroll]);
-
-        return (
-            <Container ref={containerRef} $height={listHeight} $minHeight={listMinHeight}>
-                <Content style={{ height: `${totalHeight}px` }}>
-                    {itemHeights.slice(startIndex, endIndex).map((height, index) => {
-                        const itemIndex = startIndex + index;
-                        const itemTop = itemHeights
-                            .slice(0, itemIndex)
-                            .reduce((acc, h) => acc + h, 0);
-
-                        if (!items[itemIndex]) return null;
-
-                        return (
-                            <Item
-                                key={itemIndex}
-                                style={{
-                                    top: `${itemTop}px`,
-                                    height,
-                                }}
-                            >
-                                {renderItem(items[itemIndex], itemIndex)}
-                            </Item>
-                        );
-                    })}
-                </Content>
-            </Container>
-        );
-    },
-);
+            return () => container.removeEventListener('scroll', handleScroll);
+        }
+    }, [containerRef, handleScroll]);
+
+    return (
+        <ListContainer<T>
+            ref={containerRef}
+            listHeight={listHeight}
+            listMinHeight={listMinHeight}
+            totalHeight={totalHeight}
+            items={items}
+            itemHeights={itemHeights}
+            startIndex={startIndex}
+            endIndex={endIndex}
+            renderItem={renderItem}
+        />
+    );
+}
+
+// NOTE: typecast here + memo() and forwardRef() because of passing the ref for useShadow()
+export const VirtualizedList = memo(
+    forwardRef(VirtualizedListComponent),
+) as typeof VirtualizedListComponent;