Skip to content

Commit d61feee

Browse files
committed
refactor(plugin-grid): no synchronous setState in the whole-set read's effect, and a typed test double (objectui#12081 item 3)
The pending state needs no write: the read compares the held question with the render's. The new test file and hook carry no explicit any. Claude-Session: https://claude.ai/code/session_01AswpQDLCKiZos2jCXknwKz Co-authored-by: Claude <noreply@anthropic.com>
1 parent c482b60 commit d61feee

3 files changed

Lines changed: 30 additions & 23 deletions

File tree

‎packages/plugin-grid/src/__tests__/footerSummaryScope-12081.test.tsx‎

Lines changed: 25 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@ import { render, screen, cleanup, waitFor, fireEvent } from '@testing-library/re
4343
import '@testing-library/jest-dom';
4444
import React from 'react';
4545
import { parseFilterAST } from '@objectstack/spec/data';
46+
import type { DataSource, ObjectGridSchema } from '@object-ui/types';
4647

4748
import { ObjectGrid } from '../ObjectGrid';
4849
import { __clearRecordCrudVerdictCache } from '../hooks/useRecordCrudVerdicts';
@@ -53,7 +54,7 @@ import { ActionProvider, I18nProvider, notifyDataChanged } from '@object-ui/reac
5354
beforeAll(() => {
5455
registerAllFields();
5556
if (!Element.prototype.scrollIntoView) {
56-
Element.prototype.scrollIntoView = vi.fn() as any;
57+
Element.prototype.scrollIntoView = vi.fn() as unknown as Element['scrollIntoView'];
5758
}
5859
});
5960
beforeEach(() => {
@@ -68,7 +69,7 @@ afterEach(() => {
6869
const OBJECT = 'plan_line';
6970
const PAGE = 25;
7071

71-
interface Row { id: string; name: string; stage: string; amount: number; note: string | null }
72+
type Row = { id: string; name: string; stage: string; amount: number; note: string | null } & Record<string, unknown>;
7273
/** 300 rows; every fifth `note` is empty, every third row is `won`. */
7374
const ROWS: Row[] = Array.from({ length: 300 }, (_, i) => ({
7475
id: `r${i}`,
@@ -113,26 +114,29 @@ type AggregateQuery = { where?: unknown; groupBy?: string[]; aggregations?: Agg[
113114

114115
/** One aggregation node over a row set, as `engine.aggregate` answers it. */
115116
function aggregateOf(rows: Row[], node: Agg): number | null {
116-
const values = node.field === undefined ? [] : rows.map((r) => (r as any)[node.field!]).filter((v) => v !== null && v !== undefined);
117+
const values: unknown[] = node.field === undefined ? [] : rows.map((r) => r[node.field!]).filter((v) => v !== null && v !== undefined);
118+
const total = (): number => values.reduce((a: number, b) => a + Number(b), 0);
117119
switch (node.function) {
118120
case 'count': return node.field === undefined ? rows.length : values.length;
119121
case 'count_distinct': return new Set(values).size;
120-
case 'sum': return values.reduce((a, b) => a + Number(b), 0);
121-
case 'avg': return values.length === 0 ? null : values.reduce((a, b) => a + Number(b), 0) / values.length;
122+
case 'sum': return total();
123+
case 'avg': return values.length === 0 ? null : total() / values.length;
122124
case 'min': return values.length === 0 ? null : Math.min(...values.map(Number));
123125
case 'max': return values.length === 0 ? null : Math.max(...values.map(Number));
124126
default: throw new Error(`double: unsupported aggregation ${node.function}`);
125127
}
126128
}
127129

130+
type FilterInput = Parameters<typeof parseFilterAST>[0];
131+
128132
const matching = (where: unknown, search: unknown, searchFields: unknown) =>
129-
ROWS.filter((r) => matchesWhere(r as any, where) && matchesSearch(r as any, search, searchFields));
133+
ROWS.filter((r) => matchesWhere(r, where) && matchesSearch(r, search, searchFields));
130134

131135
function makeDataSource(opts: { door?: 'answers' | 'absent' | 'refuses' | 'no-row' | 'strings' } = {}) {
132136
const door = opts.door ?? 'answers';
133-
const ds: Record<string, any> = {
137+
const ds: Record<string, ReturnType<typeof vi.fn>> = {
134138
find: vi.fn(async (_object: string, params: Record<string, unknown>) => {
135-
const where = params.$filter === undefined ? undefined : parseFilterAST(params.$filter as any);
139+
const where = params.$filter === undefined ? undefined : parseFilterAST(params.$filter as FilterInput);
136140
const rows = matching(where, params.$search, params.$searchFields);
137141
const skip = (params.$skip as number | undefined) ?? 0;
138142
const top = (params.$top as number | undefined) ?? rows.length;
@@ -162,6 +166,8 @@ function makeDataSource(opts: { door?: 'answers' | 'absent' | 'refuses' | 'no-ro
162166
return ds;
163167
}
164168

169+
type TestDataSource = ReturnType<typeof makeDataSource>;
170+
165171
const sumOf = (rows: Row[]) => rows.reduce((a, r) => a + r.amount, 0);
166172
const fmt = (n: number) => n.toLocaleString('en');
167173

@@ -170,17 +176,17 @@ const AMOUNT_SUM = [
170176
{ field: 'amount', label: 'Amount', type: 'number', summary: 'sum' },
171177
];
172178

173-
const ownFetchGrid = (ds: any, columns: unknown[] = AMOUNT_SUM, extra: Record<string, unknown> = {}) => (
179+
const ownFetchGrid = (ds: TestDataSource, columns: unknown[] = AMOUNT_SUM, extra: Record<string, unknown> = {}) => (
174180
<ActionProvider>
175181
<ObjectGrid
176-
schema={{ type: 'object-grid', objectName: OBJECT, columns, pagination: { pageSize: PAGE }, ...extra } as any}
177-
dataSource={ds}
182+
schema={{ type: 'object-grid', objectName: OBJECT, columns, pagination: { pageSize: PAGE }, ...extra } as unknown as ObjectGridSchema}
183+
dataSource={ds as unknown as DataSource}
178184
/>
179185
</ActionProvider>
180186
);
181187

182188
/** The host's page, as `ListView` hands one down (objectui#4501's literal, key for key). */
183-
function hostGrid(ds: any, opts: {
189+
function hostGrid(ds: TestDataSource, opts: {
184190
rows: Row[];
185191
page?: number;
186192
findParams?: Record<string, unknown> | null;
@@ -195,8 +201,8 @@ function hostGrid(ds: any, opts: {
195201
return (
196202
<ActionProvider>
197203
<ObjectGrid
198-
schema={{ type: 'object-grid', objectName: OBJECT, columns: opts.columns ?? AMOUNT_SUM, pagination: { pageSize: PAGE } } as any}
199-
dataSource={ds}
204+
schema={{ type: 'object-grid', objectName: OBJECT, columns: opts.columns ?? AMOUNT_SUM, pagination: { pageSize: PAGE } } as unknown as ObjectGridSchema}
205+
dataSource={ds as unknown as DataSource}
200206
data={opts.rows.slice((page - 1) * PAGE, page * PAGE)}
201207
manualPagination
202208
rowCount={opts.rowCount ?? opts.rows.length}
@@ -261,7 +267,7 @@ describe('the footer reads the whole matched set when the grid shows one page of
261267
it('(c) a filter and a search narrow the count and the footer alike', async () => {
262268
const ds = makeDataSource();
263269
const filter = ['stage', '=', 'won'];
264-
const narrowed = matching(parseFilterAST(filter as any), 'Line 1', ['name']);
270+
const narrowed = matching(parseFilterAST(filter as FilterInput), 'Line 1', ['name']);
265271
// Not a coincidence the assertions could pass by: a strict subset, larger than a page.
266272
expect(narrowed.length).toBeGreaterThan(PAGE);
267273
expect(narrowed.length).toBeLessThan(ROWS.length);
@@ -283,7 +289,7 @@ describe('the footer reads the whole matched set when the grid shows one page of
283289
const [object, query] = ds.queryGroupHeaders.mock.calls[0];
284290
expect(object).toBe(OBJECT);
285291
expect(query).toEqual({
286-
where: parseFilterAST(filter as any),
292+
where: parseFilterAST(filter as FilterInput),
287293
search: 'Line 1',
288294
searchFields: ['name'],
289295
aggregations: [
@@ -430,8 +436,8 @@ describe('CONTROL: rows that ARE the whole set keep their footer and ask nothing
430436
render(
431437
<ActionProvider>
432438
<ObjectGrid
433-
schema={{ type: 'object-grid', objectName: OBJECT, columns: AMOUNT_SUM, data: { provider: 'value', items: rows } } as any}
434-
dataSource={ds as any}
439+
schema={{ type: 'object-grid', objectName: OBJECT, columns: AMOUNT_SUM, data: { provider: 'value', items: rows } } as unknown as ObjectGridSchema}
440+
dataSource={ds as unknown as DataSource}
435441
/>
436442
</ActionProvider>,
437443
);
@@ -442,7 +448,7 @@ describe('CONTROL: rows that ARE the whole set keep their footer and ask nothing
442448
it('a grid whose one page is the whole matched set', async () => {
443449
const ds = makeDataSource();
444450
const filter = ['amount', '>', 29000];
445-
const few = matching(parseFilterAST(filter as any), undefined, undefined);
451+
const few = matching(parseFilterAST(filter as FilterInput), undefined, undefined);
446452
expect(few.length).toBeLessThan(PAGE);
447453
render(ownFetchGrid(ds, AMOUNT_SUM, { filter }));
448454
await waitFor(() => expect(footerText('amount')).toBe(`Amount: Sum: ${fmt(sumOf(few))}`));

‎packages/plugin-grid/src/useColumnSummary.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -585,7 +585,7 @@ export function useColumnSummary(
585585
*/
586586
export function useScopedColumnSummary(
587587
columns: ListColumn[] | undefined,
588-
data: any[],
588+
data: SummaryRow[],
589589
fieldMetadata: SummaryFieldMetadata | undefined,
590590
scope: ColumnSummaryScope,
591591
wholeRow: ListViewGroupHeaderRow | undefined,

‎packages/plugin-grid/src/useWholeSetSummary.ts‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -205,9 +205,10 @@ export function useWholeSetSummary(input: WholeSetSummaryInput): WholeSetSummary
205205
useEffect(() => {
206206
if (!canAsk || !dataSource || !objectName) return;
207207
let cancelled = false;
208-
// Re-asking the question in hand keeps its answer on screen until the
209-
// new one lands; a different question starts with none.
210-
setHeld((prev) => (prev.question === question ? prev : { question, answer: PENDING }));
208+
// Nothing is set before the answer lands: re-asking the question in hand
209+
// keeps its answer on screen until the new one replaces it, and a
210+
// different question reads as pending by itself (the read below compares
211+
// the held question with this render's).
211212
const asked = (JSON.parse(question) as [string, EngineAggregateOptions])[1];
212213
// Through a resolved promise, so a source that throws synchronously is a
213214
// refusal like any other rather than a render error.

0 commit comments

Comments
 (0)