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
8 changes: 8 additions & 0 deletions .changeset/10664-data-table-filter-bar.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
---
'@object-ui/plugin-dashboard': patch
---

The dashboard's `object-data-table` reads its rows once per mount, already expanded, and a dashboard filter re-reads its options when `optionsFrom.filter` changes (objectui#10664).

- `object-data-table` loaded the object definition in an effect of its own and listed it among the fetch effect's dependencies, so every object-bound mount issued two `find` calls, the first without `$expand`. It now reads the definition through `useSettledSchema` from `@object-ui/react` and its query waits until that read settles; a failed read, or an adapter with no `getObjectSchema`, still loads the rows, unexpanded. Its expansion also depends on `columns`, which did not re-run the read: adding a lookup column to a mounted table now re-reads with the new `$expand`, and relabelling a column does not.
- The filter bar's option list sends `optionsFrom.filter` on both of its reads, the dataset query and the record fallback, but did not re-read when only that filter changed. It now does, and an equal filter in a new object does not re-read.
7 changes: 7 additions & 0 deletions .changeset/10664-map-reads-once.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
'@object-ui/plugin-map': patch
---

An `object-map` block reads its records once per mount, and that read already carries the lookup expansion (objectui#10664).

The map loaded the object definition in an effect of its own and listed it among the fetch effect's dependencies. The definition lands after the first query, so every mount issued two `find` calls, the first without `$expand`. Switching the bound object also sent the new object's query with the previous object's expansion. The map now reads the definition through `useSettledSchema` from `@object-ui/react`, and its object query waits until that read settles, the way `object-timeline` (objectui#7895) and `object-gallery` (objectui#7903) already do. A definition read that fails, or an adapter with no `getObjectSchema`, settles with no definition, so the map still loads, unexpanded. Host rows (`data`) and an inline `value` set are not held.
7 changes: 7 additions & 0 deletions .changeset/10664-object-view-table-sort.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
'@object-ui/plugin-view': patch
---

The object view's own read for a non-grid view (calendar, kanban and the other views it fetches for) re-reads when `table.sort` changes (objectui#10664).

That read falls back to `table.sort` for its `$orderby` when neither the named view nor the active view declares a sort, but a change to `table.sort` alone did not re-run it, so the view kept the old order. It now keys the read on that sort's content, so an equal sort in a new array does not re-read.
7 changes: 7 additions & 0 deletions .changeset/10664-repeater-picker-sort-key.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
'@object-ui/components': patch
---

`element:repeater` and `element:record_picker` re-read their rows when the sort they send changes (objectui#10664, folding objectui#10665).

Both blocks put their sort on `$orderby` (the repeater's `properties.sort`; the picker's `properties.sort` or its `dataSource` binding's `sort`), but neither re-ran its read when only the sort changed, so a bound sort control or a live preview kept showing the old order. Each now keys its read on the sort's content, the way it already keys on the filter's, so an equal sort in a new array does not re-read.
Original file line number Diff line number Diff line change
@@ -0,0 +1,93 @@
/**
* 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#10664 (folded from objectui#10665) — `element:repeater` re-reads
* when its `sort` changes, and only then.
*
* The fetch effect lowers `properties.sort` onto `$orderby`, but its dependency
* list named `filterKey` and no sort at all. A mounted repeater whose sort
* changed (a bound sort control, a live designer preview) kept the old order
* until the object, the filter, the limit or a data-invalidation event re-ran
* the read. The sort is now keyed the way `filterKey` keys the filter: by
* CONTENT, so a new array with the same entries is not a change.
*
* Both halves are pinned. The CONTROL (an equal sort in a fresh array) is green
* before the fix and after it; it goes red for a fix that keys on the array's
* identity instead of its content (AGENTS.md #10: a key is a primitive or a
* structurally stable value).
*/

import { describe, it, expect, vi, afterEach } from 'vitest';
import * as React from 'react';
import { render, screen, waitFor, act, cleanup } from '@testing-library/react';
import { AdapterCtx, SchemaRenderer } from '@object-ui/react';
// Registers every `element:*` renderer at module scope, not in a hook
// (object-ui/no-dynamic-import-in-test-hook, objectui#3010).
import '../../../renderers';

afterEach(cleanup);

const settle = () => act(() => new Promise<void>((resolve) => setTimeout(resolve, 50)));

const BY_NAME_ASC = [{ field: 'name', order: 'asc' }];
const BY_NAME_DESC = [{ field: 'name', order: 'desc' }];

type HostHandle = { setSort: (sort: unknown) => void };

/** Holds the repeater's `sort` as state and rebuilds the node on every change. */
const Host = React.forwardRef<HostHandle, { adapter: any }>(function Host({ adapter }, ref) {
const [sort, setSort] = React.useState<unknown>(BY_NAME_ASC);
React.useImperativeHandle(ref, () => ({ setSort }), []);
const schema = React.useMemo(
() => ({ type: 'element:repeater', id: 'rep', properties: { object: 'contact', fields: ['name'], sort } }),
[sort],
);
return (
<AdapterCtx.Provider value={adapter as never}>
<SchemaRenderer schema={schema as never} />
</AdapterCtx.Provider>
);
});

function mount() {
const adapter = { find: vi.fn(async () => ({ data: [{ id: 'r1', name: 'Ada' }], total: 1 })) };
const host = React.createRef<HostHandle>();
render(<Host ref={host} adapter={adapter} />);
return { adapter, host };
}

/** The `$orderby` of every read the repeater issued, in order. */
const orderbys = (adapter: { find: { mock: { calls: any[][] } } }) =>
adapter.find.mock.calls.map((c) => c[1]?.$orderby);

describe('element:repeater keys its read on the sort it sends (objectui#10664)', () => {
it('SUBJECT: a changed sort re-reads, with the new `$orderby`', async () => {
const { adapter, host } = mount();
await waitFor(() => expect(screen.getByTestId('repeater')).toBeInTheDocument());
await settle();
expect(orderbys(adapter)).toEqual([BY_NAME_ASC]);

await act(async () => { host.current!.setSort(BY_NAME_DESC); });
await settle();

expect(orderbys(adapter), 'the changed sort never reached a read').toEqual([BY_NAME_ASC, BY_NAME_DESC]);
});

it('CONTROL: an equal sort in a fresh array does not re-read', async () => {
const { adapter, host } = mount();
await waitFor(() => expect(screen.getByTestId('repeater')).toBeInTheDocument());
await settle();
const atRest = adapter.find.mock.calls.length;

await act(async () => { host.current!.setSort([{ field: 'name', order: 'asc' }]); });
await settle();

expect(adapter.find.mock.calls.length, 'an equal sort re-read the list').toBe(atRest);
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,101 @@
/**
* 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#10664 (the census row for `element:record_picker`) — the picker
* re-reads its options when the `sort` it sends changes, and only then.
*
* Same defect as `element:repeater` in the sibling file: the fetch effect puts
* `sort` on `$orderby` (the flat `properties.sort`, or a `dataSource` binding's
* `sort`, which replaces it), but its dependency list named `filterKey` and no
* sort. A changed sort kept the old option order until something else re-ran
* the read. The sort is now keyed by CONTENT, the way `filterKey` keys the
* filter.
*
* The CONTROL (an equal sort in a fresh array) is green before and after; it
* goes red for a fix that keys on the array's identity (AGENTS.md #10).
*/

import { describe, it, expect, vi, afterEach } from 'vitest';
import * as React from 'react';
import { render, act, cleanup, waitFor } from '@testing-library/react';
import { AdapterCtx, SchemaRenderer } from '@object-ui/react';
// Registers `element:record_picker` at module scope, not in a hook
// (object-ui/no-dynamic-import-in-test-hook, objectui#3010).
import '../../../renderers';

afterEach(cleanup);

const settle = () => act(() => new Promise<void>((resolve) => setTimeout(resolve, 50)));

const BY_NAME_ASC = [{ field: 'name', order: 'asc' }];
const BY_NAME_DESC = [{ field: 'name', order: 'desc' }];

type Where = 'properties' | 'binding';
type HostHandle = { setSort: (sort: unknown) => void };

/**
* Holds the picker's `sort` as state and rebuilds the node on every change.
* `where` puts it on the flat `properties.sort` or on a `dataSource` binding
* (no `view`, so nothing waits on a saved-view read).
*/
const Host = React.forwardRef<HostHandle, { adapter: any; where: Where }>(function Host({ adapter, where }, ref) {
const [sort, setSort] = React.useState<unknown>(BY_NAME_ASC);
React.useImperativeHandle(ref, () => ({ setSort }), []);
const schema = React.useMemo(
() =>
where === 'properties'
? { type: 'element:record_picker', id: 'picker', properties: { object: 'account', sort } }
: { type: 'element:record_picker', id: 'picker', properties: {}, dataSource: { object: 'account', sort } },
[sort, where],
);
return (
<AdapterCtx.Provider value={adapter as never}>
<SchemaRenderer schema={schema as never} />
</AdapterCtx.Provider>
);
});

function mount(where: Where) {
const adapter = { find: vi.fn(async () => ({ data: [{ id: 'a1', name: 'Acme' }], total: 1 })), getObjectSchema: vi.fn() };
const host = React.createRef<HostHandle>();
render(<Host ref={host} adapter={adapter} where={where} />);
return { adapter, host };
}

/** The `$orderby` of every read the picker issued, in order. */
const orderbys = (adapter: { find: { mock: { calls: any[][] } } }) =>
adapter.find.mock.calls.map((c) => c[1]?.$orderby);

describe('element:record_picker keys its read on the sort it sends (objectui#10664)', () => {
for (const where of ['properties', 'binding'] as const) {
it(`SUBJECT: a changed sort on the ${where} re-reads, with the new \`$orderby\``, async () => {
const { adapter, host } = mount(where);
await waitFor(() => expect(adapter.find).toHaveBeenCalled());
await settle();
expect(orderbys(adapter)).toEqual([BY_NAME_ASC]);

await act(async () => { host.current!.setSort(BY_NAME_DESC); });
await settle();

expect(orderbys(adapter), 'the changed sort never reached a read').toEqual([BY_NAME_ASC, BY_NAME_DESC]);
});
}

it('CONTROL: an equal sort in a fresh array does not re-read', async () => {
const { adapter, host } = mount('properties');
await waitFor(() => expect(adapter.find).toHaveBeenCalled());
await settle();
const atRest = adapter.find.mock.calls.length;

await act(async () => { host.current!.setSort([{ field: 'name', order: 'asc' }]); });
await settle();

expect(adapter.find.mock.calls.length, 'an equal sort re-read the options').toBe(atRest);
});
});
6 changes: 5 additions & 1 deletion packages/components/src/renderers/basic/data-list.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,10 @@ function RepeaterRenderer({ schema }: { schema: any }) {
const [loading, setLoading] = React.useState(true);
const [error, setError] = React.useState<string | null>(null);
const filterKey = React.useMemo(() => (props.filter ? JSON.stringify(props.filter) : ''), [props.filter]);
// objectui#10664 — the sort reaches `$orderby` below, so the fetch effect
// keys on it, by CONTENT the way `filterKey` keys the filter: a fresh array
// with the same entries is not a change (AGENTS.md #10).
const sortKey = React.useMemo(() => (props.sort ? JSON.stringify(props.sort) : ''), [props.sort]);

const cols: RepeaterColumn[] = React.useMemo(
() => (props.fields ?? []).map((f) => (typeof f === 'string' ? { field: f } : f)),
Expand Down Expand Up @@ -161,7 +165,7 @@ function RepeaterRenderer({ schema }: { schema: any }) {
cancelled = true;
};
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [adapter, props.object, filterKey, props.limit, invalidationNonce]);
}, [adapter, props.object, filterKey, sortKey, props.limit, invalidationNonce]);

if (loading) return <p className="py-2 text-sm text-muted-foreground">Loading…</p>;
if (error) return <p className="py-2 text-sm text-destructive">{error}</p>;
Expand Down
6 changes: 5 additions & 1 deletion packages/components/src/renderers/basic/record-picker.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,10 @@ function ElementRecordPickerRenderer({ schema }: { schema: any }) {
const [loading, setLoading] = React.useState(true);
const [error, setError] = React.useState<string | null>(null);
const filterKey = React.useMemo(() => (filter ? JSON.stringify(filter) : ''), [filter]);
// objectui#10664 — the sort reaches `$orderby` below, so the fetch effect
// keys on it, by CONTENT the way `filterKey` keys the filter: a fresh array
// with the same entries is not a change (AGENTS.md #10).
const sortKey = React.useMemo(() => (sort ? JSON.stringify(sort) : ''), [sort]);

React.useEffect(() => {
let cancelled = false;
Expand Down Expand Up @@ -156,7 +160,7 @@ function ElementRecordPickerRenderer({ schema }: { schema: any }) {
cancelled = true;
};
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [adapter, object, filterKey, limit]);
}, [adapter, object, filterKey, sortKey, limit]);

// Reflect the bound variable's value back into the control. When a variable
// targets this picker we stay controlled for its whole lifetime (empty string
Expand Down
6 changes: 5 additions & 1 deletion packages/plugin-dashboard/src/DashboardFilterBar.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -274,6 +274,10 @@ function SelectFilter({ def, value, onChange, dataSource }: { def: DashboardFilt
// total failure (same tolerance style as DatasetWidget's option-color
// fetch).
const from = def.optionsFrom;
// objectui#10664 — both reads below send `from.filter`, so the effect keys on
// it, by CONTENT: an equal filter in a fresh object is not a change
// (AGENTS.md #10).
const optionsFilterKey = JSON.stringify(from?.filter ?? null);
useEffect(() => {
if (!from || !dataSource) return;
let cancelled = false;
Expand Down Expand Up @@ -350,7 +354,7 @@ function SelectFilter({ def, value, onChange, dataSource }: { def: DashboardFilt
}
return () => { cancelled = true; };
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [from?.object, from?.valueField, from?.labelField, dataSource]);
}, [from?.object, from?.valueField, from?.labelField, optionsFilterKey, dataSource]);

const localizedOptions = useMemo(() => {
// `def.options` is already normalized to `{ value, label }` PAIRS by
Expand Down
65 changes: 49 additions & 16 deletions packages/plugin-dashboard/src/ObjectDataTable.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
*/

import React, { useState, useEffect, useContext, useMemo, useCallback } from 'react';
import { useDataScope, SchemaRendererContext, SchemaRenderer, useFilterScope } from '@object-ui/react';
import { useDataScope, SchemaRendererContext, SchemaRenderer, useFilterScope, useSettledSchema } from '@object-ui/react';
import {
extractRecords,
isDrillEnabled,
Expand Down Expand Up @@ -625,7 +625,6 @@ export const ObjectDataTable: React.FC<ObjectDataTableProps> = ({ schema, dataSo
if (b && b !== 'dashboard.noDataSourceFor') noDataSourceLabel = b;

const [fetchedData, setFetchedData] = useState<any[]>([]);
const [objectSchema, setObjectSchema] = useState<any>(null);
// Start in loading state when we will fetch from a dataSource, so the
// "No data available" empty state doesn't flash on slow networks before
// the fetch effect runs and flips loading to true.
Expand Down Expand Up @@ -657,6 +656,37 @@ export const ObjectDataTable: React.FC<ObjectDataTableProps> = ({ schema, dataSo
// exists by the time render reaches the fetch effect below (objectui#7230).
const perms = usePermissions();

/**
* The object definition, and whether the read for THIS object has SETTLED:
* one piece of state, through the shared hook (objectui#10664).
*
* It sat in a local `useState` fed by its own effect and was listed in the
* fetch effect's dependencies below, so every object-bound mount queried
* twice: once before the definition landed (`computeLookupExpand` returns
* nothing without a field map, so no `$expand`), once after. The same shape
* `ObjectGallery` (objectui#7903) left, and the gate below is theirs.
*
* `dataSource` is passed on every path, as `ObjectGallery` passes it: the
* column headers and select-option cells read the definition on bound and
* inline rows too, where no query waits on it. The key is `schema.objectName`,
* the object the query names.
*
* ⚠️ The gate is only safe because the hook SETTLES ON EVERY EXIT
* (objectui#7232): no source, no `getObjectSchema`, no name, and a read that
* threw. The replaced effect returned without settling on the first three.
*/
const { ready: objectSchemaReady, def: objectSchema } = useSettledSchema<any>(
schema.objectName ?? '',
dataSource,
);

// The expansion the query below sends, as a CONTENT key (objectui#10664). The
// query reads `schema.columns` (an explicit whitelist decides which relations
// expand) and the dependency list did not name it, so a column added on a
// mounted widget never reached `$expand`. Keyed on the expansion rather than
// on the columns, so a relabelled column is not a change.
const lookupExpandKey = computeLookupExpand(schema, objectSchema).join(',');

useEffect(() => {
let isMounted = true;

Expand Down Expand Up @@ -733,27 +763,30 @@ export const ObjectDataTable: React.FC<ObjectDataTableProps> = ({ schema, dataSo
};

if (schema.objectName && !boundData && (!schema.data || schema.data.length === 0)) {
fetchData();
// ⭐ objectui#10664: the object definition GATES this query; it does not
// refine it afterwards. `objectSchema` stays in the dependency list and
// the two are one mechanism: the dependency re-runs this effect when the
// definition lands, and this branch stops the first run from spending a
// query before it has. Removing either half restores the double read.
//
// Scoped to the branch that queries. The placeholder is held across the
// window: `loading` starts `true` here only on first mount, and a later
// object switch closes the gate with the previous object's rows in state.
if (!objectSchemaReady) {
if (isMounted) setLoading(true);
} else {
fetchData();
}
} else if (isMounted) {
// We have inline / bound data and won't fetch — make sure loading is
// cleared (matters when we lazily-initialized it to true).
setLoading(false);
}

return () => { isMounted = false; };
}, [schema.objectName, dataSource, boundData, schema.data, schema.filter, objectSchema, filterScope, perms]);

// Fetch object schema for column-header translation and select-option cell labels.
useEffect(() => {
let isMounted = true;
if (!dataSource || !schema.objectName || typeof dataSource.getObjectSchema !== 'function') {
return;
}
dataSource.getObjectSchema(schema.objectName)
.then((s: any) => { if (isMounted) setObjectSchema(s); })
.catch(() => { /* schema lookup failure is non-fatal */ });
return () => { isMounted = false; };
}, [schema.objectName, dataSource]);
// `schema.columns` is read through `lookupExpandKey`, by content; see above.
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [schema.objectName, dataSource, boundData, schema.data, schema.filter, objectSchemaReady, objectSchema, lookupExpandKey, filterScope, perms]);

// Resolve data: bound data > static schema data > fetched data
const rawData = boundData || schema.data || fetchedData;
Expand Down
Loading
Loading