feat(table): virtualization #4285
Conversation
🦋 Changeset detectedLatest commit: f73a7a5 The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
WalkthroughThe changes in this pull request involve updates to the documentation and functionality of various table components within the NextUI framework. Key modifications include the introduction of virtualization features to enhance performance when rendering large datasets. The Changes
Assessment against linked issues
📜 Recent review detailsConfiguration used: .coderabbit.yaml ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (2)
⏰ Context from checks skipped due to timeout of 90000ms (2)
🔇 Additional comments (1)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
vinroger
force-pushed
the
feat/eng-1633-virtualization-for-table
branch
2 times, most recently
from
6346107 to
9f83094
Compare
vinroger
force-pushed
the
feat/eng-1633-virtualization-for-table
branch
from
9f83094 to
ed0a7d8
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
packages/components/table/src/virtualized-table-body.tsx (1)
128-129:⚠️ Potential issueCorrect the
transformcalculation for proper row positioning.The transform calculation is incorrect and can lead to improper row positioning.
- transform: `translateY(${virtualRow.start - index * virtualRow.size}px)`, + transform: `translateY(${virtualRow.start}px)`,
🧹 Nitpick comments (6)
packages/components/table/src/virtualized-table-body.tsx (2)
25-25: Consider improving type safety forrowVirtualizer.The
rowVirtualizerprop usesanytype which could lead to type safety issues. Consider using a more specific type from@tanstack/react-virtual.- rowVirtualizer: Virtualizer<any, Element>; + rowVirtualizer: Virtualizer<unknown, Element>;
64-96: Consider memoizing empty and loading content.The empty and loading content are recreated on every render. Consider using
useMemoto optimize performance.+import {useMemo} from "react"; + const VirtualizedTableBody = forwardRef<"tbody", VirtualizedTableBodyProps>((props, ref) => { // ...existing code... - let emptyContent; - let loadingContent; + const emptyContent = useMemo(() => { + if (collection.size === 0 && bodyProps.emptyContent) { + return ( + <tr role="row"> + <td + className={slots?.emptyWrapper({class: classNames?.emptyWrapper})} + colSpan={collection.columnCount} + role="gridcell" + > + {!isLoading && bodyProps.emptyContent} + </td> + </tr> + ); + } + return null; + }, [collection.size, bodyProps.emptyContent, slots, classNames, isLoading, collection.columnCount]);packages/components/table/src/virtualized-table.tsx (4)
39-39: Add prop validation forrowHeightandmaxTableHeight.Consider adding validation to ensure these values are positive numbers and documenting the default values in JSDoc.
+/** + * @property {number} [rowHeight=40] - Height of each row in pixels + * @property {number} [maxTableHeight=600] - Maximum height of the table in pixels + */ export interface TableProps<T = object> extends Omit<UseTableProps<T>, "isSelectable" | "isMultiSelectable"> { isVirtualized?: boolean; - rowHeight?: number; - maxTableHeight?: number; + rowHeight?: number & Record<never, never>; + maxTableHeight?: number & Record<never, never>; } const VirtualizedTable = forwardRef<"table", TableProps>((props, ref) => { + const {rowHeight = 40, maxTableHeight = 600} = props; + + if (rowHeight <= 0 || maxTableHeight <= 0) { + throw new Error('rowHeight and maxTableHeight must be positive numbers'); + }
71-75: Improve header height calculation resilience.The current implementation might miss header height updates if the header content changes dynamically.
useLayoutEffect(() => { + const updateHeaderHeight = () => { if (headerRef.current) { setHeaderHeight(headerRef.current.getBoundingClientRect().height); } + }; + + updateHeaderHeight(); + + // Optional: Setup ResizeObserver for dynamic content + const resizeObserver = new ResizeObserver(updateHeaderHeight); + if (headerRef.current) { + resizeObserver.observe(headerRef.current); + } + + return () => resizeObserver.disconnect(); }, [headerRef]);
77-82: Consider makingoverscanconfigurable.The overscan value determines how many rows to render outside the visible area. Different use cases might benefit from different values.
export interface TableProps<T = object> extends Omit<UseTableProps<T>, "isSelectable" | "isMultiSelectable"> { isVirtualized?: boolean; rowHeight?: number; maxTableHeight?: number; + overscan?: number; } const VirtualizedTable = forwardRef<"table", TableProps>((props, ref) => { - const {rowHeight = 40, maxTableHeight = 600} = props; + const {rowHeight = 40, maxTableHeight = 600, overscan = 5} = props; const rowVirtualizer = useVirtualizer({ count, getScrollElement: () => parentRef.current, estimateSize: () => rowHeight, - overscan: 5, + overscan, });
51-52: Enhance documentation for the display property requirement.The inline comment about display property is crucial for maintaining scroll functionality. Consider making it more visible in the component's documentation.
+/** + * @important The wrapper must have display: "block" to maintain scroll position + * when virtualizing rows. Changing this value will break the virtualization. + */ return ( <BaseComponent {...getWrapperProps()} ref={parentRef} - /* Display must be block to maintain the scroll "progress" */ style={{height: maxTableHeight, display: "block"}} >
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/components/table/src/virtualized-table-body.tsx(1 hunks)packages/components/table/src/virtualized-table.tsx(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: TypeScript
- GitHub Check: Build
…ub.com/heroui-inc/heroui into feat/eng-1633-virtualization-for-table
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
packages/components/table/src/virtualized-table-body.tsx (1)
128-129:⚠️ Potential issueCorrect the
transformcalculation for proper row positioningThe
transformproperty in the row's style should usevirtualRow.startdirectly. Subtractingindex * virtualRow.sizecan lead to incorrect row positioning.Apply this diff to fix the calculation:
- transform: `translateY(${virtualRow.start - index * virtualRow.size}px)`, + transform: `translateY(${virtualRow.start}px)`,
🧹 Nitpick comments (1)
packages/components/table/src/virtualized-table.tsx (1)
77-82: Add a comment explaining the overscan value.Consider adding a comment explaining why the overscan value is set to 5. This helps other developers understand the performance implications of this configuration.
Apply this diff to add the comment:
const rowVirtualizer = useVirtualizer({ count, getScrollElement: () => parentRef.current, estimateSize: () => rowHeight, + // Pre-render 5 items above and below the visible area for smooth scrolling overscan: 5, });
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (4)
packages/components/table/package.json(1 hunks)packages/components/table/src/virtualized-table-body.tsx(1 hunks)packages/components/table/src/virtualized-table.tsx(1 hunks)packages/components/table/stories/table.stories.tsx(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/components/table/package.json
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: TypeScript
- GitHub Check: Build
🔇 Additional comments (5)
packages/components/table/src/virtualized-table-body.tsx (1)
14-26: Well-defined props interface!The
VirtualizedTableBodyPropsinterface is well-structured and properly extendsHTMLHeroUIProps<"tbody">. The props cover all necessary aspects of virtualization, selection, and styling.packages/components/table/src/virtualized-table.tsx (1)
13-18: Well-defined props interface!The
TablePropsinterface properly extends the base props and adds virtualization-specific properties with appropriate types.packages/components/table/stories/table.stories.tsx (3)
123-129: LGTM!The
generateRowsutility function is well-implemented, usingArray.fromto efficiently generate test data with unique keys.
923-953: Well-structured story template!The
VirtualizedTemplateis well-implemented with proper virtualization settings and test data generation.
1154-1170: Consider consolidating duplicate story implementations.The
VirtualizedandTenThousandRowsstories share almost identical implementation, differing only in row count. Consider refactoring to reduce code duplication.Apply this diff to consolidate the stories:
+const VirtualizedTableStory = ({rowCount, ...args}: TableProps & {rowCount: number}) => { + const rows = generateRows(rowCount); + const columns = [ + {key: "name", label: "Name"}, + {key: "value", label: "Value"}, + ]; + + return ( + <div> + <Table + aria-label={`Example of virtualized table with ${rowCount} rows`} + {...args} + isVirtualized + maxTableHeight={300} + rowHeight={40} + > + <TableHeader columns={columns}> + {(column) => <TableColumn key={column.key}>{column.label}</TableColumn>} + </TableHeader> + <TableBody items={rows}> + {(item) => ( + <TableRow key={item.key}> + {(columnKey) => <TableCell>{item[columnKey]}</TableCell>} + </TableRow> + )} + </TableBody> + </Table> + </div> + ); +}; + +export const Virtualized = { + render: (args: TableProps) => <VirtualizedTableStory rowCount={500} {...args} />, + args: { + ...defaultProps, + className: "max-w-3xl", + }, +}; + +export const TenThousandRows = { + render: (args: TableProps) => <VirtualizedTableStory rowCount={10000} {...args} />, + args: { + ...defaultProps, + className: "max-w-3xl", + }, +};
Closes #3697
Results: Demo for 10000 rows + Full Page Refresh (Delay reduced significantly)
table.virtualized.mp4
📝 Description
This PR introduces virtualization support for the
Tablecomponent, enabling efficient rendering and handling of large datasets by only rendering visible items in the viewport. The implementation utilizes the@tanstack/react-virtuallibrary and adds several customizable props to enhance usability and performance.⛳️ Current behavior (updates)
The current
Tablecomponent renders all rows in the DOM, which can lead to significant performance issues when displaying large datasets.🚀 New behavior
Virtualization Support:
isVirtualizedprop to enable virtualization.rowHeight: Specifies the height of each row (default: 40px).maxTableHeight: Sets the maximum height of the table (default: 600px).Documentation and Examples:
rowHeightfor variable row sizes.maxTableHeightconfigurations.Performance Improvements:
💣 Is this a breaking change (Yes/No):
No
📝 Additional Information
This feature is powered by the
@tanstack/react-virtuallibrary for efficient list rendering. All new props and behavior changes are backward-compatible, ensuring seamless integration with existing implementations.Summary by CodeRabbit
Summary by CodeRabbit
Summary by CodeRabbit
New Features
Tablecomponent, allowing efficient rendering of large datasets.isVirtualized,rowHeight, andmaxTableHeightfor enhanced configurability.Bug Fixes
TableRowGroupcomponent to improve type safety.Documentation
Tablecomponent with new sections on virtualization and updated API details.Chores