From c976f43d7d2edffa33831ee02f129df6a16f368e Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 25 Aug 2026 22:44:17 +0000 Subject: [PATCH 1/2] fix(plugin-kanban): gate ObjectKanban's record query on the object definition MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The board ran its fetch effect twice on every standalone mount: `objectDef` sat in that effect's dependency list while a separate effect resolved it, so the first query went out before the definition landed and carried no `$expand` at all, and a second, expanded one followed. The definition now gates the query rather than refining it afterwards — one query per mount, expanded the first time. The gate is on the read having SETTLED, not on the definition being truthy: an adapter with no `getObjectSchema`, and a read that throws, settle with nothing to report and the board falls through to an unexpanded query instead of waiting forever. Chosen on measurement (recorded in the PR body and the changeset): the first response never reached the screen in the regimes that matter, and a schema read is one small cached GET behind the discovery call `find` already awaits. Part of #6271 Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_011SfZeFWrhGLHmfq61xbz4q --- .changeset/6271-kanban-fetch-gate.md | 41 ++++ packages/plugin-kanban/src/ObjectKanban.tsx | 98 +++++++- .../fetchGate.objectDef-6271.test.tsx | 230 ++++++++++++++++++ 3 files changed, 363 insertions(+), 6 deletions(-) create mode 100644 .changeset/6271-kanban-fetch-gate.md create mode 100644 packages/plugin-kanban/src/__tests__/fetchGate.objectDef-6271.test.tsx diff --git a/.changeset/6271-kanban-fetch-gate.md b/.changeset/6271-kanban-fetch-gate.md new file mode 100644 index 0000000000..dd74c6c0bb --- /dev/null +++ b/.changeset/6271-kanban-fetch-gate.md @@ -0,0 +1,41 @@ +--- +'@object-ui/plugin-kanban': patch +--- + +`ObjectKanban` no longer queries twice on mount (objectui#6271). A standalone board issued +its first `find` before the object definition resolved — so `buildExpandFields` saw no +fields and that query carried no `$expand` at all — then issued a second, expanded one once +the definition landed: + +``` +['deal', { $top: 100 }] +['deal', { $top: 100, $expand: ['owner'] }] +``` + +The definition now GATES the query instead of refining it afterwards: one query per mount, +carrying the expansion the first time. + +Decided on measurement rather than on the two shapes' relative appeal. The first response +never reached the screen in the regimes that matter: with the schema resolving no slower +than the row query (profiles schema/find = 30/30, 30/60, 5/30 ms), the definition lands +first, the effect re-runs, its cleanup flips `isMounted` false, and the unexpanded rows are +discarded on arrival — a DOM probe polling every 2ms for a title only that response carried +never fired once. What the gate costs is one schema resolution ahead of the query, and that +read is cheap and shared: one small GET behind the same discovery call `find` already +awaits, served thereafter from `MetadataCache` (5-minute TTL, concurrent readers coalesced +onto one request). Measured against the real `ObjectStackAdapter` over loopback HTTP, 22 +reads of one object produced exactly one metadata request and every read after the first +returned in 0.01ms. End to end the board is not slower for it — same harness, before → +after, time to the fully populated board: 156.9 → 145.2ms (30/30), 119.8 → 110.6ms (30/60), +54.7 → 52.4ms (5/30). + +The gate is on the definition read having **settled**, not on the definition being truthy: +an adapter that exposes no `getObjectSchema`, and a read that throws, both settle with +nothing to report and the board falls through to an unexpanded query rather than waiting +forever. Boards fed rows by a parent (`data`, `bind`, inline `schema.data`) are untouched — +they never ran this effect, and they still read the definition for lane titles and labels. + +The `isOpaqueId` suppression in the card-description path is unchanged and keeps its +comment beside it: part of what it hid was this fetch ordering, but unexpanded rows still +reach it from parents that pass rows they fetched without `$expand`, from author-supplied +data, and from backends that decline an expansion. diff --git a/packages/plugin-kanban/src/ObjectKanban.tsx b/packages/plugin-kanban/src/ObjectKanban.tsx index 2c99db839a..1e99dfa648 100644 --- a/packages/plugin-kanban/src/ObjectKanban.tsx +++ b/packages/plugin-kanban/src/ObjectKanban.tsx @@ -172,7 +172,24 @@ export const ObjectKanban: React.FC = ({ const hasExternalData = Array.isArray(externalData); const [fetchedData, setFetchedData] = useState([]); - const [objectDef, setObjectDef] = useState(null); + // The object-definition read and the fact that it has SETTLED are one piece + // of state, keyed by the object it belongs to (objectui#6271). Two separate + // states could disagree for one commit — long enough for the record query to + // fire against the previous object's expand set — and a bare `objectDef` + // cannot express "settled with nothing", which is a legitimate outcome (an + // adapter with no `getObjectSchema`, or a read that threw). `key` is compared + // against the CURRENT object name during render, so switching objects closes + // the gate in the same commit that changes it, not one commit later. + const [schemaResolution, setSchemaResolution] = useState<{ key: string; def: any } | null>(null); + const schemaKey = schema.objectName ?? ''; + /** + * Has the object definition for THIS object finished resolving? Note what + * this is NOT: "`objectDef` is truthy". A board whose adapter exposes no + * `getObjectSchema`, or whose schema read failed, must still get its cards — + * gating on a truthy definition would leave those boards empty forever. + */ + const objectDefReady = schemaResolution !== null && schemaResolution.key === schemaKey; + const objectDef = objectDefReady ? schemaResolution.def : null; // loading state const [loading, setLoading] = useState(hasExternalData ? (externalLoading ?? false) : false); const [error, setError] = useState(null); @@ -201,16 +218,29 @@ export const ObjectKanban: React.FC = ({ } }, [externalLoading, hasExternalData]); - // Fetch object definition for metadata (labels, options) + // Fetch object definition for metadata (labels, options). + // + // Every exit settles the resolution — success, failure, and "there is nothing + // to read from" alike — because the record query below WAITS on this + // (objectui#6271). A path that returned without settling would not merely + // skip the expansion, it would hold the query open forever. useEffect(() => { let isMounted = true; + const key = schema.objectName ?? ''; const fetchMeta = async () => { - if (!dataSource || !schema.objectName) return; + if (!dataSource || !schema.objectName || typeof dataSource.getObjectSchema !== 'function') { + // No source for a definition: settle with none, so the board still + // queries (unexpanded — with no schema there is no expand set to + // derive, which is the same query this case produced before). + if (isMounted) setSchemaResolution({ key, def: null }); + return; + } try { const def = await dataSource.getObjectSchema(schema.objectName); - if (isMounted) setObjectDef(def); + if (isMounted) setSchemaResolution({ key, def }); } catch (e) { console.warn("Failed to fetch object def", e); + if (isMounted) setSchemaResolution({ key, def: null }); } }; fetchMeta(); @@ -221,12 +251,36 @@ export const ObjectKanban: React.FC = ({ // Skip internal fetch when data is managed by a parent component if (hasExternalData) return; + // ⭐ objectui#6271 — the object definition GATES this query; it does not + // refine it afterwards. Before this line the effect ran twice on every + // mount: once with `objectDef` still unresolved (so `buildExpandFields` + // saw no fields and the query carried NO `$expand` at all), then again + // once the definition landed. Measured on the standalone board, the first + // response never even reached the screen in the regimes that matter — the + // definition settles first, the effect re-runs, its cleanup flips + // `isMounted` false, and the unexpanded rows are dropped on arrival. So + // that round trip bought no earlier paint; it was a query whose answer was + // thrown away. + // + // What the gate costs is one schema resolution before the first query, and + // that is the measurement this was decided on: a metadata read is one small + // GET behind the same shared discovery call `find` already awaits, and it + // is served from `MetadataCache` (5-min TTL, concurrent readers coalesced + // onto one request) for every reader after the first — 0.01ms, no request. + // Anything hosting this board (ObjectView, ListView) has already read the + // same definition through the same adapter, so the gate is free there. + if (!objectDefReady) return; + let isMounted = true; const fetchData = async () => { if (!dataSource || typeof dataSource.find !== 'function' || !schema.objectName) return; if (isMounted) setLoading(true); try { - // Auto-inject $expand for lookup/master_detail fields + // Auto-inject $expand for lookup/master_detail fields. Reached only + // with the definition resolved (the gate above), so a board whose + // object declares lookups queries WITH its expansion the first + // time — `objectDef` here is `null` only when there was nothing to + // resolve it from. const expand = buildExpandFields(objectDef?.fields); // The row cap is a REAL `$top` (objectui#4025). It used to be // `{ options: { $top: 100 } }` — `$filter` at the top level where the @@ -259,7 +313,11 @@ export const ObjectKanban: React.FC = ({ fetchData(); } return () => { isMounted = false; }; - }, [schema.objectName, dataSource, boundData, schema.data, schema.filter, schema.limit, hasExternalData, objectDef, refreshKey]); + // `objectDefReady` is what re-runs this effect once the definition lands; + // `objectDef` stays listed because the body reads it, and with the gate in + // place the two flip together in one commit — the pre-resolution run now + // returns above without querying instead of issuing an unexpanded one. + }, [schema.objectName, dataSource, boundData, schema.data, schema.filter, schema.limit, hasExternalData, objectDefReady, objectDef, refreshKey]); // Determine which data to use: external -> bound -> inline -> fetched const rawData = (hasExternalData ? externalData : undefined) || boundData || schema.data || fetchedData; @@ -321,6 +379,34 @@ export const ObjectKanban: React.FC = ({ }; // Detect strings that look like opaque foreign-key IDs so we don't dump // gibberish into card descriptions when the server didn't expand the lookup. + // + // ⭐ WHY THIS IS STILL HERE, given objectui#6271 (read this before + // deleting it). Part of what this suppression used to hide was THIS + // component's own fetch ordering: the board issued its first query before + // the object definition resolved, so that query carried no `$expand` and + // the first paint was rendered from raw lookup ids. That half is gone — + // the record query is now gated on the definition (see the fetch effect + // above), so the board's own rows arrive expanded or not at all. + // + // The suppression is NOT thereby redundant, because unexpanded rows still + // reach this function from sources the gate does not sit in front of: + // + // 1. `data` handed down by a parent. Measured, not assumed: ObjectView + // hosts the board this way and its own query goes out as + // `{ $top: 100 }` — it reads the schema through a ref that is still + // empty on the one run it makes, so it never injects `$expand` at + // all. Every card in that path is built from raw ids. + // 2. `bind` / inline `schema.data` — author-supplied rows, expanded by + // nobody. + // 3. An adapter or backend that ignores `$expand`, or a single lookup + // it cannot resolve (a dangling reference), which comes back as the + // bare id inside an otherwise expanded row. + // + // So the coupling the two issues share is real and stays recorded: the + // display heuristic and the fetch ordering move together. What changed is + // the JUSTIFICATION, not the code — this predicate is now a guard against + // genuinely unexpanded DATA, no longer a cover for a query this component + // issued too early. const OPAQUE_ID_RE = /^[A-Za-z0-9_-]{12,32}$/; const isOpaqueId = (v: unknown): boolean => { if (typeof v !== 'string') return false; diff --git a/packages/plugin-kanban/src/__tests__/fetchGate.objectDef-6271.test.tsx b/packages/plugin-kanban/src/__tests__/fetchGate.objectDef-6271.test.tsx new file mode 100644 index 0000000000..4d557d60dd --- /dev/null +++ b/packages/plugin-kanban/src/__tests__/fetchGate.objectDef-6271.test.tsx @@ -0,0 +1,230 @@ +/** + * 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#6271 — the object definition GATES `ObjectKanban`'s record query. + * + * ## What this replaces + * + * The board ran its fetch effect twice on every standalone mount, because + * `objectDef` was in that effect's dependency list while being resolved by a + * SEPARATE effect. Captured from a real render, the two argument sets in order: + * + * ['deal', { $top: 100 }] + * ['deal', { $top: 100, $expand: ['owner'] }] + * + * The first ran before the definition resolved, so `buildExpandFields` saw no + * fields and the query carried no `$expand` at all. + * + * ## Why gating, and not "make the unexpanded first paint deliberate" + * + * Both were live options and the choice was made on measurement, not taste. + * + * 1. That first response never reached the screen in the regimes that + * matter. With the schema resolving no slower than the row query + * (measured profiles schema/find = 30/30, 30/60, 5/30 ms), the definition + * lands first, the effect re-runs, its cleanup flips `isMounted` false — + * and the unexpanded rows are DISCARDED on arrival. A probe watching the + * DOM every 2ms for a title only the first response carried never fired + * once. So the extra round trip bought no earlier paint; it bought a + * query whose answer was thrown away. + * 2. What the gate costs is one schema resolution ahead of the query, and a + * schema read is cheap and shared: one small GET behind the same + * discovery call `find` already awaits, served thereafter from + * `MetadataCache` (5-minute TTL, concurrent readers coalesced onto ONE + * request). Measured against the real `ObjectStackAdapter` over loopback + * HTTP: 22 reads of the same object produced exactly one metadata + * request, and every read after the first returned in 0.01ms. + * 3. End to end the board is not slower for it. Same harness, before/after: + * the fully-populated board landed at 156.9 → 145.2ms (30/30), + * 119.8 → 110.6ms (30/60), 54.7 → 52.4ms (5/30) — one query instead of + * two, and nothing left to overwrite. + * + * ## ⚠️ What "gated" must mean — the trap this file exists to hold shut + * + * The gate is on the definition read having **settled**, NOT on `objectDef` + * being truthy. Those differ for exactly the boards least able to report it: + * an adapter that exposes no `getObjectSchema`, and a schema read that throws. + * Under a truthy-value gate both wait forever and the board renders empty, with + * no error and no request — the third and fourth tests below are red the moment + * anyone writes that. They are also the control the first test needs: a + * query-count assertion that would pass with ZERO fetches is decoration, so + * every count here is reached only after waiting for a real call. + */ +import { describe, it, expect, vi } from 'vitest'; +import { render, waitFor } from '@testing-library/react'; +import React from 'react'; +import { SchemaRenderer, SchemaRendererProvider } from '@object-ui/react'; +// Registers `object-kanban`. +import '../index'; +// The cards asserted below render INSIDE `KanbanRenderer`'s `React.lazy` +// boundary. Importing the chunk at module scope bills the cold transform to the +// import phase (unbounded) instead of racing a `waitFor` budget under full +// parallelism — the objectui#3010 rule, same specifier as `index.tsx`'s factory +// so ESM's module cache makes that factory resolve immediately. +import '../KanbanImpl'; + +const DEAL_SCHEMA = { + name: 'deal', + label: 'Deal', + fields: { + name: { type: 'text', label: 'Name' }, + status: { type: 'text', label: 'Status' }, + // The only expandable member, so `$expand` has one predictable entry. + owner: { type: 'lookup', reference_to: 'user', label: 'Owner' }, + }, +}; + +const ROWS = [{ id: 'd1', name: 'Q3 renewal', status: 'open', owner: { id: 'u1', name: 'Jane Ops' } }]; + +/** + * `getObjectSchema` deliberately resolves a tick LATER than a bare + * `mockResolvedValue` would, so a board that queries before the definition + * settles is caught rather than accidentally passing on scheduling luck. + */ +function makeAdapter( + getObjectSchema?: () => Promise, +): Record { + const order: string[] = []; + const adapter: Record = { + order, + find: vi.fn(async (_object: string, _params: any) => { + order.push('find'); + return { data: ROWS }; + }), + findOne: vi.fn(), + create: vi.fn(), + update: vi.fn(), + delete: vi.fn(), + }; + if (getObjectSchema) { + adapter.getObjectSchema = vi.fn(async (...args: unknown[]) => { + order.push('schema:issued'); + try { + return await getObjectSchema.apply(null, args as []); + } finally { + order.push('schema:settled'); + } + }); + } + return adapter; +} + +const resolvesSchema = () => + makeAdapter(async () => { + await new Promise((r) => setTimeout(r, 10)); + return DEAL_SCHEMA; + }); + +function renderBoard(adapter: Record) { + return render( + + + , + ); +} + +const paramsOf = (adapter: Record) => + adapter.find.mock.calls.map((c: any[]) => c[1] ?? {}); +const unexpandedCalls = (adapter: Record) => + paramsOf(adapter).filter((p: any) => !Array.isArray(p.$expand) || p.$expand.length === 0); + +describe('ObjectKanban gates its record query on the object definition (objectui#6271)', () => { + it('issues ONE query, and it carries the object’s `$expand`', async () => { + const adapter = resolvesSchema(); + renderBoard(adapter); + + // Wait for the EXPANDED query specifically. This is the control that keeps + // the two assertions under it from being vacuous: if the gate ever stops + // opening, no call is recorded, this times out, and the file goes red — + // "0 queries" can never read as success here. + await waitFor(() => + expect(paramsOf(adapter).filter((p: any) => p.$expand?.includes('owner'))).toHaveLength(1), + ); + + // RED before the fix: the board also issued `['deal', { $top: 100 }]` + // before the definition resolved. + expect(unexpandedCalls(adapter)).toEqual([]); + expect(adapter.find).toHaveBeenCalledTimes(1); + expect(adapter.find.mock.calls[0][0]).toBe('deal'); + }); + + it('issues that query only AFTER the definition read settles', async () => { + const adapter = resolvesSchema(); + renderBoard(adapter); + + await waitFor(() => expect(adapter.find).toHaveBeenCalled()); + // Ordering, not just counting: a fix that merely deduplicated the second + // query would satisfy the test above while still querying too early. + expect(adapter.order).toEqual(['schema:issued', 'schema:settled', 'find']); + }); + + it('still queries — and paints — when the adapter exposes NO `getObjectSchema`', async () => { + // The gate is on the read having settled, not on a truthy definition. An + // adapter without the method settles with nothing to report, and the board + // must fall through to an unexpanded query rather than wait forever. + const adapter = makeAdapter(); + const { container } = renderBoard(adapter); + + await waitFor(() => expect(container.textContent).toContain('Q3 renewal')); + expect(adapter.find).toHaveBeenCalledTimes(1); + // Nothing declared any field, so there is no expand set to derive. + expect(unexpandedCalls(adapter)).toHaveLength(1); + }); + + it('still queries — and paints — when the definition read REJECTS', async () => { + const adapter = makeAdapter(async () => { + await new Promise((r) => setTimeout(r, 10)); + throw new Error('metadata endpoint down'); + }); + const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}); + try { + const { container } = renderBoard(adapter); + await waitFor(() => expect(container.textContent).toContain('Q3 renewal')); + expect(adapter.find).toHaveBeenCalledTimes(1); + expect(unexpandedCalls(adapter)).toHaveLength(1); + expect(adapter.order).toEqual(['schema:issued', 'schema:settled', 'find']); + } finally { + warn.mockRestore(); + } + }); + + it('a parent-fed board still reads the definition and issues no query of its own', async () => { + // `data` from a parent has always suppressed the internal fetch; the gate + // must not have turned that into a suppressed SCHEMA read, which is what + // the lane titles and card labels are built from. + const adapter = resolvesSchema(); + render( + + + , + ); + + await waitFor(() => expect(adapter.getObjectSchema).toHaveBeenCalled()); + expect(adapter.find).not.toHaveBeenCalled(); + }); +}); From 88e10b836372827d4e1bf7c38f5bf725f6152aea Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 25 Aug 2026 22:54:19 +0000 Subject: [PATCH 2/2] test(plugin-kanban): re-spell the #5874 expand-delta wait off the double fetch MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `awaitSchemaFetch` waited for a SECOND `find` call, because only the second carried the expand set — a synchronisation built on the defect #6271 removes. Same condition, spelled against the ordering that now holds: one call, issued after the definition settles. The comment records why one call is sufficient and what re-opens the vacuum if the gate is ever removed. Also drops a `.apply()` in the new pin's adapter stub (`prefer-spread`). Part of #6271 Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_011SfZeFWrhGLHmfq61xbz4q --- .../expandableFamily.identity-5874.test.tsx | 23 +++++++++++++------ .../fetchGate.objectDef-6271.test.tsx | 4 ++-- 2 files changed, 18 insertions(+), 9 deletions(-) diff --git a/packages/plugin-kanban/src/__tests__/expandableFamily.identity-5874.test.tsx b/packages/plugin-kanban/src/__tests__/expandableFamily.identity-5874.test.tsx index 6e2d005d70..2a3e76dbf2 100644 --- a/packages/plugin-kanban/src/__tests__/expandableFamily.identity-5874.test.tsx +++ b/packages/plugin-kanban/src/__tests__/expandableFamily.identity-5874.test.tsx @@ -296,15 +296,24 @@ describe('the membership delta is OBSERVABLE on this read — the counter-probe * stand in `resolveDisplay` lacked. So this pair is exactly the delta the * convergence introduced, read off the wire instead of off the DOM. * - * ⚠️ The board fetches TWICE: once before the object schema resolves (no - * field types are known yet, so no `$expand` can be computed) and again once - * `objectDef` arrives. Only the second call can carry the delta, so both - * cases below wait for it — otherwise the negative case would be green - * against a query that had not been built yet, which is the vacuous form of - * this assertion. + * ⚠️ This wait exists to keep the negative case from going green against a + * query that had not been built yet — the vacuous form of this assertion. It + * used to spell that as "wait for a SECOND call", because the board fetched + * twice: once before the object definition resolved (no field types known, + * so no `$expand` computable) and again once it arrived, and only the second + * could carry the delta. + * + * objectui#6271 gated the query on the definition instead, so there is now + * exactly ONE call and it is the schema-informed one. The wait moved with it + * — same condition ("a query built from resolved field types has been + * issued"), spelled against the ordering that actually holds. ⛔ Do not read + * this as a weakened wait: what makes one call sufficient is the gate. If + * that gate is ever removed, the pre-resolution query comes back, this wait + * starts admitting it, and the negative case below goes vacuous again — + * `fetchGate.objectDef-6271.test.tsx` is what fails first if anyone tries. */ const awaitSchemaFetch = (adapter: ReturnType) => - waitFor(() => expect(adapter.find.mock.calls.length).toBeGreaterThanOrEqual(2)); + waitFor(() => expect(adapter.find.mock.calls.length).toBeGreaterThanOrEqual(1)); const everyExpand = (adapter: ReturnType): string[] => adapter.find.mock.calls.flatMap( diff --git a/packages/plugin-kanban/src/__tests__/fetchGate.objectDef-6271.test.tsx b/packages/plugin-kanban/src/__tests__/fetchGate.objectDef-6271.test.tsx index 4d557d60dd..d256ae87b1 100644 --- a/packages/plugin-kanban/src/__tests__/fetchGate.objectDef-6271.test.tsx +++ b/packages/plugin-kanban/src/__tests__/fetchGate.objectDef-6271.test.tsx @@ -103,10 +103,10 @@ function makeAdapter( delete: vi.fn(), }; if (getObjectSchema) { - adapter.getObjectSchema = vi.fn(async (...args: unknown[]) => { + adapter.getObjectSchema = vi.fn(async () => { order.push('schema:issued'); try { - return await getObjectSchema.apply(null, args as []); + return await getObjectSchema(); } finally { order.push('schema:settled'); }