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
13 changes: 13 additions & 0 deletions .changeset/objectgrid-external-pagination-props-4277.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
---
'@object-ui/plugin-grid': minor
---

ObjectGrid's host-driven pagination mode is a declared interface instead of twelve `(rest as any)` reads

`ObjectGridProps` declared twelve members while the component read twelve more out of `...rest`, each through an `as any` cast: `data`, `manualPagination`, `rowCount`, `page`, `pageSize`, `onPageChange`, `onPageSizeChange`, `sort`, `onSortChange`, `search`, `onSearchChange` and `onColumnStateChange`. They are not accidental — together they are the host-driven external-pagination path from framework#2212, where a host has already fetched one window of a larger collection and drives the page/sort/search controls itself, and the component's own comment said so. They were simply declared nowhere, so no call site could be checked against them and no editor could offer them.

Nothing had caught it because the only untyped caller is `ObjectGridRenderer`, whose `{ schema: any; [key: string]: any }` index signature accepts anything; every typed caller happens to pass only declared props; and the test that exercises the path was compiled by nothing.

They now live on a named `ObjectGridExternalPaginationProps`, which `ObjectGridProps` extends — a separate interface rather than twelve more members flattened into the authoring surface, so the "advanced host-driven mode" boundary stays visible. The eleven members that already have a counterpart on `DataTableSchema` — the type ObjectGrid forwards them to — are **type-derived** from that declaration (`Partial< Pick< DataTableSchema, … > >`) rather than hand-copied, so the two cannot drift apart; only `onColumnStateChange` is declared explicitly, because the table vocabulary reports per-event `onColumnResize` / `onColumnReorder` rather than the merged `{ order, widths }` layout this reports. `ObjectGridColumnState` is exported for that payload.

Purely additive for callers: every member is optional, so existing code compiles unchanged, and hosts that were already passing these props now get them checked instead of silently accepted. Runtime behavior is unchanged.
2 changes: 1 addition & 1 deletion packages/plugin-grid/package.json
Original file line numberDiff line numberDiff line change
Expand Up@@ -17,7 +17,7 @@
"scripts": {
"build": "vite build",
"test": "vitest run",
"type-check": "tsc --noEmit && tsc -p tsconfig.typetests.json",
"type-check": "tsc --noEmit && tsc -p tsconfig.typetests.json && tsc -p tsconfig.test.json",
"lint": "eslint ."
},
"dependencies": {
Expand Down
136 changes: 110 additions & 26 deletions packages/plugin-grid/src/ObjectGrid.tsx
Original file line numberDiff line numberDiff line change
Expand Up@@ -22,7 +22,7 @@
*/

import React, { useEffect, useState, useCallback, useMemo } from 'react';
import type { ObjectGridSchema, DataSource, ListColumn, ViewData, TableSortItem } from '@object-ui/types';
import type { ObjectGridSchema, DataSource, ListColumn, ViewData, TableSortItem, DataTableSchema } from '@object-ui/types';
import { isSystemManagedField } from '@object-ui/types';
import type { I18nLabel } from '@objectstack/spec/ui';
import { SchemaRenderer, useDataScope, useNavigationOverlay, useAction, useSafeFieldLabel, usePredicateScope } from '@object-ui/react';
Expand DownExpand Up@@ -157,7 +157,78 @@ function resolveColumnLabel(label: string | I18nLabel | undefined): string | und
return typeof label === 'string' ? label : undefined;
}

export interface ObjectGridProps {
/**
* The column layout ObjectGrid persists and reports back — the merged result of
* a resize and a reorder, not either event on its own. Named so the state hook
* below and the `onColumnStateChange` prop cannot drift apart.
*/
export interface ObjectGridColumnState {
order?: string[];
widths?: Record<string, number>;
}

/**
* The HOST-DRIVEN ("external") mode of ObjectGrid — framework#2212.
*
* The ordinary authoring surface hands ObjectGrid a `schema` and a `dataSource`
* and lets it fetch, page, sort and search for itself. In this mode a host
* (ListView, a designer preview, an app screen with its own toolbar) has
* already fetched one window of a larger collection and drives the controls
* itself: it passes the window as `data` plus the real match total and the
* page/sort/search state, and ObjectGrid forwards them straight to its
* DataTable instead of client-slicing the window it was handed.
*
* Kept as its own named interface rather than flattened into `ObjectGridProps`
* (#4277 裁决 B, 2026-08-11): the two are different classes of contract, and a
* dozen more members merged into the authoring surface would erase that
* boundary. Until this existed, every member below was read out of `...rest`
* through an `as any` cast and was declared nowhere at all.
*
* DERIVATION (#4277 裁决 §3, the anti-drift pin): this vocabulary is already
* declared once, on `DataTableSchema` — which is exactly where ObjectGrid
* forwards it — so the members that have a counterpart there are TYPE-DERIVED
* from that declaration rather than hand-copied into a second enumeration. Two
* hand-written copies of one vocabulary is how the next drift happens. The
* `Partial<...>` wrapper is deliberate and is the only shape change: the whole
* mode is opt-in, and `DataTableSchema['data']` is required because a table
* always has rows, while a grid that was given no `data` fetches its own.
*/
export interface ObjectGridExternalPaginationProps
extends Partial<
Pick<
DataTableSchema,
// The host's already-fetched window. Highest-priority data source: it
// wins over `schema.data` / `schema.bind` when present.
| 'data'
// Turns off client slicing. With `rowCount` + `onPageChange` it is what
// makes the mode active at all (see `externalManualPagination` below).
| 'manualPagination'
| 'rowCount'
| 'page'
| 'pageSize'
| 'onPageChange'
| 'onPageSizeChange'
| 'sort'
| 'onSortChange'
| 'search'
| 'onSearchChange'
>
> {
/**
* Grid-only: `DataTableSchema` has no counterpart to derive from.
*
* The table vocabulary reports column changes as separate per-event
* callbacks — `onColumnResize(columnKey, width)` and
* `onColumnReorder(newOrder)` — whereas this reports the MERGED, persisted
* `{ order, widths }` layout after ObjectGrid has folded either event into
* the state it also writes to `localStorage`, so a host can save one blob
* through `dataSource.updateViewConfig`. Deriving it from either table
* callback would misstate both the payload and when it fires.
*/
onColumnStateChange?: (state: ObjectGridColumnState) => void;
}

export interface ObjectGridProps extends ObjectGridExternalPaginationProps {
schema: ObjectGridSchema;
dataSource?: DataSource;
className?: string;
Expand DownExpand Up@@ -266,7 +337,23 @@ export const ObjectGrid: React.FC<ObjectGridProps> = ({
onRowSave,
onBatchSave,
onAddRecord,
...rest
// The host-driven mode (`ObjectGridExternalPaginationProps`). Every one of
// these was read out of `...rest` through an `as any` cast until #4277 gave
// them a declaration; they are ordinary typed props now, and `rest` is gone
// with them. Renamed on the way in only where the component already owns the
// plain name (`data` is the fetched rows, `pageSize` the schema's).
data: passedData,
manualPagination: hostManualPagination,
rowCount: hostRowCount,
page: hostPage,
pageSize: hostPageSize,
onPageChange: hostOnPageChange,
onPageSizeChange: hostOnPageSizeChange,
sort: hostSort,
onSortChange: hostOnSortChange,
search: hostSearch,
onSearchChange: hostOnSearchChange,
onColumnStateChange,
}) => {
const [data, setData] = useState<any[]>([]);
const [loading, setLoading] = useState(true);
Expand DownExpand Up@@ -317,10 +404,7 @@ export const ObjectGrid: React.FC<ObjectGridProps> = ({
: `grid-columns-${schema.objectName}`;
}, [schema.objectName, schema.id]);

const [columnState, setColumnState] = useState<{
order?: string[];
widths?: Record<string, number>;
}>(() => {
const [columnState, setColumnState] = useState<ObjectGridColumnState>(() => {
// Priority: 1) externally provided (e.g. persisted view override),
// 2) localStorage (per-browser fallback), 3) empty.
const fromProps = (schema as any).columnState;
Expand DownExpand Up@@ -356,11 +440,10 @@ export const ObjectGrid: React.FC<ObjectGridProps> = ({
console.warn('Failed to persist column state:', e);
}
// Notify parent so it can persist via dataSource.updateViewConfig.
const onChange = (rest as any).onColumnStateChange;
if (typeof onChange === 'function') {
try { onChange(state); } catch (e) { console.warn('onColumnStateChange threw:', e); }
if (typeof onColumnStateChange === 'function') {
try { onColumnStateChange(state); } catch (e) { console.warn('onColumnStateChange threw:', e); }
}
}, [columnStorageKey, rest]);
}, [columnStorageKey, onColumnStateChange]);

const handlePullRefresh = useCallback(async () => {
setRefreshKey(k => k + 1);
Expand All@@ -380,8 +463,8 @@ export const ObjectGrid: React.FC<ObjectGridProps> = ({
return () => window.removeEventListener('resize', checkWidth);
}, []);

// Check if data is passed directly (from ListView)
const passedData = (rest as any).data;
// `passedData` — data handed down directly (from ListView) — is destructured
// from props above.

// Resolve bound data if 'bind' property exists
const boundData = useDataScope(schema.bind);
Expand DownExpand Up@@ -415,11 +498,12 @@ export const ObjectGrid: React.FC<ObjectGridProps> = ({
// real match total + page controls. We must forward those straight to DataTable
// instead of client-slicing the window — otherwise the footer would report
// "pages = window / pageSize" and records beyond the window stay unreachable
// (framework #2212). `data` arrives via `rest` (a prop), so do these too.
// (framework #2212). `data` is a prop, and so are these — all declared on
// `ObjectGridExternalPaginationProps` since #4277.
const externalManualPagination =
(rest as any).manualPagination === true &&
typeof (rest as any).rowCount === 'number' &&
typeof (rest as any).onPageChange === 'function';
hostManualPagination === true &&
typeof hostRowCount === 'number' &&
typeof hostOnPageChange === 'function';

// Extract stable primitive/reference-stable values from schema for dependency arrays.
// This prevents infinite re-render loops when schema is a new object on each render
Expand DownExpand Up@@ -2159,16 +2243,16 @@ export const ObjectGrid: React.FC<ObjectGridProps> = ({
? schema.searchableFields.length > 0
: (schema.showSearch !== undefined ? schema.showSearch : true);

const manualRowCount = externalManualPagination ? (rest as any).rowCount : totalMatching;
const manualPage = externalManualPagination ? (rest as any).page : serverPage;
const manualRowCount = externalManualPagination ? hostRowCount : totalMatching;
const manualPage = externalManualPagination ? hostPage : serverPage;
const manualPageSize = externalManualPagination
? ((rest as any).pageSize ?? serverPageSize)
? (hostPageSize ?? serverPageSize)
: serverPageSize;
const manualOnPageChange = externalManualPagination
? (rest as any).onPageChange
? hostOnPageChange
: setServerPage;
const manualOnPageSizeChange = externalManualPagination
? (rest as any).onPageSizeChange
? hostOnPageSizeChange
: (size: number) => { setServerPageSize(size); setServerPage(1); };

// Before anyone clicks, the headers show the sort the view was authored with
Expand All@@ -2185,10 +2269,10 @@ export const ObjectGrid: React.FC<ObjectGridProps> = ({
schemaSort ?? (schema.defaultSort ? [schema.defaultSort] : undefined),
);
const manualSort: TableSortItem[] = externalManualPagination
? ((rest as any).sort ?? [])
? (hostSort ?? [])
: (headerSort ?? declaredSort);
const manualOnSortChange = externalManualPagination
? (rest as any).onSortChange
? hostOnSortChange
: setHeaderSort;

// The search term, in whichever server mode applies. When a parent owns the
Expand All@@ -2198,10 +2282,10 @@ export const ObjectGrid: React.FC<ObjectGridProps> = ({
// turns it into `$search`. A parent that drives the rows but offers no
// `onSearchChange` gets NO box rather than one scoped to its window.
const manualSearch = externalManualPagination
? ((rest as any).search ?? '')
? (hostSearch ?? '')
: searchTerm;
const manualOnSearchChange = externalManualPagination
? (rest as any).onSearchChange
? hostOnSearchChange
: setSearchTerm;

const dataTableSchema: any = {
Expand Down
Original file line numberDiff line numberDiff line change
Expand Up@@ -30,9 +30,24 @@ function pasteRows(text: string) {
act(() => { window.dispatchEvent(evt); });
}

// A data source offering none of the optional import capabilities. These cases
// never leave the mapping step, so nothing on it is ever called — but the wizard
// probes it for `listImportMappings` / `importRecords` / `listImportJobs` on
// render, and an empty object is the honest answer to all three: "a source that
// supports none of them". `ImportWizardProps.dataSource` is required, so it has
// to be passed rather than omitted (the omission was invisible until #4040 let
// `tsc` read this file).
const NO_CAPABILITY_DATA_SOURCE = {};

function renderWizard() {
render(
<ImportWizard objectName="account" fields={FIELDS} open onOpenChange={() => {}} />,
<ImportWizard
objectName="account"
fields={FIELDS}
dataSource={NO_CAPABILITY_DATA_SOURCE}
open
onOpenChange={() => {}}
/>,
);
}

Expand Down
33 changes: 25 additions & 8 deletions packages/plugin-grid/src/__tests__/useBulkExecutor.test.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -354,8 +354,27 @@ describe('useBulkExecutor', () => {
} as BulkActionDef);
const ds = () => ({ update: vi.fn(), delete: vi.fn() });

// A stub that DECLARES the parameters the hook really passes.
// `BulkExecutorOptions.runAggregate` is `(def, rows, params)` and the hook
// dispatches it with all three (`hooks/useBulkExecutor.ts`), but a bare
// `vi.fn(async () => undefined)` declares none of them: vitest records the
// real arguments at runtime, so `mock.calls[0][1]` works while the compiler
// is told the call tuple has length 0. The cases below read that second
// argument and papered over the contradiction with casts — a types-only lie
// about the exact signature they exist to pin (#4277). Typing it here once
// makes the reads compile on their own and keeps every aggregate case
// agreeing about the dispatcher's shape.
const aggregateStub = () =>
vi.fn(
async (
_def: BulkActionDef,
_rows: Array<Record<string, unknown>>,
_params: Record<string, unknown>,
): Promise<unknown> => undefined,
);

it('dispatches runAggregate exactly once with every row and the params', async () => {
const runAggregate = vi.fn(async () => undefined);
const runAggregate = aggregateStub();
const runAction = vi.fn(async () => undefined);
const rows = [{ id: 'r1' }, { id: 'r2' }, { id: 'r3' }];
const { result } = renderHook(() =>
Expand All@@ -366,9 +385,7 @@ describe('useBulkExecutor', () => {
});

expect(runAggregate).toHaveBeenCalledTimes(1);
const [defArg, rowsArg, paramsArg] = runAggregate.mock.calls[0] as unknown as [
BulkActionDef, Array<Record<string, unknown>>, Record<string, unknown>,
];
const [defArg, rowsArg, paramsArg] = runAggregate.mock.calls[0];
expect(defArg.name).toBe('generate_qr_zip');
expect(rowsArg.map(r => r.id)).toEqual(['r1', 'r2', 'r3']);
expect(paramsArg).toEqual({ format: 'png' });
Expand All@@ -378,7 +395,7 @@ describe('useBulkExecutor', () => {
});

it('a single-row selection still goes through the ONE aggregate call, never per-record', async () => {
const runAggregate = vi.fn(async () => undefined);
const runAggregate = aggregateStub();
const runAction = vi.fn(async () => undefined);
const { result } = renderHook(() =>
useBulkExecutor({ resource: 'device', dataSource: ds(), runAction, runAggregate }));
Expand All@@ -393,7 +410,7 @@ describe('useBulkExecutor', () => {
});

it('ignores batchSize — 5 rows with batchSize 2 is still one call', async () => {
const runAggregate = vi.fn(async () => undefined);
const runAggregate = aggregateStub();
const rows = [{ id: '1' }, { id: '2' }, { id: '3' }, { id: '4' }, { id: '5' }];
const { result } = renderHook(() =>
useBulkExecutor({ resource: 'device', dataSource: ds(), runAggregate }));
Expand All@@ -403,7 +420,7 @@ describe('useBulkExecutor', () => {
});

expect(runAggregate).toHaveBeenCalledTimes(1);
expect((runAggregate.mock.calls[0][1] as unknown[]).length).toBe(5);
expect(runAggregate.mock.calls[0][1].length).toBe(5);
expect(result.current.result?.succeeded).toBe(5);
});

Expand DownExpand Up@@ -462,7 +479,7 @@ describe('useBulkExecutor', () => {
});

it('a def WITHOUT execution: aggregate keeps per-record dispatch even when runAggregate is wired', async () => {
const runAggregate = vi.fn(async () => undefined);
const runAggregate = aggregateStub();
const runAction = vi.fn(async () => undefined);
const rows = [{ id: '1' }, { id: '2' }];
const { result } = renderHook(() =>
Expand Down
2 changes: 1 addition & 1 deletion packages/plugin-grid/src/index.tsx
Original file line numberDiff line numberDiff line change
Expand Up@@ -35,7 +35,7 @@ export { useGroupReorder } from './useGroupReorder';
export { useColumnSummary } from './useColumnSummary';
export { FormulaBar } from './FormulaBar';
export { SplitPaneGrid } from './SplitPaneGrid';
export type { ObjectGridProps } from './ObjectGrid';
export type { ObjectGridProps, ObjectGridExternalPaginationProps, ObjectGridColumnState } from './ObjectGrid';
export type { VirtualGridProps, VirtualGridColumn } from './VirtualGrid';
export type { InlineEditingProps } from './InlineEditing';
export type { ImportWizardProps, ImportResult } from './ImportWizard';
Expand Down
Loading
Loading