From 2ae2ce0797eb788c8b1b969c1b52a166dc118e94 Mon Sep 17 00:00:00 2001 From: Marco Beretta <81851188+berry-13@users.noreply.github.com> Date: Sat, 13 Sep 2025 23:59:28 +0200 Subject: [PATCH] feat: enhance deepEqual function for array support and improve column style stability --- packages/client/src/components/DataTable.tsx | 163 +++++++++++++------ 1 file changed, 117 insertions(+), 46 deletions(-) diff --git a/packages/client/src/components/DataTable.tsx b/packages/client/src/components/DataTable.tsx index 5167e05b4e..3715412134 100644 --- a/packages/client/src/components/DataTable.tsx +++ b/packages/client/src/components/DataTable.tsx @@ -173,19 +173,33 @@ const deepEqual = (a: unknown, b: unknown): boolean => { if (typeof a !== typeof b) return false; if (typeof a === 'object') { - const keysA = Object.keys(a as Record); - const keysB = Object.keys(b as Record); - if (keysA.length !== keysB.length) return false; - - for (const key of keysA) { - if ( - !keysB.includes(key) || - !deepEqual((a as Record)[key], (b as Record)[key]) - ) { - return false; + // Handle arrays + if (Array.isArray(a) && Array.isArray(b)) { + if (a.length !== b.length) return false; + for (let i = 0; i < a.length; i++) { + if (!deepEqual(a[i], b[i])) return false; } + return true; } - return true; + + // Handle non-array objects + if (!Array.isArray(a) && !Array.isArray(b)) { + const keysA = Object.keys(a as Record); + const keysB = Object.keys(b as Record); + if (keysA.length !== keysB.length) return false; + + for (const key of keysA) { + if ( + !keysB.includes(key) || + !deepEqual((a as Record)[key], (b as Record)[key]) + ) { + return false; + } + } + return true; + } + + return false; } return false; @@ -211,7 +225,8 @@ const SelectionCheckbox = memo( } e.stopPropagation(); }} - className={`flex h-full ${DATA_TABLE_CONSTANTS.CHECKBOX_WIDTH} items-center justify-center`} + className={`flex h-full items-center justify-center`} + style={{ width: DATA_TABLE_CONSTANTS.CHECKBOX_WIDTH }} onClick={(e) => { e.stopPropagation(); onChange(!checked); @@ -224,12 +239,15 @@ const SelectionCheckbox = memo( SelectionCheckbox.displayName = 'SelectionCheckbox'; -// Memoized column style computation +// Memoized column style computation - Fixed: Stable keying for columns without explicit id const useColumnStyles = ( columns: TableColumn[], isSmallScreen: boolean, ) => { return useMemo(() => { + const keyFor = (column: TableColumn) => + column.id ?? (column as any).accessorKey ?? ''; + const getColumnStyle = (column: TableColumn): React.CSSProperties => ({ width: isSmallScreen ? column?.meta?.mobileSize : column?.meta?.size, minWidth: column?.meta?.minWidth, @@ -238,8 +256,9 @@ const useColumnStyles = ( return columns.reduce( (acc, column) => { - if (column.id) { - acc[column.id] = getColumnStyle(column); + const key = keyFor(column); + if (key) { + acc[key] = getColumnStyle(column); } return acc; }, @@ -329,19 +348,18 @@ const DeleteButton = memo( variant="outline" onClick={onDelete} disabled={disabled} + style={{ minWidth: DATA_TABLE_CONSTANTS.DELETE_BUTTON_MIN_WIDTH }} className={cn( - `min-w-[${DATA_TABLE_CONSTANTS.DELETE_BUTTON_MIN_WIDTH}] transition-all ${DATA_TABLE_CONSTANTS.ROW_TRANSITION_DURATION} hover:border-red-300 hover:bg-red-50 dark:hover:bg-red-950/20`, + `transition-all hover:border-red-300 hover:bg-red-50 dark:hover:bg-red-950/20`, isSmallScreen && 'px-2 py-1', )} aria-label={isDeleting ? 'Deleting selected rows' : 'Delete selected rows'} > {isDeleting ? ( - + ) : ( <> - + {!isSmallScreen && Delete} )} @@ -629,7 +647,7 @@ export default function DataTable({ const [columnVisibility, setColumnVisibility] = useState({}); const [columnPinning, setColumnPinning] = useState({}); const [optimizedRowSelection, setOptimizedRowSelection] = useOptimizedRowSelection(); - const [term, setTerm] = useState(''); + const [term, setTerm] = useState(filterValue ?? ''); const [isDeleting, setIsDeleting] = useState(false); const [error, setError] = useState(null); const [isSearching, setIsSearching] = useState(false); @@ -651,6 +669,16 @@ export default function DataTable({ // Mount tracking for cleanup - Fixed: Declare mountedRef before any callback that uses it const mountedRef = useRef(true); + // Sync internal sorting with defaultSort changes + useEffect(() => { + if (!sorting) setInternalSorting(defaultSort); + }, [defaultSort, sorting]); + + // Keep search input in sync with external filterValue + useEffect(() => { + setTerm(filterValue ?? ''); + }, [filterValue]); + // Sorting handler: call external callback if provided, otherwise use internal state const handleSortingChangeInternal = useCallback( (updater: SortingState | ((old: SortingState) => SortingState)) => { @@ -726,7 +754,8 @@ export default function DataTable({ id: 'select', header: ({ table }: { table: TTable }) => (
({ // Memoized column styles for performance const columnStyles = useColumnStyles(tableColumns as TableColumn[], isSmallScreen); - // Set CSS variables for column sizing - Fixed: Add SSR guard + // Set CSS variables for column sizing with hash optimization + const columnSizesHashRef = useRef(''); useLayoutEffect(() => { if (typeof window === 'undefined' || !tableContainerRef.current) return; - tableColumns.forEach((column, index) => { - if (column.id) { - const size = columnStyles[column.id]?.width || 'auto'; - tableContainerRef.current!.style.setProperty(`--col-${index}-size`, `${size}`); - } - }); + // Calculate hash of column sizes to avoid unnecessary DOM writes + const sizesHash = tableColumns + .map((col, index) => `${index}:${columnStyles[col.id!]?.width || 'auto'}`) + .join('|'); + + if (sizesHash !== columnSizesHashRef.current) { + columnSizesHashRef.current = sizesHash; + + tableColumns.forEach((column, index) => { + if (column.id) { + const size = columnStyles[column.id]?.width || 'auto'; + tableContainerRef.current!.style.setProperty(`--col-${index}-size`, `${size}`); + } + }); + } }, [tableColumns, columnStyles]); // Memoized row data with stable references @@ -811,7 +850,11 @@ export default function DataTable({ const measuredHeightsRef = useRef([]); const rowVirtualizer = useVirtualizer({ count: rows.length, - getScrollElement: () => tableContainerRef.current, + getScrollElement: () => { + // SSR safety: return null during SSR + if (typeof window === 'undefined') return null; + return tableContainerRef.current; + }, estimateSize: useCallback(() => { const heights = measuredHeightsRef.current; if (heights.length > 0) { @@ -860,10 +903,13 @@ export default function DataTable({ [virtualRows, rows], ); - // Fixed: Infinite scrolling with simplified trigger logic and removed accidental local mountedRef + // Fixed: Infinite scrolling with early return pattern and always attached listener const handleScrollInternal = useCallback(async () => { if (!mountedRef.current || !tableContainerRef.current) return; + // Early return if conditions not met + if (!hasNextPage || isFetchingNextPage) return; + const scrollElement = tableContainerRef.current; const clientHeight = scrollElement.clientHeight; const virtualEnd = virtualRows.length > 0 ? virtualRows[virtualRows.length - 1].end : 0; @@ -883,20 +929,29 @@ export default function DataTable({ onError?.(sanitizedError); } } - }, [fetchNextPage, totalSize, virtualRows, onError, sanitizeError]); + }, [ + fetchNextPage, + totalSize, + virtualRows, + onError, + sanitizeError, + hasNextPage, + isFetchingNextPage, + ]); const throttledHandleScroll = useMemo( () => throttle(handleScrollInternal, DATA_TABLE_CONSTANTS.SCROLL_THROTTLE_MS), [handleScrollInternal], ); + // Always attach scroll listener, early return in handler useEffect(() => { const scrollElement = tableContainerRef.current; - if (!scrollElement || !hasNextPage || isFetchingNextPage) return; + if (!scrollElement) return; scrollElement.addEventListener('scroll', throttledHandleScroll, { passive: true }); return () => scrollElement.removeEventListener('scroll', throttledHandleScroll); - }, [throttledHandleScroll, hasNextPage, isFetchingNextPage]); + }, [throttledHandleScroll]); // Resize observer for virtualizer revalidation const handleWindowResize = useCallback(() => { @@ -1064,11 +1119,13 @@ export default function DataTable({ > {/* Accessible live region for loading announcements */}
- {isFetchingNextPage - ? 'Loading more rows' - : hasNextPage - ? 'More rows available' - : 'All rows loaded'} + {isSearching + ? 'Searching...' + : isFetchingNextPage + ? 'Loading more rows' + : hasNextPage + ? 'More rows available' + : 'All rows loaded'}
{/* Error display - kept outside boundary for non-rendering errors */} @@ -1093,7 +1150,10 @@ export default function DataTable({ )} {shouldShowSearch && ( -
+
{ @@ -1119,18 +1179,25 @@ export default function DataTable({ ref={tableContainerRef} className={cn( `relative h-[calc(100vh-20rem)] max-w-full overflow-auto rounded-md border border-black/10 dark:border-white/10`, - `transition-all ${DATA_TABLE_CONSTANTS.ROW_TRANSITION_DURATION} ease-out`, + `transition-all ease-out`, 'scrollbar-thin scrollbar-track-transparent scrollbar-thumb-gray-300 dark:scrollbar-thumb-gray-600', isRefreshing && 'bg-surface-secondary/30', )} + style={{ + minWidth: DATA_TABLE_CONSTANTS.TABLE_MIN_WIDTH, + transitionDuration: DATA_TABLE_CONSTANTS.ROW_TRANSITION_DURATION, + }} role="grid" aria-label="Data grid" aria-rowcount={data.length} - aria-busy={isLoading || isFetchingNextPage} + aria-busy={isLoading || isFetching || isFetchingNextPage} > {headerGroups.map((headerGroup) => ( @@ -1166,7 +1233,7 @@ export default function DataTable({ : undefined } className={cn( - 'relative whitespace-nowrap bg-surface-secondary px-2 py-2 text-left text-sm font-medium text-text-secondary transition-colors duration-200 sm:px-4', + 'group relative whitespace-nowrap bg-surface-secondary px-2 py-2 text-left text-sm font-medium text-text-secondary transition-colors duration-200 sm:px-4', canSort && 'cursor-pointer hover:bg-surface-hover focus-visible:bg-surface-hover focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-primary', )} @@ -1175,6 +1242,7 @@ export default function DataTable({ ...getCommonPinningStyles(header.column, table), }} role="columnheader" + scope="col" tabIndex={canSort ? 0 : -1} aria-sort={ariaSort} > @@ -1203,6 +1271,7 @@ export default function DataTable({
{header.column.getIsPinned() !== 'left' && (