fix(plugin-list): ObjectGallery waits for the object definition instead of querying twice (objectui#7903)

.changeset/7903-gallery-settled-schema-gate.md

@@ -0,0 +1,56 @@
+---
+'@object-ui/plugin-list': patch
+---
+
+`ObjectGallery` waits for the object definition instead of querying twice
+(objectui#7903).
+
+It sat outside the set objectui#6482 converged on the shared settled-schema gate
+— `ObjectKanban`, `ObjectView`, `ObjectCalendar` and `ObjectTree` were named
+there, `ObjectGantt` was ask 2 of objectui#7225 and `ObjectTimeline` was
+objectui#7895 — and nothing marked it a deliberate exclusion. It still held the
+object definition in a local `useState` fed by its own metadata effect, and
+listed that definition in the record-fetch effect's dependency array.
+
+**User-visible.** Every object-bound gallery load issued **two** `find` calls
+instead of one: the first before the definition landed, with `buildExpandFields`
+seeing no fields and therefore carrying no `$expand` at all, and a second one
+after. The first paint was therefore a grid of cards rendered from raw
+foreign-key ids. After this change the gallery paints once, from a query that
+already carries its expansion.
+
+Measured on the component with an instrumented renderer, one mount per hold,
+`getObjectSchema` held 0/1/2/3/4/5/6/7/8/9/10/15/25/50/100 ms, with
+`ObjectCalendar` as a positive control in the same run: before, 2 `find` calls
+with expand sets `[null, ['owner']]` at every hold, the issue order always
+`schema:issued, find(unexpanded), schema:settled, find(expanded)`, two distinct
+painted states, 3 late writes into the card grid after the first paint, and a
+first-paint time flat at 3-7 ms across the whole sweep; after, 1 `find` carrying
+`['owner']`, one painted state, 0 late writes, and a first paint that tracks the
+hold (9 ms at +3, 15 ms at +10, 35 ms at +25, 106 ms at +100). The control read
+1 `find` carrying `['owner']` and a hold-tracking first paint both before and
+after.
+
+The cost this component was paying is a **two**-step paint, not the three-step
+one `ObjectCalendar` and `ObjectTimeline` each measured: those make `loading` an
+unconditional early return, so their re-run drops back to a placeholder in
+between, while this component's early return is `loading && !items.length` — the
+raw ids were replaced in place. Measured here rather than inherited from the
+matching shape, which is objectui#6482's own per-component standard.
+
+The resolution half is now `useSettledSchema` from `@object-ui/react`, which
+settles on **every** exit — no source, no `getObjectSchema`, no object name, and
+a read that threw alike. That is what makes the gate safe: the replaced effect
+returned without settling on all four, which cost nothing while nothing waited on
+it and would have held a gated query open forever. Pinned by
+`ObjectGallery.fetchGate-7903.test.tsx`, including a gallery whose adapter
+exposes no `getObjectSchema` and one whose definition read rejects — both still
+query, unexpanded.
+
+Two departures, each judged for this component rather than copied. Like
+`ObjectTimeline` and unlike `ObjectCalendar` / `ObjectGantt`, the metadata read
+is **not** disabled for a gallery whose records were authored inline: this
+component reads the definition on every path, for cell semantics and for ADR-0079
+card titles, not only to expand a query. And the gate window now holds the
+loading placeholder rather than showing "No items to display", which the two
+siblings get from their initial `loading` state and this one did not.

packages/plugin-list/src/ObjectGallery.tsx

@@ -7,7 +7,7 @@
  */
 
 import React, { useState, useEffect, useCallback, useMemo, useContext } from 'react';
-import { useDataScope, SchemaRendererContext, useNavigationOverlay, useSafeFieldLabel } from '@object-ui/react';
+import { useDataScope, SchemaRendererContext, useNavigationOverlay, useSafeFieldLabel, useSettledSchema } from '@object-ui/react';
 import { ComponentRegistry, buildExpandFields, getRecordDisplayName } from '@object-ui/core';
 import { cn, Card, CardContent, NavigationOverlay } from '@object-ui/components';
 import type { GalleryConfig, ObjectGallerySchema } from '@object-ui/types';
@@ -219,7 +219,77 @@ export const ObjectGallery: React.FC<ObjectGalleryProps> = (props) => {
 
     const [fetchedData, setFetchedData] = useState<Record<string, unknown>[]>([]);
     const [loading, setLoading] = useState(false);
-    const [objectDef, setObjectDef] = useState<any>(null);
+
+    /**
+     * The object definition, and whether the read for THIS object has SETTLED —
+     * one piece of state, through the shared hook (objectui#7903).
+     *
+     * `ObjectGallery` sat outside the set objectui#6482 converged
+     * (`ObjectKanban`, `ObjectView`, `ObjectCalendar`, `ObjectTree`; `ObjectGantt`
+     * at objectui#7225 ask 2, `ObjectTimeline` at objectui#7895) and nothing
+     * marked it a deliberate exclusion. It still held the definition in a local
+     * `useState` fed by its own metadata effect, with `objectDef` listed in the
+     * record-fetch effect's dependency array below — so every object-bound
+     * gallery load issued the record query TWICE: once before the definition
+     * landed, with `buildExpandFields` seeing no fields and therefore carrying
+     * no `$expand` at all, and once after.
+     *
+     * Measured on THIS component before the change (objectui#6482's per-component
+     * standard — the cost differs by component), instrumented renderer, one mount
+     * per hold, `getObjectSchema` held 0/1/2/3/4/5/6/7/8/9/10/15/25/50/100 ms,
+     * with `ObjectCalendar` as a positive control in the same run: 2 `find` calls
+     * with expand sets `[null, ['owner']]` at EVERY hold, the issue order always
+     * `schema:issued, find(unexpanded), schema:settled, find(expanded)`, two
+     * distinct painted states (raw foreign-key ids, then the expanded rows), 3
+     * late writes into the card grid after the first paint, and a first paint
+     * FLAT at 3-8 ms across the whole 0->100 ms sweep. The control read 1 `find`
+     * carrying `['owner']`, one painted state, and a first paint that TRACKS the
+     * hold (33-69 ms at 0-15 ms, 77 ms at +50, 161 ms at +100).
+     *
+     * ⚠️ This component's visible cost is a TWO-step paint, not the three-step
+     * one `ObjectCalendar` (objectui#6453) and `ObjectTimeline` (objectui#7895)
+     * each measured. Those two make `loading` an unconditional early return, so
+     * the re-run's `setLoading(true)` drops them back to their placeholder
+     * between the two paints. Here the early return is `loading && !items.length`
+     * — once the first (raw) rows are in state the skeleton cannot come back —
+     * so the user sees raw foreign-key ids replaced in place by the expanded
+     * rows. Measured, not inherited: `skelAfter=false` at every hold.
+     *
+     * ⚠️ The gate below is only safe because this resolution SETTLES ON EVERY
+     * EXIT (objectui#7232) — no source, no `getObjectSchema`, no object name, and
+     * a read that threw alike. The hand-written effect it replaces returned
+     * WITHOUT settling on all four, which cost nothing while nothing waited on it
+     * and would hold a gated query open forever.
+     *
+     * ⛔ `dataSource` is passed unconditionally, NOT `hasInlineData ? undefined :
+     * dataSource` the way `ObjectCalendar` and `ObjectGantt` pass it — the same
+     * departure `ObjectTimeline` made, and it applies here for a stronger reason.
+     * Those two read metadata only to expand a record query, so an inline data
+     * set has nothing to wait for. This component reads the definition on EVERY
+     * path, query or not: `buildEnrichedField` above reads `objectDef.fields` for
+     * each visible field's type, options, currency, precision and reference
+     * target, and `getRecordDisplayName(objectDef, item)` below resolves each
+     * card's title under ADR-0079. Disabling the read for authored `data` /
+     * `bind` items would strip cell semantics and card titles off exactly the
+     * paths that issue no query — a second, unasked-for change riding on a
+     * fetch-sequencing fix.
+     *
+     * The key is `schema.objectName` — the object the record query itself names
+     * (`dataSource.find(schema.objectName, …)` below) and the one the replaced
+     * effect read. ⛔ Not `resolveRecordSourceObjectName`, the same departure
+     * `ObjectTimeline` made, and again for a stronger reason: that reader's
+     * second rung is a resolved `data` BLOCK (`dataConfig.provider === 'object'`),
+     * and `ObjectGallerySchema['data']` is typed `Record<string, unknown>[]` — a
+     * bare inline record array, not a provider config. This component calls
+     * `getDataConfig` nowhere, so there is no second name to read; the ladder
+     * would degenerate to `schema.objectName` with extra spelling. Gating on a
+     * key the query does not use is exactly the stale-key mismatch
+     * `useSettledSchema`'s render-time comparison exists to make unrepresentable.
+     */
+    const { ready: objectDefReady, def: objectDef } = useSettledSchema<any>(
+        schema.objectName ?? '',
+        dataSource as any,
+    );
 
     // --- NavigationConfig support ---
     const navigation = useNavigationOverlay({
@@ -288,22 +358,6 @@ export const ObjectGallery: React.FC<ObjectGalleryProps> = (props) => {
       return enriched;
     }, [objectDef, schema.objectName, fieldLabel, fieldOptionLabel]);
 
-    // Fetch object definition for metadata
-    useEffect(() => {
-        let isMounted = true;
-        const fetchMeta = async () => {
-            if (!dataSource || typeof dataSource.getObjectSchema !== 'function' || !schema.objectName) return;
-            try {
-                const def = await dataSource.getObjectSchema(schema.objectName);
-                if (isMounted) setObjectDef(def);
-            } catch (e) {
-                console.warn('Failed to fetch object def for ObjectGallery', e);
-            }
-        };
-        fetchMeta();
-        return () => { isMounted = false; };
-    }, [schema.objectName, dataSource]);
-
     useEffect(() => {
         let isMounted = true;
 
@@ -346,10 +400,37 @@ export const ObjectGallery: React.FC<ObjectGalleryProps> = (props) => {
         };
 
         if (schema.objectName && !boundData && !schema.data && !props.data) {
+            // ⭐ objectui#7903 — the object definition GATES this query; it does
+            // not refine it afterwards. `objectDef` stays in the dependency list
+            // below and the two are ONE mechanism, not two: the dependency is
+            // what makes this effect re-run when the definition lands, and this
+            // branch is what stops the first run from spending a query before it
+            // has. Removing either half alone restores the double fetch.
+            //
+            // Scoped to the branch that actually issues the query. The authored
+            // (`props.data` / `schema.data`) and bound (`bind`) paths never query,
+            // so gating them would hold nothing useful — and this component still
+            // reads the definition on those paths (cell semantics and ADR-0079
+            // card titles), which is why the resolution above is not disabled for
+            // them.
+            if (!objectDefReady) {
+                // ⚠️ Hold the placeholder across the gate window. `loading` starts
+                // `false` here and was only ever flipped inside `fetchData`, so a
+                // bare `return` would leave the gallery showing "No items to
+                // display" — a FALSE empty state — for the whole metadata read,
+                // where before the gate it showed the loading placeholder. The
+                // two siblings get this for free from their own initial state
+                // (`ObjectCalendar` starts `loading` at `true`; `ObjectTimeline`
+                // computes the same thing in a lazy initializer); this component
+                // does not, so the same guarantee is stated here, in the one
+                // branch that knows a query is coming.
+                if (isMounted) setLoading(true);
+                return;
+            }
             fetchData();
         }
         return () => { isMounted = false; };
-    }, [schema.objectName, dataSource, boundData, schema.data, schema.filter, props.data, objectDef]);
+    }, [schema.objectName, dataSource, boundData, schema.data, schema.filter, props.data, objectDefReady, objectDef]);
 
     const items: Record<string, unknown>[] = props.data || boundData || schema.data || fetchedData || [];