Skip to content

Commit a3ae404

Browse files
yinlianghuiclaude
andauthored
fix(components,plugin-dashboard): the static-data table widget renders instead of looping (#4618) (#4623)
* fix(components,plugin-dashboard): the static-data table widget renders instead of looping (#4618) Reported as a `table` dashboard widget crashing into the error boundary with "Maximum update depth exceeded", thrown at data-table.tsx:788 — a line in the sticky-offset layout effect that has nothing to do with the cause. Three hops, all inside data-table.tsx: the `columns: rawColumns = []` destructuring default evaluates a fresh array every render, an identity-keyed useMemo turns that into a fresh `initialColumns`, and the `useEffect(() => setColumns(initialColumns))` sync writes state for it — scheduling the render that regenerates the literal. React throws from the layout effect keyed on `columns` because that is the synchronous nested-update path it counts. Also fixes the two reasons the same widget rendered nothing once it stopped crashing: neither dashboard surface supplied the REQUIRED `columns` key, and DashboardGridLayout read the authored rows as `widgetData?.items` — `[]` for the array shape its sibling has always handled. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017Qqyix2QcnpUC9XeYVDzx3 * refactor(components): type the data-table loop guard without `any` (#4618) eslint net count vs origin/main for the two touched packages: 920 -> 918 warnings, all in @object-ui/components, and the delta is exactly the two `react-hooks/exhaustive-deps` warnings this card's fix removes — The 'data' conditional could make the dependencies of useMemo Hook (at line 759 / 899) change on every render. which is the reported defect's own shape, reported by the linter on main all along. The first draft of the guard added three `no-explicit-any` warnings back; `readonly unknown[]` plus a `Record< string, unknown >` narrowing, and `DataTableSchema['data']` for the shared empty, add none. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017Qqyix2QcnpUC9XeYVDzx3 --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent e028dfc commit a3ae404

8 files changed

Lines changed: 455 additions & 22 deletions

File tree

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
---
2+
"@object-ui/components": patch
3+
"@object-ui/plugin-dashboard": patch
4+
---
5+
6+
fix(components,plugin-dashboard): a static-data `table` widget renders instead of crashing
7+
8+
A dashboard widget authored as `{ type: 'table', options: { data: [ … ] } }` fell into the
9+
error boundary with "Maximum update depth exceeded" the moment its tile re-rendered, while
10+
every chart family on the identical static surface rendered clean.
11+
12+
- `data-table` no longer re-renders itself to death. Its `columns` / `data` fallbacks are
13+
module-scope empties instead of per-render array literals, and the prop→state column sync
14+
re-seeds on a value change rather than on a new identity — so a consumer that derives its
15+
columns each render (which both dashboard surfaces do) costs the table nothing.
16+
- Both dashboard surfaces now give the static table the `columns` key `DataTableSchema`
17+
requires, derived from the rows when the author declared none — the same derivation the
18+
`provider: 'object'` half of the widget family already performed. Previously such a table
19+
drew one empty row per record: no headers, no cells.
20+
- `DashboardGridLayout` reads an authored `options.data` ARRAY for its static table, which
21+
its `widgetData?.items` expression resolved to `[]`. `DashboardRenderer` had the arm all
22+
along.
Lines changed: 132 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,132 @@
1+
/**
2+
* ObjectUI
3+
* Copyright (c) 2024-present ObjectStack Inc.
4+
*
5+
* This source code is licensed under the MIT license found in the
6+
* LICENSE file in the root directory of this source tree.
7+
*/
8+
9+
/**
10+
* objectui#4618 — `data-table` must not re-render itself to death.
11+
*
12+
* The reported crash ("Maximum update depth exceeded", thrown at the
13+
* `setMeasuredStickyLefts` call in the sticky-offset layout effect) is NOT
14+
* about sticky offsets. The loop is three hops of prop→state synchronization,
15+
* all inside `data-table.tsx`:
16+
*
17+
* 1. `columns: rawColumns = []` — the destructuring DEFAULT. When the schema
18+
* carries no `columns` key, this evaluates a FRESH array literal on every
19+
* render, so `rawColumns` has a new identity each time even though the
20+
* schema object never changed.
21+
* 2. `initialColumns = useMemo(() => rawColumns.map(…), [rawColumns])` — an
22+
* identity-keyed memo over hop 1, so it recomputes to a new array each
23+
* render.
24+
* 3. `useEffect(() => setColumns(initialColumns), [initialColumns])` — the
25+
* prop→state sync. New identity ⇒ new state ⇒ re-render ⇒ hop 1 again.
26+
*
27+
* Self-sustaining, because the unstable value is derived INSIDE the component:
28+
* the table's own re-render regenerates it, so nothing external has to change.
29+
* React throws from the layout effect keyed on `columns` (the synchronous
30+
* nested-update path it counts), which is why the stack points at a line that
31+
* has nothing to do with the cause.
32+
*
33+
* Mount alone is safe — `useState(initialColumns)` seeds from the very array
34+
* the mount-render's effect then re-sets, so the first sync is an Object.is
35+
* no-op. The loop starts at render #2, i.e. the first time ANY host re-renders
36+
* the tile. That is why this went unseen: every existing suite renders once.
37+
*
38+
* The negative control below is the load-bearing half: with `columns` declared
39+
* (a stable array off the schema), the same host churn must stay flat — the
40+
* fix must not have been "stop syncing columns".
41+
*/
42+
import { describe, it, expect, afterEach } from 'vitest';
43+
import { render, cleanup, act } from '@testing-library/react';
44+
import React from 'react';
45+
import { ComponentRegistry } from '@object-ui/core';
46+
import '../data-table';
47+
48+
afterEach(cleanup);
49+
50+
const ROWS = [
51+
{ name: 'INV-1', amount: 100 },
52+
{ name: 'INV-2', amount: 200 },
53+
];
54+
55+
/**
56+
* Render `schema` (a STABLE object, declared once) inside a host that can
57+
* re-render on demand, counting the tile's commits. A host re-render is the
58+
* most ordinary thing on a dashboard — a filter change, a resize, a parent
59+
* state update — and must cost the tile a bounded number of commits.
60+
*/
61+
function renderUnderHostChurn(schema: Record<string, unknown>) {
62+
const DataTable = ComponentRegistry.get('data-table') as React.ComponentType<{ schema: unknown }>;
63+
if (!DataTable) throw new Error('data-table not registered');
64+
let commits = 0;
65+
let bump: (() => void) | null = null;
66+
const Host = () => {
67+
const [, setTick] = React.useState(0);
68+
bump = () => setTick((n) => n + 1);
69+
return (
70+
<React.Profiler id="tile" onRender={() => { commits += 1; }}>
71+
<DataTable schema={schema} />
72+
</React.Profiler>
73+
);
74+
};
75+
const utils = render(<Host />);
76+
return {
77+
...utils,
78+
churn: (times: number) => {
79+
for (let i = 0; i < times; i += 1) act(() => { bump!(); });
80+
},
81+
get commits() { return commits; },
82+
};
83+
}
84+
85+
describe('data-table survives host re-renders (#4618)', () => {
86+
it('does not exceed the update depth when the schema declares no columns', () => {
87+
// Pre-fix this threw on the FIRST host re-render, after ~50 nested commits:
88+
// "Maximum update depth exceeded. This can happen when a component
89+
// repeatedly calls setState inside componentWillUpdate or
90+
// componentDidUpdate…"
91+
const tile = renderUnderHostChurn({ data: ROWS, searchable: false, pagination: false, className: 'border-0' });
92+
expect(() => tile.churn(3)).not.toThrow();
93+
// Bounded, not merely finite: one commit per host render (plus the mount
94+
// pair). A regression that re-introduced a per-render state write would
95+
// still "not throw" while doubling this.
96+
expect(tile.commits).toBeLessThanOrEqual(6);
97+
});
98+
99+
it('keeps syncing declared columns — the negative control stays flat too', () => {
100+
const tile = renderUnderHostChurn({
101+
data: ROWS,
102+
columns: [{ header: 'Name', accessorKey: 'name' }, { header: 'Amount', accessorKey: 'amount' }],
103+
searchable: false,
104+
pagination: false,
105+
});
106+
expect(() => tile.churn(3)).not.toThrow();
107+
expect(tile.commits).toBeLessThanOrEqual(6);
108+
// Declared columns still reach the DOM — the sync was hardened, not removed.
109+
const headers = Array.from(tile.container.querySelectorAll('th')).map((th) => th.textContent);
110+
expect(headers.join('|')).toContain('Name');
111+
expect(headers.join('|')).toContain('Amount');
112+
expect(tile.container.textContent).toContain('INV-1');
113+
});
114+
115+
it('re-syncs when the host actually changes the declared columns', () => {
116+
const DataTable = ComponentRegistry.get('data-table') as React.ComponentType<{ schema: unknown }>;
117+
let setCols: ((c: Array<Record<string, unknown>>) => void) | null = null;
118+
const Host = () => {
119+
const [cols, set] = React.useState<Array<Record<string, unknown>>>([
120+
{ header: 'Name', accessorKey: 'name' },
121+
]);
122+
setCols = set;
123+
return <DataTable schema={{ data: ROWS, columns: cols, searchable: false, pagination: false }} />;
124+
};
125+
const { container } = render(<Host />);
126+
expect(container.textContent).toContain('Name');
127+
expect(container.textContent).not.toContain('Amount');
128+
act(() => { setCols!([{ header: 'Name', accessorKey: 'name' }, { header: 'Amount', accessorKey: 'amount' }]); });
129+
expect(container.textContent).toContain('Amount');
130+
expect(container.textContent).toContain('200');
131+
});
132+
});

packages/components/src/renderers/complex/data-table.tsx

Lines changed: 63 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -595,6 +595,54 @@ function resolveSelectionMode(selectable: DataTableSchema['selectable'] | 'none'
595595
return 'multiple';
596596
}
597597

598+
/**
599+
* Shared empty fallbacks for the `columns` / `data` props (objectui#4618).
600+
*
601+
* A destructuring default (`columns: rawColumns = []`) or an inline fallback
602+
* (`Array.isArray(raw) ? raw : []`) evaluates a FRESH array on every render, so
603+
* an absent prop churns identity on a schema that never changed. Downstream
604+
* that identity is a `useMemo` key and — for `columns` — a `useEffect` key whose
605+
* body writes state, which closes a self-sustaining render loop: the table's own
606+
* re-render regenerates the literal that scheduled it. Hoisting the empties to
607+
* module scope makes "absent" a stable value, so the memo and the effect see
608+
* what is actually true — nothing changed.
609+
*
610+
* Frozen so a consumer that mutates the array it was handed cannot corrupt the
611+
* shared instance for every other table on the page.
612+
*/
613+
const EMPTY_COLUMNS = Object.freeze([]) as unknown as DataTableSchema['columns'];
614+
const EMPTY_ROWS = Object.freeze([]) as unknown as DataTableSchema['data'];
615+
616+
/**
617+
* Value-equality over two normalized column lists (objectui#4618).
618+
*
619+
* The prop→state sync below re-seeds `columns` whenever `initialColumns` is a
620+
* new object. That is the right trigger for a real change and the wrong one for
621+
* identity churn, which every consumer that derives its columns per render
622+
* produces — `ObjectDataTable` and both dashboard surfaces build the node fresh
623+
* on each of their renders. Comparing the values instead lets the sync stay
624+
* exactly as eager as before for genuine edits (a renamed header, an added
625+
* column, a hidden one) while a re-derived-but-identical list costs nothing.
626+
*
627+
* Shallow per column, on purpose: column entries carry render functions
628+
* (`cell`, `render`), and comparing those by identity is what the sync already
629+
* did — deep-comparing them is neither possible nor wanted.
630+
*/
631+
function columnsAreEquivalent(a: readonly unknown[], b: readonly unknown[]): boolean {
632+
if (a === b) return true;
633+
if (a.length !== b.length) return false;
634+
return a.every((col, i) => {
635+
const other = b[i];
636+
if (col === other) return true;
637+
if (!col || !other || typeof col !== 'object' || typeof other !== 'object') return false;
638+
const left = col as Record<string, unknown>;
639+
const right = other as Record<string, unknown>;
640+
const keys = Object.keys(left);
641+
if (keys.length !== Object.keys(right).length) return false;
642+
return keys.every((k) => Object.is(left[k], right[k]));
643+
});
644+
}
645+
598646
/**
599647
* Enterprise-level data table component with Airtable-like features.
600648
*
@@ -634,8 +682,9 @@ function resolveSelectionMode(selectable: DataTableSchema['selectable'] | 'none'
634682
const DataTableRenderer = ({ schema }: { schema: DataTableSchema }) => {
635683
const {
636684
caption,
637-
columns: rawColumns = [],
638-
data: rawData = [],
685+
// Module-scope empties, never `[]` literals — see EMPTY_COLUMNS/EMPTY_ROWS.
686+
columns: rawColumns = EMPTY_COLUMNS,
687+
data: rawData = EMPTY_ROWS,
639688
pagination = true,
640689
pageSize: initialPageSize = 10,
641690
pageSizeOptions,
@@ -715,8 +764,10 @@ const DataTableRenderer = ({ schema }: { schema: DataTableSchema }) => {
715764
}, [language]);
716765

717766
// Ensure data is always an array – provider config objects or null/undefined
718-
// must not reach array operations like .filter() / .some()
719-
const data = Array.isArray(rawData) ? rawData : [];
767+
// must not reach array operations like .filter() / .some(). The non-array
768+
// fallback is the shared empty, so a provider-config schema does not re-key
769+
// every downstream memo on each render (objectui#4618).
770+
const data = Array.isArray(rawData) ? rawData : EMPTY_ROWS;
720771

721772
// Normalize columns to support legacy keys (label/name) from existing JSONs
722773
const initialColumns = useMemo(() => {
@@ -862,9 +913,15 @@ const DataTableRenderer = ({ schema }: { schema: DataTableSchema }) => {
862913
// pending change existed" from "the pending value was undefined".
863914
const editRevertRef = useRef<{ had: boolean; value: any } | null>(null);
864915

865-
// Update columns when schema changes
916+
// Update columns when schema changes.
917+
//
918+
// Re-seed only on a real change: `initialColumns` is a fresh array whenever
919+
// its memo recomputes, and every consumer that derives columns per render
920+
// hands us one. Writing state for a value-identical list re-renders the whole
921+
// table for nothing, and — when the churn originates inside this component —
922+
// schedules the render that regenerates the churn (objectui#4618).
866923
useEffect(() => {
867-
setColumns(initialColumns);
924+
setColumns((prev) => (columnsAreEquivalent(prev, initialColumns) ? prev : initialColumns));
868925
}, [initialColumns]);
869926

870927
// Clear the internal checkbox selection when the host bumps selectionResetKey.

packages/plugin-dashboard/src/DashboardGridLayout.tsx

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ import { Edit, GripVertical, Save, X, RefreshCw } from 'lucide-react';
66
import { SchemaRenderer, useHasDndProvider, useDnd } from '@object-ui/react';
77
import { useObjectTranslation, pickLocalized } from '@object-ui/i18n';
88
import type { BaseSchema, DashboardComponentSchema, DashboardWidgetSchema } from '@object-ui/types';
9-
import { isObjectProvider } from './utils';
9+
import { isObjectProvider, deriveStaticTableColumns } from './utils';
1010
import { classifyWidgetType } from './widgetDispatch';
1111
import { LEGACY_RETIRED_WIDGET_SCHEMA, isLegacyRetiredWidget } from './legacyRetiredWidget';
1212
import { DatasetWidget } from './DatasetWidget';
@@ -313,10 +313,21 @@ export const DashboardGridLayout: React.FC<DashboardGridLayoutProps> = ({
313313
};
314314
}
315315

316+
// Static (data-array) table. The `Array.isArray` arm is not decoration:
317+
// `options.data` for this widget is authored as a plain ARRAY, and
318+
// `widgetData?.items` is `undefined` for one — so this branch resolved
319+
// every authored static table to `[]` while `DashboardRenderer`'s mirror
320+
// of it read the array all along (objectui#4618).
321+
const staticRows = Array.isArray(widgetData) ? widgetData : widgetData?.items || [];
316322
return {
317323
type: 'data-table',
318324
...options,
319-
data: widgetData?.items || [],
325+
data: staticRows,
326+
// See DashboardRenderer: `columns` is required, and an author who
327+
// declared none got a table of empty rows. Explicit columns win.
328+
columns: Array.isArray(options.columns) && options.columns.length > 0
329+
? options.columns
330+
: deriveStaticTableColumns(staticRows),
320331
searchable: false,
321332
pagination: false,
322333
className: "border-0"

packages/plugin-dashboard/src/DashboardRenderer.tsx

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ import {
3737
verticalListSortingStrategy,
3838
} from '@dnd-kit/sortable';
3939
import { CSS } from '@dnd-kit/utilities';
40-
import { isObjectProvider } from './utils';
40+
import { isObjectProvider, deriveStaticTableColumns } from './utils';
4141
import { classifyWidgetType, METRIC_LIKE_TYPES } from './widgetDispatch';
4242
import { LEGACY_RETIRED_WIDGET_SCHEMA, isLegacyRetiredWidget } from './legacyRetiredWidget';
4343
import { DatasetWidget } from './DatasetWidget';
@@ -686,10 +686,20 @@ const DashboardRendererInner = forwardRef<HTMLDivElement, DashboardRendererProps
686686
}
687687

688688
// Static (data-array) table — drill-to-record stays opt-in.
689+
const staticRows = Array.isArray(widgetData) ? widgetData : widgetData?.items || [];
689690
return {
690691
type: 'data-table',
691692
...options,
692-
data: Array.isArray(widgetData) ? widgetData : widgetData?.items || [],
693+
data: staticRows,
694+
// `columns` is a REQUIRED key of `DataTableSchema`, and this
695+
// branch used to omit it whenever the author declared none —
696+
// a table of empty rows, no headers, no cells (objectui#4618).
697+
// Derive from the rows, as the `provider: 'object'` half of
698+
// this same family already does. An explicit list always wins:
699+
// a declared column set is a whitelist, never a starting point.
700+
columns: Array.isArray(options.columns) && options.columns.length > 0
701+
? options.columns
702+
: deriveStaticTableColumns(staticRows),
693703
searchable: false,
694704
pagination: false,
695705
className: "border-0"

packages/plugin-dashboard/src/ObjectDataTable.tsx

Lines changed: 4 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ import { extractRecords, isDrillEnabled } from '@object-ui/core';
1212
import type { DrillDownConfig } from '@object-ui/types';
1313
import { Skeleton, RefreshIndicator, cn } from '@object-ui/components';
1414
import { useSafeFieldLabel, useObjectTranslation, useLocalization, useDisplayLocale } from '@object-ui/i18n';
15-
import { resolveFilterPlaceholders } from './utils';
15+
import { resolveFilterPlaceholders, humanizeFieldKey } from './utils';
1616
import {
1717
buildFieldMeta,
1818
renderFieldValue,
@@ -56,17 +56,9 @@ interface NormalizedColumn {
5656
export function normalizeColumns(columns: (string | Record<string, any>)[]): NormalizedColumn[] {
5757
return columns.map((col) => {
5858
if (typeof col === 'string') {
59-
return {
60-
header: col
61-
// snake_case → spaces
62-
.replace(/_/g, ' ')
63-
// camelCase → spaces before uppercase letters
64-
.replace(/([A-Z])/g, ' $1')
65-
.trim()
66-
// Title Case each word
67-
.replace(/\b\w/g, (c: string) => c.toUpperCase()),
68-
accessorKey: col,
69-
};
59+
// Shared with the static-table derivation so both halves of the `table`
60+
// widget family spell a header the same way (objectui#4618).
61+
return { header: humanizeFieldKey(col), accessorKey: col };
7062
}
7163
return col as NormalizedColumn;
7264
});

0 commit comments

Comments
 (0)