Fixed expandable content bug when asynchronously re-rendering Table (…

…#2306)

.changeset/ninety-horses-fry.md

@@ -0,0 +1,5 @@
+---
+'@itwin/itwinui-react': patch
+---
+
+Fixed broken expandable content when `Table` is asynchronously re-rendered as well as the max depth reached error caused by the `getRowId` prop passed into `Table`.

packages/itwinui-react/src/core/Table/Table.test.tsx

@@ -1736,6 +1736,56 @@ it('should expand correctly', async () => {
   getByText('Expanded component, name: Name1');
 });
 
+it('should show expandable content correctly after re-rendering', async () => {
+  const data = mockedData();
+  const getRowIdMock = vi.fn();
+  const { rerender } = render(
+    <Table
+      columns={[
+        {
+          id: 'name',
+          Header: 'Name',
+          Cell: () => <>test1</>,
+        },
+      ]}
+      data={data}
+      emptyTableContent='No data.'
+      subComponent={(row) => (
+        <div>{`Expanded component, name: ${row.original.name}`}</div>
+      )}
+    />,
+  );
+  expect(screen.getAllByText('test1').length).toBe(3);
+
+  rerender(
+    <Table
+      columns={[
+        {
+          id: 'name',
+          Header: 'Name',
+          Cell: () => <>test2</>,
+        },
+      ]}
+      data={data}
+      emptyTableContent='No data.'
+      subComponent={(row) => (
+        <div>{`Expanded component, name: ${row.original.name}`}</div>
+      )}
+      getRowId={getRowIdMock}
+    />,
+  );
+  expect(screen.getAllByText('test2').length).toBe(3);
+
+  await act(async () => {
+    await userEvent.click(screen.getAllByRole('button')[0]);
+  });
+
+  screen.getByText('Expanded component, name: Name1');
+
+  // getRowId is called for each row and sub-row before and after re-rendering.
+  expect(getRowIdMock).toHaveBeenCalledTimes(12);
+});
+
 it('should expand correctly with a custom expander cell', async () => {
   const onExpandMock = vi.fn();
   const { getByText, queryByText } = renderComponent({

packages/itwinui-react/src/core/Table/Table.tsx

@@ -71,6 +71,7 @@ const singleRowSelectedAction = 'singleRowSelected';
 const shiftRowSelectedAction = 'shiftRowSelected';
 export const tableResizeStartAction = 'tableResizeStart';
 const tableResizeEndAction = 'tableResizeEnd';
+const iuiId = Symbol('iui-id');
 
 export type TablePaginatorRendererProps = {
   /**
@@ -412,6 +413,7 @@ export const Table = <
     headerProps,
     bodyProps,
     emptyTableContentProps,
+    getRowId,
     ...rest
   } = props;
 
@@ -578,18 +580,52 @@ export const Table = <
     );
   }, [data, getSubRows]);
 
-  const [rowHasParent] = React.useState(() => new WeakSet<T>());
-
   const getSubRowsWithSubComponents = React.useCallback(
-    (originalRow: T) => {
-      if (!rowHasParent.has(originalRow)) {
-        rowHasParent.add(originalRow);
-        return [originalRow];
+    (originalRow: T, relativeIndex: number) => {
+      // If originalRow represents a sub-component, don't add any sub-rows to it
+      if (originalRow[iuiId as any]) {
+        return [];
+      }
+
+      // If sub-rows already exist, return them as they are.
+      if (originalRow.subRows) {
+        return originalRow.subRows as T[];
+      }
+
+      // If originalRow represents the top-most level row, add itself as a sub-row
+      // that will represent a subComponent.
+      // Add a symbol as a key on this sub-row to distinguish it from the real row.
+      // This distinction is needed as the sub-row needs to have all fields of the row
+      // since react-table expects even sub-rows to be rows (Row<T>).
+      return [
+        {
+          [iuiId]: `subcomponent-${relativeIndex}`,
+          ...originalRow,
+        },
+      ];
+    },
+    [],
+  );
+
+  /**
+   * Gives `subComponent` a unique id since `subComponent` has the same react-table id as its parent row.
+   * Avoiding duplicate react-table ids prevents react-table errors.
+   */
+  const getRowIdWithSubComponents = React.useCallback(
+    (originalRow: T, relativeIndex: number, parent?: Row<T>) => {
+      const defaultRowId = parent
+        ? `${parent.id}.${relativeIndex}`
+        : `${relativeIndex}`;
+      const rowIdFromUser = getRowId?.(originalRow, relativeIndex, parent);
+      // If the row contains the Symbol, it indicates that the current row is a sub-component row.
+      // We need to append the ID passed by user with its according sub-component ID.
+      if (rowIdFromUser !== undefined && originalRow[iuiId as any]) {
+        return `${rowIdFromUser}-${defaultRowId}`;
       }
 
-      return (originalRow.subRows as T[]) || [];
+      return rowIdFromUser ?? defaultRowId;
     },
-    [rowHasParent],
+    [getRowId],
   );
 
   const instance = useTable<T>(
@@ -607,6 +643,7 @@ export const Table = <
       getSubRows: subComponent ? getSubRowsWithSubComponents : getSubRows,
       initialState: { pageSize, ...props.initialState },
       columnResizeMode,
+      getRowId: subComponent ? getRowIdWithSubComponents : getRowId, // only call this wrapper function when sub-component is present
     },
     useFlexLayout,
     useResizeColumns(ownerDocument),
@@ -857,32 +894,9 @@ export const Table = <
     ) => {
       const row = page[index];
       prepareRow(row);
+      const isRowASubComponent = !!row.original[iuiId as any] && !!subComponent;
 
-      if (row.subRows.length > 0 || !subComponent) {
-        return (
-          <TableRowMemoized
-            row={row}
-            rowProps={rowProps}
-            isLast={index === page.length - 1}
-            onRowInViewport={onRowInViewportRef}
-            onBottomReached={onBottomReachedRef}
-            intersectionMargin={intersectionMargin}
-            state={state}
-            key={row.getRowProps().key}
-            onClick={onRowClickHandler}
-            subComponent={subComponent}
-            isDisabled={!!isRowDisabled?.(row.original)}
-            tableHasSubRows={hasAnySubRows}
-            tableInstance={instance}
-            expanderCell={expanderCell}
-            scrollContainerRef={tableRef.current}
-            tableRowRef={enableVirtualization ? undefined : tableRowRef(row)}
-            density={density}
-            virtualItem={virtualItem}
-            virtualizer={virtualizer}
-          />
-        );
-      } else {
+      if (isRowASubComponent) {
         return (
           <TableExpandableContentMemoized
             key={row.getRowProps().key}
@@ -898,17 +912,41 @@ export const Table = <
           </TableExpandableContentMemoized>
         );
       }
+
+      return (
+        <TableRowMemoized
+          row={row}
+          rowProps={rowProps}
+          isLast={index === page.length - 1}
+          onRowInViewport={onRowInViewportRef}
+          onBottomReached={onBottomReachedRef}
+          intersectionMargin={intersectionMargin}
+          state={state}
+          key={row.getRowProps().key}
+          onClick={onRowClickHandler}
+          subComponent={subComponent}
+          isDisabled={!!isRowDisabled?.(row.original)}
+          tableHasSubRows={hasAnySubRows}
+          tableInstance={instance}
+          expanderCell={expanderCell}
+          scrollContainerRef={tableRef.current}
+          tableRowRef={enableVirtualization ? undefined : tableRowRef(row)}
+          density={density}
+          virtualItem={virtualItem}
+          virtualizer={virtualizer}
+        />
+      );
     },
     [
       page,
       prepareRow,
+      subComponent,
       rowProps,
       onRowInViewportRef,
       onBottomReachedRef,
       intersectionMargin,
       state,
       onRowClickHandler,
-      subComponent,
       isRowDisabled,
       hasAnySubRows,
       instance,