Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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 libs/ui/src/lib/data-table/grid/__tests__/buildColumnDefs.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,18 @@ describe('buildColumnDefs', () => {
expect(defs.map((def) => def.id)).toEqual([ACTION_COLUMN_KEY, SELECT_COLUMN_KEY, 'Name', 'Amount']);
});

test('falls back to a positional id when a column key is missing (TanStack throws on a falsy id)', () => {
// `key` is typed as string, but malformed columns can reach here from `any` query-result data.
const malformed = [
{ key: undefined as unknown as string, name: 'Broken' },
{ key: '', name: 'Empty' },
];
const defs = buildColumnDefs(malformed as ColumnWithFilter<Row>[]);
expect(defs[0].id).toBe('__jgrid_col_0');
expect(defs[1].id).toBe('__jgrid_col_1');
defs.forEach((def) => expect(typeof def.id).toBe('string'));
});

test('classifies cellKind for select/action/data columns', () => {
const defs = buildColumnDefs(columns);
expect(metaOf(defs[0]).cellKind).toBe('action');
Expand All @@ -42,6 +54,19 @@ describe('buildColumnDefs', () => {
expect('accessorFn' in defs[2]).toBe(true);
});

test('accessor indexes by the normalized id; a malformed key reads the fallback id, not row["undefined"]', () => {
const malformed = [
{ key: 'Name', name: 'Name' },
{ key: undefined as unknown as string, name: 'Broken' },
];
const defs = buildColumnDefs(malformed as ColumnWithFilter<Row>[]);
const accessorOf = (def: (typeof defs)[number]) => (def as unknown as { accessorFn: (row: unknown) => unknown }).accessorFn;
// `undefined`/`''` props would be picked up if the accessor still indexed by a falsy raw key.
const row = { Name: 'Alpha', undefined: 'LEAK', '': 'LEAK' };
expect(accessorOf(defs[0])(row)).toBe('Alpha');
expect(accessorOf(defs[1])(row)).toBeUndefined();
});

test('sizing maps width/min/max; string/absent width falls back to default', () => {
const defs = buildColumnDefs(columns);
expect(defs[2].size).toBe(250);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,13 @@ describe('useJetstreamTable', () => {
expect(result.current.table.getRowModel().rows).toHaveLength(3);
});

test('coerces row ids to strings even when getRowKey returns a non-string', () => {
// A numeric row id would break string ops (e.g. rowId.startsWith() inside isSummaryRowId) during keyboard nav.
const { result } = setup({ getRowKey: (row) => Number(row._key) as unknown as string });
result.current.table.getRowModel().rows.forEach((row) => expect(typeof row.id).toBe('string'));
expect(result.current.table.getRowModel().rows.map((row) => row.id)).toEqual(['1', '2', '3']);
});

test('quick filter narrows rows case-insensitively', () => {
const { result } = setup({ quickFilterText: 'ALPHA' });
const rows = result.current.table.getRowModel().rows.map((row) => row.original);
Expand Down
18 changes: 13 additions & 5 deletions libs/ui/src/lib/data-table/grid/buildColumnDefs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ export function buildColumnDefs<TRow, TSummaryRow = unknown>(
columns: ColumnWithFilter<TRow, TSummaryRow>[],
defaultColumnOptions?: DefaultColumnOptions<TRow, TSummaryRow>,
): ColumnDef<TRow, unknown>[] {
return columns.map((column) => toColumnDef(column, defaultColumnOptions));
return columns.map((column, index) => toColumnDef(column, index, defaultColumnOptions));
}

function resolveCellKind(key: string): CellKind {
Expand All @@ -31,11 +31,17 @@ function resolveCellKind(key: string): CellKind {

function toColumnDef<TRow, TSummaryRow>(
column: ColumnWithFilter<TRow, TSummaryRow>,
index: number,
defaultColumnOptions?: DefaultColumnOptions<TRow, TSummaryRow>,
): ColumnDef<TRow, unknown> {
const cellKind = resolveCellKind(column.key);
const isDataColumn = cellKind === 'data';

// TanStack throws a bare `Error` while building any column whose resolved id is falsy. `key` is typed
// as a string but originates from `any` query-result data upstream, so fall back to a positional id
// rather than crashing the whole grid when a malformed column slips through.
const columnId = column.key != null && String(column.key).length > 0 ? String(column.key) : `__jgrid_col_${index}`;

Comment thread
paustint marked this conversation as resolved.
const resizable = column.resizable ?? defaultColumnOptions?.resizable ?? false;
const sortable = isDataColumn && (column.sortable ?? defaultColumnOptions?.sortable ?? false);
const hasFilters = isDataColumn && Array.isArray(column.filters) && column.filters.length > 0;
Expand All @@ -59,10 +65,12 @@ function toColumnDef<TRow, TSummaryRow>(
};

const columnDef: ColumnDef<TRow, unknown> = {
id: column.key,
// Display (select/action) columns have no meaningful accessor.
...(isDataColumn ? { accessorFn: (row: TRow) => (row as Record<string, unknown>)[column.key] } : {}),
header: typeof column.name === 'string' ? column.name : column.key,
id: columnId,
// Display (select/action) columns have no meaningful accessor. Data columns index by `columnId`
// (not raw `column.key`) so a malformed/falsy key reads the unused fallback id — an empty column —
// instead of the accidental `row['undefined']` lookup, keeping the accessor aligned with the id.
...(isDataColumn ? { accessorFn: (row: TRow) => (row as Record<string, unknown>)[columnId] } : {}),
header: typeof column.name === 'string' ? column.name : columnId,
Comment thread
paustint marked this conversation as resolved.
enableSorting: sortable,
enableResizing: resizable,
enableColumnFilter: hasFilters,
Expand Down
5 changes: 4 additions & 1 deletion libs/ui/src/lib/data-table/grid/core/useJetstreamTable.ts
Original file line number Diff line number Diff line change
Expand Up @@ -341,7 +341,10 @@ export function useJetstreamTable<TRow = RowWithKey>(options: UseJetstreamTableO
data,
columns: columnDefs,
state: { sorting, columnFilters, globalFilter, columnOrder, columnSizing, rowSelection, grouping: groupingState, expanded },
getRowId: (row) => stableGetRowKey(row),
// Coerce to string: `getRowKey` is typed to return a string but some callers derive it from `any`
// record data (e.g. a numeric id), and a non-string `row.id` later breaks string ops such as the
// `rowId.startsWith(...)` inside `isSummaryRowId` during keyboard navigation.
getRowId: (row) => String(stableGetRowKey(row)),
enableRowSelection: enableRowSelection ?? false,
enableMultiSort,
enableSortingRemoval: true,
Expand Down
5 changes: 3 additions & 2 deletions libs/ui/src/lib/data-table/grid/grid-column-utils.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -379,8 +379,9 @@ export function reorderColumnOrder(order: string[], sourceId: string, targetId:
export function addFieldLabelToColumn(columnDefinitions: ColumnWithFilter<RowWithKey>[], fieldMetadata: Record<string, Field>) {
if (fieldMetadata) {
return columnDefinitions.map((col) => {
if (fieldMetadata[col.key?.toLowerCase()]?.label) {
const label = fieldMetadata[col.key.toLowerCase()].label;
const normalizedKey = col.key?.toLowerCase();
const label = normalizedKey ? fieldMetadata[normalizedKey]?.label : undefined;
if (label) {
return { ...col, name: `${col.name} (${label})` };
}
Comment thread
paustint marked this conversation as resolved.
return col;
Expand Down
2 changes: 1 addition & 1 deletion libs/ui/src/lib/data-table/grid/grid-constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ export const HEADER_ROW_ID = '__jgrid_header__';
*/
export const SUMMARY_ROW_ID_PREFIX = '__jgrid_summary__';
export const getSummaryRowId = (index: number): string => `${SUMMARY_ROW_ID_PREFIX}${index}`;
export const isSummaryRowId = (rowId: string): boolean => rowId.startsWith(SUMMARY_ROW_ID_PREFIX);
export const isSummaryRowId = (rowId: string): boolean => typeof rowId === 'string' && rowId.startsWith(SUMMARY_ROW_ID_PREFIX);
Comment thread
paustint marked this conversation as resolved.
export const getSummaryRowIndex = (rowId: string): number => Number.parseInt(rowId.slice(SUMMARY_ROW_ID_PREFIX.length), 10);

export const ACTION_COLUMN_KEY = '_actions';
Expand Down
Loading