Skip to content

fix: Use Map for O(1) column lookups in table width calculations #4142

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

Open
TrevorBurnham wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix: Use Map for O(1) column lookups in table width calculations #4142

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
25 changes: 25 additions & 0 deletions src/table/__tests__/columns-width.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -224,3 +224,28 @@ describe('with stickyHeader=true', () => {
]);
});
});

test('does not measure column widths when re-rendering with unchanged columns', () => {
const getBoundingClientRectSpy = jest.spyOn(Element.prototype, 'getBoundingClientRect');
const columns: TableProps.ColumnDefinition<Item>[] = [
{ id: 'id', header: 'id', cell: item => item.id, width: 150 },
{ id: 'text', header: 'text', cell: item => item.text, width: 200 },
];
const { rerender } = renderTable(
<Table columnDefinitions={columns} items={defaultItems} resizableColumns={true} stickyColumns={{ first: 1 }} />
);

// Clear call count after initial render
getBoundingClientRectSpy.mockClear();

// Re-render with different items but same columns
const newItems = [
{ id: 1, text: 'updated' },
{ id: 2, text: 'new item' },
];
rerender(<Table columnDefinitions={columns} items={newItems} resizableColumns={true} stickyColumns={{ first: 1 }} />);

expect(getBoundingClientRectSpy).not.toHaveBeenCalled();

getBoundingClientRectSpy.mockRestore();
});
36 changes: 19 additions & 17 deletions src/table/internal.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
// SPDX-License-Identifier: Apache-2.0
import React, { useCallback, useImperativeHandle, useRef } from 'react';
import React, { useCallback, useImperativeHandle, useMemo, useRef } from 'react';
import clsx from 'clsx';

import { useContainerQuery } from '@cloudscape-design/component-toolkit';
Expand Down Expand Up @@ -294,11 +294,10 @@ const InternalTable = React.forwardRef(
const { moveFocusDown, moveFocusUp, moveFocus } = useSelectionFocusMove(selectionType, allItems.length);
const { onRowClickHandler, onRowContextMenuHandler } = useRowEvents({ onRowClick, onRowContextMenu });

const visibleColumnDefinitions = getVisibleColumnDefinitions({
columnDefinitions,
columnDisplay,
visibleColumns,
});
const visibleColumnDefinitions = useMemo(
() => getVisibleColumnDefinitions({ columnDefinitions, columnDisplay, visibleColumns }),
[columnDefinitions, columnDisplay, visibleColumns]
);

const { isItemSelected, getSelectAllProps, getItemSelectionProps } = useSelection({
items: allItems,
Expand Down Expand Up @@ -344,17 +343,20 @@ const InternalTable = React.forwardRef(
headerIdRef.current = id;
}, []);

const visibleColumnWidthsWithSelection: ColumnWidthDefinition[] = [];
const visibleColumnIdsWithSelection: PropertyKey[] = [];
if (hasSelection) {
visibleColumnWidthsWithSelection.push({ id: selectionColumnId, width: SELECTION_COLUMN_WIDTH });
visibleColumnIdsWithSelection.push(selectionColumnId);
}
for (let columnIndex = 0; columnIndex < visibleColumnDefinitions.length; columnIndex++) {
const columnId = getColumnKey(visibleColumnDefinitions[columnIndex], columnIndex);
visibleColumnWidthsWithSelection.push({ ...visibleColumnDefinitions[columnIndex], id: columnId });
visibleColumnIdsWithSelection.push(columnId);
}
const { visibleColumnWidthsWithSelection, visibleColumnIdsWithSelection } = useMemo(() => {
const widths: ColumnWidthDefinition[] = [];
const ids: PropertyKey[] = [];
if (hasSelection) {
widths.push({ id: selectionColumnId, width: SELECTION_COLUMN_WIDTH });
ids.push(selectionColumnId);
}
for (let columnIndex = 0; columnIndex < visibleColumnDefinitions.length; columnIndex++) {
const columnId = getColumnKey(visibleColumnDefinitions[columnIndex], columnIndex);
widths.push({ ...visibleColumnDefinitions[columnIndex], id: columnId });
ids.push(columnId);
}
return { visibleColumnWidthsWithSelection: widths, visibleColumnIdsWithSelection: ids };
}, [hasSelection, visibleColumnDefinitions]);

const stickyState = useStickyColumns({
visibleColumns: visibleColumnIdsWithSelection,
Expand Down
16 changes: 9 additions & 7 deletions src/table/use-column-widths.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
// SPDX-License-Identifier: Apache-2.0
import React, { createContext, useContext, useEffect, useRef, useState } from 'react';
import React, { createContext, useContext, useEffect, useMemo, useRef, useState } from 'react';

import { useResizeObserver, useStableCallback } from '@cloudscape-design/component-toolkit/internal';
import { getLogicalBoundingClientRect } from '@cloudscape-design/component-toolkit/internal';
Expand Down Expand Up @@ -35,18 +35,17 @@ function readWidths(
}

function updateWidths(
visibleColumns: readonly ColumnWidthDefinition[],
column: ColumnWidthDefinition | undefined,
oldWidths: Map<PropertyKey, number>,
newWidth: number,
columnId: PropertyKey
) {
const column = visibleColumns.find(column => column.id === columnId);
let minWidth = DEFAULT_COLUMN_WIDTH;
if (typeof column?.width === 'number' && column.width < DEFAULT_COLUMN_WIDTH) {
minWidth = column?.width;
minWidth = column.width;
}
if (typeof column?.minWidth === 'number') {
minWidth = column?.minWidth;
minWidth = column.minWidth;
}
newWidth = Math.max(newWidth, minWidth);
if (oldWidths.get(columnId) === newWidth) {
Expand Down Expand Up @@ -83,6 +82,8 @@ export function ColumnWidthsProvider({ visibleColumns, resizableColumns, contain
const containerWidthRef = useRef(0);
const [columnWidths, setColumnWidths] = useState<null | Map<PropertyKey, number>>(null);

const columnById = useMemo(() => new Map(visibleColumns.map(column => [column.id, column])), [visibleColumns]);
Copy link
Member

@pan-kot pan-kot Jan 12, 2026

Choose a reason for hiding this comment

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

The visibleColumns is created on every render, so there is no reason to memoize this computation - it will still recompute every time.

Copy link
Member

@pan-kot pan-kot Jan 12, 2026

Choose a reason for hiding this comment

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

btw, it is not O(1) as specified in the PR description but O(n) - which is the cost of dictionary creation

Copy link
Contributor Author

@TrevorBurnham TrevorBurnham Jan 12, 2026

Choose a reason for hiding this comment

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

Good catch! I've updated this PR to memoize visibleColumns as well.

Copy link
Member

@pan-kot pan-kot Jan 13, 2026

Choose a reason for hiding this comment

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

But why all this memoization is needed? It adds a lot of clumsiness to the code but the performance impact of transforming columns is negligible even if there are 100 columns, which is rare.

Copy link
Contributor Author

@TrevorBurnham TrevorBurnham Jan 14, 2026

Choose a reason for hiding this comment

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

So, the memoization is actually a much bigger win than the Map change: Every time this useEffect gets triggered, it calls updateColumnWidths, which is potentially very expensive because it makes DOM measurements with getBoundingClientRect(). Doing that on table renders that don't affect the column widths is a recipe for making interactions feel slow. I'm sure you've experienced times where there's noticeable lag when typing in a Cloudscape table's search box; the updateColumnWidths call is likely a big contributor to that lag.

Copy link
Contributor Author

@TrevorBurnham TrevorBurnham Jan 14, 2026

Choose a reason for hiding this comment

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

I've updated this PR with a "does not measure column widths when re-rendering with unchanged columns" test, which I think is useful for validating the performance improvement here and preventing future regressions.


const cellsRef = useRef(new Map<PropertyKey, HTMLElement>());
const stickyCellsRef = useRef(new Map<PropertyKey, HTMLElement>());
const getCell = (columnId: PropertyKey): null | HTMLElement => cellsRef.current.get(columnId) ?? null;
Expand All @@ -96,7 +97,7 @@ export function ColumnWidthsProvider({ visibleColumns, resizableColumns, contain
};

const getColumnStyles = (sticky: boolean, columnId: PropertyKey): ColumnWidthStyle => {
const column = visibleColumns.find(column => column.id === columnId);
const column = columnById.get(columnId);
if (!column) {
return {};
}
Expand Down Expand Up @@ -190,7 +191,8 @@ export function ColumnWidthsProvider({ visibleColumns, resizableColumns, contain
}, []);

function updateColumn(columnId: PropertyKey, newWidth: number) {
setColumnWidths(columnWidths => updateWidths(visibleColumns, columnWidths ?? new Map(), newWidth, columnId));
const column = columnById.get(columnId);
setColumnWidths(columnWidths => updateWidths(column, columnWidths ?? new Map(), newWidth, columnId));
}

return (
Expand Down