Skip to content

fix(tablev2): keep RenderRow stable #1120

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
bert-e merged 3 commits into from
Jun 4, 2026
Merged

fix(tablev2): keep RenderRow stable #1120

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
8fb49f0
fix(tablev2): keep RenderRow stable so rows aren't remounted on re-re…
JeanMarcMilletScality May 29, 2026
f463fc4
docs(tablev2): document memoizing columns to avoid cell remounts
JeanMarcMilletScality Jun 1, 2026
fa5bfc8
fix(tablev2): read row-selection callbacks at click time, not render …
JeanMarcMilletScality Jun 3, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
179 changes: 107 additions & 72 deletions src/lib/components/tablev2/MultiSelectableContent.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { useEffect, useState, memo, CSSProperties } from 'react';
import { useEffect, useState, memo, useMemo, useRef } from 'react';
import { Row } from 'react-table';
import { areEqual } from 'react-window';
import { areEqual, ListChildComponentProps } from 'react-window';
import { useTableContext } from './Tablev2.component';
import {
HeadRow,
Expand All @@ -14,7 +14,7 @@ import {
TableLocalType,
TableVariantType,
} from './TableUtils';
import { RenderRowType, TableRows, useTableScrollbar } from './TableCommon';
import { TableRows, useTableScrollbar } from './TableCommon';
import useSyncedScroll from './useSyncedScroll';
import { Box } from '../box/Box';
import { Loader } from '../loader/Loader.component';
Expand Down Expand Up @@ -124,79 +124,114 @@ export const MultiSelectableContent = <

const { headerRef } = useSyncedScroll<DATA_ROW>();

const RenderRow = memo(({ index, style }: RenderRowType) => {
const row = rows[index];
prepareRow(row);
/**
* These values change identity on (almost) every render. We read them through refs so the row
* renderer below can keep a stable identity (see RenderRow).
*/
const prepareRowRef = useRef(prepareRow);
prepareRowRef.current = prepareRow;
const selectedRowIdsRef = useRef(selectedRowIds);
selectedRowIdsRef.current = selectedRowIds;
const onSingleRowSelectedRef = useRef(onSingleRowSelected);
onSingleRowSelectedRef.current = onSingleRowSelected;
const toggleAllRowsSelectedRef = useRef(toggleAllRowsSelected);
toggleAllRowsSelectedRef.current = toggleAllRowsSelected;
const handleMultipleSelectedRowsRef = useRef(handleMultipleSelectedRows);
handleMultipleSelectedRowsRef.current = handleMultipleSelectedRows;

const rowProps = {
...row.getRowProps({
/**
* Note:We need to pass the style property to the row component.
* Otherwise when we scroll down, the next rows are flashing
* because they are re-rendered in loop.
*/
style: { ...style },
}),
onClick: onSingleRowSelected
? () => {
onSingleRowSelected(row);
toggleAllRowsSelected(false);
setActiveRowId(row.id);
}
: () => handleMultipleSelectedRows(selectedRowIds, rows, row, index),
};
/**
* RenderRow MUST keep a stable identity across re-renders. It used to be redefined inline on
* every render, so react-window saw a new component type each time and remounted (not just
* re-rendered) every row — and therefore every cell — whenever the table re-rendered for any
* reason. That made async cell content reload and flash. We now read the row from react-window's
* `data` (itemData) prop and the volatile callbacks/state from refs, so the component is only
* recreated when something that affects the rendered output (activeRowId / separationLineVariant)
* actually changes. Checkbox selection still updates because react-table rebuilds `rows` (and
* therefore `data`) when `selectedRowIds` changes.
*/
const RenderRow = useMemo(
() =>
memo(({ index, style, data }: ListChildComponentProps<Row<DATA_ROW>[]>) => {
const rows = data;
const row = data[index];
prepareRowRef.current(row);

return (
<TableRowMultiSelectable
{...rowProps}
isSelected={row.isSelected || activeRowId === row.id}
separationLineVariant={separationLineVariant}
className="tr"
>
{row.cells.map((cell) => {
const cellProps = cell.getCellProps({
style: {
...cell.column.cellStyle,
// Vertically center the text in cells.
display: 'flex',
flexDirection: 'column',
justifyContent: 'center',
},
role: 'gridcell',
});
const rowProps = {
...row.getRowProps({
/**
* Note:We need to pass the style property to the row component.
* Otherwise when we scroll down, the next rows are flashing
* because they are re-rendered in loop.
*/
style: { ...style },
}),
onClick: () => {
const onSingleRowSelected = onSingleRowSelectedRef.current;
if (onSingleRowSelected) {
onSingleRowSelected(row);
toggleAllRowsSelectedRef.current(false);
setActiveRowId(row.id);
} else {
handleMultipleSelectedRowsRef.current(
selectedRowIdsRef.current,
rows,
row,
index,
);
}
},
};

if (cell.column.id === 'selection') {
return (
<div
{...cellProps}
onClick={
onSingleRowSelected
? (event) => {
event.stopPropagation();
handleMultipleSelectedRows(
selectedRowIds,
rows,
row,
index,
);
}
: undefined
}
>
{cell.render('Cell')}
</div>
);
}
return (
<TableRowMultiSelectable
{...rowProps}
isSelected={row.isSelected || activeRowId === row.id}
separationLineVariant={separationLineVariant}
className="tr"
>
{row.cells.map((cell) => {
const cellProps = cell.getCellProps({
style: {
...cell.column.cellStyle,
// Vertically center the text in cells.
display: 'flex',
flexDirection: 'column',
justifyContent: 'center',
},
role: 'gridcell',
});

if (cell.column.id === 'selection') {
return (
<div
{...cellProps}
onClick={(event) => {
if (!onSingleRowSelectedRef.current) return;
event.stopPropagation();
handleMultipleSelectedRowsRef.current(
selectedRowIdsRef.current,
rows,
row,
index,
);
}}
>
{cell.render('Cell')}
</div>
);
}

return (
<div {...cellProps} className="td">
{cell.render('Cell')}
</div>
);
})}
</TableRowMultiSelectable>
);
}, areEqual);
return (
<div {...cellProps} className="td">
{cell.render('Cell')}
</div>
);
})}
</TableRowMultiSelectable>
);
}, areEqual),
[activeRowId, separationLineVariant],

@claude claude Bot Jun 1, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

activeRowId in the useMemo deps means every row click (via onSingleRowSelected) creates a new component type, causing react-window to unmount and remount all visible rows — the same flashing problem this PR fixes for other re-render triggers. The same applies to selectedId in SingleSelectableContent.

To keep the component stable across selection changes, move activeRowId into a ref (like the other volatile values) and pass it through itemData so areEqual can trigger re-renders without changing the component identity:

const activeRowIdRef = useRef<string | null>(null);
activeRowIdRef.current = activeRowId;

// in useMemo deps: remove activeRowId, keep only [separationLineVariant]
// in the row component: read activeRowIdRef.current instead of activeRowId
// pass activeRowId through itemData so areEqual detects the change:
//   itemData={useMemo(() => ({ rows, activeRowId }), [rows, activeRowId])}

This changes the itemData shape from Row[] to { rows, activeRowId }, which touches VirtualizedRows and TableRows types — a bigger refactor, so it may belong in a follow-up.

);

return (
<>
Expand Down
154 changes: 89 additions & 65 deletions src/lib/components/tablev2/SingleSelectableContent.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { memo, useEffect, useRef } from 'react';
import { areEqual, FixedSizeList } from 'react-window';
import { memo, useEffect, useMemo, useRef } from 'react';
import { areEqual, FixedSizeList, ListChildComponentProps } from 'react-window';
import { Row } from 'react-table';
import { useTableContext } from './Tablev2.component';
import {
Expand All @@ -14,7 +14,7 @@ import {
TableLocalType,
TableVariantType,
} from './TableUtils';
import { RenderRowType, TableRows, useTableScrollbar } from './TableCommon';
import { TableRows, useTableScrollbar } from './TableCommon';
import useSyncedScroll from './useSyncedScroll';
import { Loader } from '../loader/Loader.component';
import { Box } from '../box/Box';
Expand Down Expand Up @@ -77,69 +77,93 @@ export function SingleSelectableContent<
return () => clearTimeout(timer);
}, [autoScrollToSelected, selectedId, rows]);

const RenderRow = memo(({ index, style }: RenderRowType) => {
const row = rows[index];
prepareRow(row);
let rowProps = row.getRowProps({
/**
* Note: We need to pass the style property to the row component.
* Otherwise when we scroll down, the next rows are flashing
* because they are re-rendered in loop.
*/
style: { ...style },
});

rowProps = {
...rowProps,
...{
onClick: () => {
if (onRowSelected) return onRowSelected(row);
},
tabIndex: onRowSelected ? 0 : undefined,
onKeyDown: (event) => {
if (
onRowSelected &&
(event.key === ' ' ||
event.key === 'Enter' ||
event.key === 'Spacebar')
) {
event.preventDefault();
onRowSelected(row);
}
},
},
};

return (
<TableRow
{...rowProps}
isSelected={selectedId === row.id}
aria-selected={selectedId === row.id ? 'true' : 'false'}
separationLineVariant={separationLineVariant}
selectedId={selectedId}
className="tr"
>
{row.cells.map((cell) => {
let cellProps = cell.getCellProps({
style: {
...cell.column.cellStyle,
// Vertically center the text in cells.
display: 'flex',
flexDirection: 'column',
justifyContent: 'center',
/**
* `prepareRow` and `onRowSelected` change identity on every render. We read them through refs
* so the row renderer below can keep a stable identity (see RenderRow).
*/
const prepareRowRef = useRef(prepareRow);
prepareRowRef.current = prepareRow;
const onRowSelectedRef = useRef(onRowSelected);
onRowSelectedRef.current = onRowSelected;

/**
* RenderRow MUST keep a stable identity across re-renders. It used to be redefined inline on
* every render, so react-window saw a new component type each time and remounted (not just
* re-rendered) every row — and therefore every cell — whenever the table re-rendered for any
* reason. That made async cell content reload and flash. We now read the row from react-window's
* `data` (itemData) prop and the volatile callbacks from refs, so the component only needs to be
* recreated when something that affects the rendered output (selectedId / separationLineVariant)
* actually changes.
*/
const RenderRow = useMemo(
() =>
memo(({ index, style, data }: ListChildComponentProps<Row<DATA_ROW>[]>) => {
const row = data[index];
prepareRowRef.current(row);
let rowProps = row.getRowProps({
/**
* Note: We need to pass the style property to the row component.
* Otherwise when we scroll down, the next rows are flashing
* because they are re-rendered in loop.
*/
style: { ...style },
});

rowProps = {
...rowProps,
...{
onClick: () => {
const onRowSelected = onRowSelectedRef.current;
if (onRowSelected) return onRowSelected(row);
},
role: 'gridcell',
});

return (
<div {...cellProps} className="td">
{cell.render('Cell')}
</div>
);
})}
</TableRow>
);
}, areEqual);
tabIndex: onRowSelectedRef.current ? 0 : undefined,
onKeyDown: (event) => {
const onRowSelected = onRowSelectedRef.current;
if (
onRowSelected &&
(event.key === ' ' ||
event.key === 'Enter' ||
event.key === 'Spacebar')
) {
event.preventDefault();
onRowSelected(row);
}
},
},
};

return (
<TableRow
{...rowProps}
isSelected={selectedId === row.id}
aria-selected={selectedId === row.id ? 'true' : 'false'}
separationLineVariant={separationLineVariant}
selectedId={selectedId}
className="tr"
>
{row.cells.map((cell) => {
let cellProps = cell.getCellProps({
style: {
...cell.column.cellStyle,
// Vertically center the text in cells.
display: 'flex',
flexDirection: 'column',
justifyContent: 'center',
},
role: 'gridcell',
});

return (
<div {...cellProps} className="td">
{cell.render('Cell')}
</div>
);
})}
</TableRow>
);
}, areEqual),
[selectedId, separationLineVariant],
);

const { hasScrollbar, scrollBarWidth, handleScrollbarWidth } =
useTableScrollbar();
Expand Down
4 changes: 1 addition & 3 deletions src/lib/components/tablev2/TableCommon.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -128,9 +128,7 @@ type TableRowsProps<
locale?: TableLocalType;
children?: (children: JSX.Element) => JSX.Element;
customItemKey?: (index: number, data: DATA_ROW) => string;
RenderRow: React.MemoExoticComponent<
({ index, style }: RenderRowType) => JSX.Element
>;
RenderRow: ComponentType<ListChildComponentProps<Row<DATA_ROW>[]>>;

@claude claude Bot Jun 1, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

RenderRowType (line 120) and its CSSProperties import (line 18) are now dead code — no remaining importers after this change. Clean them up.

listRef?: Ref<FixedSizeList<Row<DATA_ROW>[]>>;
};
export function TableRows<
Expand Down
Loading
Loading