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
7 changes: 7 additions & 0 deletions .changeset/10689-grid-harvest-inputs-refetch.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
'@object-ui/plugin-grid': patch
---

`object-grid` re-reads its rows when an input its query reads changes: a `conditionalFormatting` rule, a row or bulk action def, or the view's `searchableFields` (objectui#10689).

The grid puts each predicate operand of its `conditionalFormatting` rules and its row and bulk action defs into `$select`, and sends `searchableFields` as `$searchFields` beside a search term, but its load effect did not re-run when any of them changed. So a rule added to a mounted grid never had its field fetched, and the rule never matched. The effect now keys on the operand names and on the `$searchFields` it sends, both by content, so an equal value in a new array, or a change the query cannot see such as a rule's colour, does not re-read.
7 changes: 7 additions & 0 deletions .changeset/10689-list-harvest-inputs-refetch.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
'@object-ui/plugin-list': patch
---

`list-view` re-reads its rows when a `conditionalFormatting` rule, a row action def or a bulk action def adds a field its predicates read (objectui#10689).

The list puts each predicate operand of those three into `$select`, but its fetch effect did not re-run when any of them changed. So a rule added to a mounted list never had its field fetched, and the rule never matched. The effect now keys on the operand names by content, so an equal value in a new array, or a change the query cannot see such as a rule's colour, does not re-read.
42 changes: 41 additions & 1 deletion packages/plugin-grid/src/ObjectGrid.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -1961,6 +1961,34 @@ export const ObjectGrid: React.FC<ObjectGridComponentProps> = ({
() => JSON.stringify(collectGroupingFieldRefs(schema.grouping)),
[schema.grouping],
);
// [objectui#10689] The fetch effect's dep on the three view-level PREDICATE
// carriers its projection harvests (objectui#3501), as a CONTENT key over the
// operand NAMES alone, the shape `groupingProjectionKey` takes above.
//
// The load effect reads `conditionalFormatting`, `rowActionDefs` and
// `bulkActionDefs` through `listViewPredicates` and puts each operand into
// `$select`, but none of the three was a dependency. So a rule added to a
// mounted grid never had its operand fetched: the rows kept arriving without
// the field, and the rule, which reads it, never matched.
//
// Names ONLY: a rule's style, or an action's label, is a render-time concern
// the projection cannot see, so changing it costs no round trip. The same
// harvest over the same three inputs as the effect's; the object-level
// `actions` / `userActions` it adds come from the definition the effect reads
// for itself. The `rowActionDefs` cast is the objectui#5091 NON-AUTHOR
// SURFACE exemption stated at the effect's own read of that key.
// `__tests__/ObjectGrid.harvestInputsFetchKey-10689.test.tsx` pins each input.
const conditionalFormattingRaw = schema.conditionalFormatting as readonly unknown[] | undefined;
const rowActionDefsRaw = (schema as { rowActionDefs?: readonly unknown[] }).rowActionDefs;
const bulkActionDefsRaw = (schema as { bulkActionDefs?: readonly unknown[] }).bulkActionDefs;
const predicateProjectionKey = useMemo(
() => JSON.stringify(collectPredicateFieldRefs(listViewPredicates({
conditionalFormatting: conditionalFormattingRaw,
rowActionDefs: rowActionDefsRaw,
bulkActionDefs: bulkActionDefsRaw,
}))),
[conditionalFormattingRaw, rowActionDefsRaw, bulkActionDefsRaw],
);
// The view's declared filter, lowered ONCE through the repo's single filter
// sink for both consumers below (the fetch and the server-side export).
//
Expand Down Expand Up @@ -2050,6 +2078,15 @@ export const ObjectGrid: React.FC<ObjectGridComponentProps> = ({
// is the one typed into this grid's box.
const hostOwnsSearch = externalManualPagination || hostSearch !== undefined;
const querySearchTerm = hostOwnsSearch ? (hostSearch ?? '') : searchTerm;
// [objectui#10689] The `$searchFields` the load effect sends, as a CONTENT
// key for its dependency list: the view's `searchableFields`, and only while
// a term goes out, since without one the query does not send them. A view
// that narrows its search to other fields re-reads once; an equal array in a
// new object does not.
const searchFieldsKey =
querySearchTerm.trim() && schema.searchableFields && schema.searchableFields.length > 0
? JSON.stringify(schema.searchableFields)
: '';

// --- Inline data effect (synchronous, no fetch needed) ---
useEffect(() => {
Expand Down Expand Up @@ -2819,7 +2856,10 @@ export const ObjectGrid: React.FC<ObjectGridComponentProps> = ({
// `groupingNeedsHeaderQuery` (objectui#10881): it flips when the object's
// types land and show the only grouping key masked, and the flat fetch it
// withheld must then go out.
}, [objectName, schemaFields, schemaColumns, schemaFilter, schemaFilterRefusal, schemaDefaultFilters, schemaSort, headerSort, querySearchTerm, schemaPagination, schemaPageSize, serverPage, fetchWindow, dataSource, hasInlineData, dataConfig, refreshKey, perms.isLoaded, groupingProjectionKey, invalidationNonce, serverGroupedFetch, groupingNeedsHeaderQuery]);
// `predicateProjectionKey` and `searchFieldsKey` (objectui#10689): the
// harvested predicate operands and the `$searchFields` this query sends, each
// a string compared by value — see their declarations above.
}, [objectName, schemaFields, schemaColumns, schemaFilter, schemaFilterRefusal, schemaDefaultFilters, schemaSort, headerSort, querySearchTerm, schemaPagination, schemaPageSize, serverPage, fetchWindow, dataSource, hasInlineData, dataConfig, refreshKey, perms.isLoaded, groupingProjectionKey, predicateProjectionKey, searchFieldsKey, invalidationNonce, serverGroupedFetch, groupingNeedsHeaderQuery]);

// The same reset, for the path the loader above never runs on (objectui#4501
// clause 2). "All N matching are selected" is a claim about ONE query, so it
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,209 @@
/**
* ObjectUI
* Copyright (c) 2024-present ObjectStack Inc.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*/

/**
* objectui#10689 — `ObjectGrid`'s load effect re-reads when an input its query
* reads changes, and only then. The objectui#10664 family rule: every input the
* query reads is in the effect's dependencies, keyed by content.
*
* The query reads four schema inputs that the dependency list did not name:
*
* - `conditionalFormatting`, `rowActionDefs` and `bulkActionDefs`, through the
* predicate-operand harvest (objectui#3501) that puts each predicate's
* operands into `$select`;
* - `searchableFields`, sent as `$searchFields` beside a search term.
*
* So a rule added to a mounted grid never had its operand fetched: the rows
* kept arriving without the field, and the rule, which reads it, never matched.
*
* Each input is keyed by the part of it the query reads, as a string compared
* by value (AGENTS.md #10): the harvested operand NAMES for the three predicate
* carriers (the shape `groupingProjectionKey` already takes for `grouping`), and
* the `$searchFields` the query sends. So an equal re-render, or a change the
* query cannot see (a rule's colour), costs no round trip.
*/
import { describe, it, expect, vi, beforeAll, afterEach } from 'vitest';
import { render, waitFor, act, cleanup } from '@testing-library/react';
import '@testing-library/jest-dom';
import React from 'react';

import { ObjectGrid } from '../ObjectGrid';
import { registerAllFields } from '@object-ui/fields';

registerAllFields();

beforeAll(() => {
if (!Element.prototype.scrollIntoView) {
Element.prototype.scrollIntoView = vi.fn() as any;
}
});

afterEach(() => cleanup());

const OBJECT = 'account';

// Held as module constants so every re-render below hands the grid the SAME
// references for the inputs this card does not touch. `columns` and
// `pagination` are named by identity in the effect's dependency list, so a
// fresh literal of either would re-read for a reason of its own.
const COLUMNS = [{ field: 'name', label: 'Name' }];
const PAGINATION = { pageSize: 50 };

function makeDataSource() {
const find = vi.fn(async () => ({
data: [{ id: 'r1', name: 'Acme', industry: 'Tech', owner: 'u1', status: 'open', region: 'EU' }],
total: 1,
hasMore: false,
pageSize: 50,
}));
return {
find,
getObjectSchema: async (name: string) => ({
name,
fields: {
id: { type: 'text' },
name: { type: 'text' },
industry: { type: 'text' },
owner: { type: 'text' },
status: { type: 'text' },
region: { type: 'text' },
},
}),
} as any;
}

const gridSchema = (extra: Record<string, unknown> = {}): any => ({
type: 'object-grid',
objectName: OBJECT,
columns: COLUMNS,
pagination: PAGINATION,
...extra,
});

/** Long enough for any effect a step schedules to have run and settled. */
const settle = () => act(() => new Promise<void>((resolve) => setTimeout(resolve, 150)));

const selects = (find: ReturnType<typeof vi.fn>) =>
find.mock.calls.map((call: any[]) => call[1]?.$select as string[] | undefined);

/** Mount, wait for the first read to land, and settle every follow-up effect. */
async function mount(ds: any, extra: Record<string, unknown> = {}, props: Record<string, unknown> = {}) {
const view = render(<ObjectGrid schema={gridSchema(extra)} dataSource={ds} {...props} />);
await waitFor(() => expect(ds.find).toHaveBeenCalled());
await settle();
const rerender = async (next: Record<string, unknown>, nextProps: Record<string, unknown> = props) => {
view.rerender(<ObjectGrid schema={gridSchema(next)} dataSource={ds} {...nextProps} />);
await settle();
};
return { rerender };
}

const INDUSTRY_RULE = { condition: "record.industry == 'Tech'", style: { backgroundColor: '#fee2e2' } };
const ROW_DEF = { name: 'escalate', label: 'Escalate', visible: "record.owner == 'u1'" };
const BULK_DEF = { name: 'archive', label: 'Archive', disabled: "record.status == 'closed'" };

describe('ObjectGrid re-reads when a harvested predicate input changes (objectui#10689)', () => {
it('CONTROL — a fresh mount with the rule selects its operand, in one read', async () => {
const ds = makeDataSource();
await mount(ds, { conditionalFormatting: [INDUSTRY_RULE] });
expect(selects(ds.find)).toEqual([['id', 'name', 'industry']]);
});

it('a `conditionalFormatting` rule added to a mounted grid re-reads once, with its operand in `$select`', async () => {
const ds = makeDataSource();
const { rerender } = await mount(ds);
expect(selects(ds.find)).toEqual([['id', 'name']]);

await rerender({ conditionalFormatting: [INDUSTRY_RULE] });

expect(
selects(ds.find),
'The rule was added to a mounted grid and the grid never re-read, so the rows keep\n'
+ 'arriving without `industry` and the rule never matches. The load effect must name\n'
+ 'the harvested operands in its dependencies (objectui#10689).',
).toEqual([['id', 'name'], ['id', 'name', 'industry']]);
});

it('a `rowActionDefs` def added to a mounted grid re-reads once, with its `visible` operand', async () => {
const ds = makeDataSource();
const { rerender } = await mount(ds);

await rerender({ rowActionDefs: [ROW_DEF] });

expect(selects(ds.find)).toEqual([['id', 'name'], ['id', 'name', 'owner']]);
});

it('a `bulkActionDefs` def added to a mounted grid re-reads once, with its `disabled` operand', async () => {
const ds = makeDataSource();
const { rerender } = await mount(ds);

await rerender({ bulkActionDefs: [BULK_DEF] });

expect(selects(ds.find)).toEqual([['id', 'name'], ['id', 'name', 'status']]);
});

it('CONTROL — an equal re-render (new arrays, same content) does not re-read', async () => {
const ds = makeDataSource();
const { rerender } = await mount(ds, {
conditionalFormatting: [INDUSTRY_RULE],
rowActionDefs: [ROW_DEF],
bulkActionDefs: [BULK_DEF],
searchableFields: ['name', 'industry'],
}, { search: 'acme' });
expect(ds.find).toHaveBeenCalledTimes(1);

await rerender({
conditionalFormatting: [{ ...INDUSTRY_RULE, style: { ...INDUSTRY_RULE.style } }],
rowActionDefs: [{ ...ROW_DEF }],
bulkActionDefs: [{ ...BULK_DEF }],
searchableFields: ['name', 'industry'],
}, { search: 'acme' });

expect(ds.find, 'an equal re-render must cost no round trip').toHaveBeenCalledTimes(1);
});

it('a rule change the query cannot see (its style) does not re-read', async () => {
const ds = makeDataSource();
const { rerender } = await mount(ds, { conditionalFormatting: [INDUSTRY_RULE] });

await rerender({
conditionalFormatting: [{ ...INDUSTRY_RULE, style: { backgroundColor: '#dcfce7' } }],
});

expect(ds.find).toHaveBeenCalledTimes(1);
});
});

describe('ObjectGrid re-reads when `searchableFields` changes under a search term (objectui#10689)', () => {
const searchFieldsOf = (find: ReturnType<typeof vi.fn>) =>
find.mock.calls.map((call: any[]) => call[1]?.$searchFields as string[] | undefined);

it('`searchableFields` changed on a mounted grid re-reads once, with the new `$searchFields`', async () => {
const ds = makeDataSource();
const { rerender } = await mount(ds, { searchableFields: ['name'] }, { search: 'acme' });
expect(searchFieldsOf(ds.find)).toEqual([['name']]);

await rerender({ searchableFields: ['name', 'region'] }, { search: 'acme' });

expect(
searchFieldsOf(ds.find),
'The view narrowed its search to new fields and the grid never re-read, so the rows on\n'
+ 'screen still answer the old `$searchFields` (objectui#10689).',
).toEqual([['name'], ['name', 'region']]);
});

it('without a search term `$searchFields` is not sent, so a change to it does not re-read', async () => {
const ds = makeDataSource();
const { rerender } = await mount(ds, { searchableFields: ['name'] });
expect(searchFieldsOf(ds.find)).toEqual([undefined]);

await rerender({ searchableFields: ['name', 'region'] });

expect(ds.find).toHaveBeenCalledTimes(1);
});
});
36 changes: 35 additions & 1 deletion packages/plugin-list/src/ListView.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2254,6 +2254,37 @@ export const ListView = React.forwardRef<ListViewHandle, ListViewProps>(({
!Array.isArray(schema.data) && (schema.data as any)?.provider !== 'value' && !ganttOwnsData;
const invalidationNonce = useDataInvalidation(listFetchesForItself ? schema.objectName || undefined : undefined);

// objectui#10689 — the fetch effect's dep on the three view-level PREDICATE
// carriers its projection harvests (objectui#3501), as a CONTENT key over the
// operand NAMES alone: the shape `plugin-grid`'s `predicateProjectionKey`
// takes for the grid's own load effect.
//
// The effect below reads `conditionalFormatting`, `rowActionDefs` and
// `bulkActionDefs` through `listViewPredicates` and adds each operand to
// `$select`, but none of the three was a dependency. So a rule added to a
// mounted list never had its operand fetched: the rows kept arriving without
// the field, and the rule, which reads it, never matched.
//
// Names ONLY: a rule's style, or an action's label, is a render-time concern
// the projection cannot see, so changing it costs no round trip. The same
// harvest over the same three inputs as the effect's; the object-level
// `actions` / `userActions` it adds come from `objectDef`, not from the view,
// and are outside this key. The `rowActionDefs` cast is
// the NON-AUTHOR SURFACE exemption stated at the effect's own read of that
// key. `__tests__/ListView.harvestInputsFetchKey-10689.test.tsx` pins each
// input.
const conditionalFormattingRaw = schema.conditionalFormatting as readonly unknown[] | undefined;
const rowActionDefsRaw = (schema as { rowActionDefs?: readonly unknown[] }).rowActionDefs;
const bulkActionDefsRaw = (schema as { bulkActionDefs?: readonly unknown[] }).bulkActionDefs;
const predicateProjectionKey = React.useMemo(
() => JSON.stringify(collectPredicateFieldRefs(listViewPredicates({
conditionalFormatting: conditionalFormattingRaw,
rowActionDefs: rowActionDefsRaw,
bulkActionDefs: bulkActionDefsRaw,
}))),
[conditionalFormattingRaw, rowActionDefsRaw, bulkActionDefsRaw],
);

// Fetch data effect — supports schema.data (ViewDataSchema) provider modes
React.useEffect(() => {
let isMounted = true;
Expand Down Expand Up @@ -2843,13 +2874,16 @@ export const ListView = React.forwardRef<ListViewHandle, ListViewProps>(({
// down. It reads `currentView`, so leaving a refused grouped grid for a
// board re-runs this effect once, and that run is the board's fetch.
//
// objectui#10689 — `predicateProjectionKey` is the harvested predicate
// operands, a string compared by value; see its declaration above.
//
// ⚠️ The directive below governs the NEXT LINE. Anything written between it
// and the dependency array detaches it from the array and turns it into an
// unused directive — which `eslint .` reports as an ERROR, and which also
// silently un-suppresses nothing, because the finding it was suppressing
// simply moves elsewhere. Add prose ABOVE this point, never below it.
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [schema.objectName, schema.data, dataSource, authoredFilter, effectivePageSize, currentSort, appliedFilters, appliedUserFilterConditions, refreshKey, searchTerm, schema.searchableFields, schema.columns, (schema as any).kanban, (schema as any).calendar, (schema as any).gallery, (schema as any).timeline, (schema as any).gantt, schema.map, (schema as any).options, objectDef?.fields, objectDefLoaded, schema.refreshTrigger, perms, fetchSkip, groupingConfig, ganttOwnsData, invalidationNonce, groupingNeedsHeaderQuery]); // Re-fetch on filter/sort/search/refreshTrigger/perms/window change
}, [schema.objectName, schema.data, dataSource, authoredFilter, effectivePageSize, currentSort, appliedFilters, appliedUserFilterConditions, refreshKey, searchTerm, schema.searchableFields, schema.columns, (schema as any).kanban, (schema as any).calendar, (schema as any).gallery, (schema as any).timeline, (schema as any).gantt, schema.map, (schema as any).options, objectDef?.fields, objectDefLoaded, schema.refreshTrigger, perms, fetchSkip, groupingConfig, predicateProjectionKey, ganttOwnsData, invalidationNonce, groupingNeedsHeaderQuery]); // Re-fetch on filter/sort/search/refreshTrigger/perms/window change

// Any change to the result-defining inputs (object, filters, sort, search,
// grouping, page size) invalidates the current page number — snap back to
Expand Down
Loading
Loading