fix(plugin-tree): key ObjectTree's schema-settled gate to the bound object

.changeset/6481-objecttree-keyed-schema-latch.md

@@ -0,0 +1,32 @@
+---
+'@object-ui/plugin-tree': patch
+---
+
+`ObjectTree` no longer queries a switched-to object with the previous object's
+`$expand` set (objectui#6481).
+
+The schema-settled gate was two separate pieces of state — the definition
+(`objectSchema`) and a bare `schemaSettled` boolean that was set `true` in a
+`finally` and never reset. Two independent values cannot express "settled, but
+for a DIFFERENT object", so when the host swapped the bound object both effects
+re-ran while the latch still read `true` from the previous object's settle and
+the definition still held the previous object's fields. The tree issued
+
+    find(newObject, { $filter: …, $expand: [ …previous object's relation fields… ] })
+
+— rejected or silently ignored depending on the adapter, plus the transient it
+painted — before a correct second query followed.
+
+`ObjectTree` now adopts `useSettledSchema` from `@object-ui/react` (the shared
+resolution hook ruled in objectui#6482), replacing BOTH pieces of state with the
+hook's single `{ key, def } | null`. Readiness is derived during render by
+comparing the settled key against the currently bound object, so the gate closes
+in the same commit that changes the object rather than one commit later — the
+stale-key window is not merely fixed but unrepresentable.
+
+Behaviour that deliberately does NOT change: the gate stays inside the
+object-provider branch of the record effect (the inline/static branches issue no
+metadata read and must not wait on one), and every exit still settles — no
+`getObjectSchema`, no object name, or a rejected read each settle with no
+definition, so a tree whose adapter serves no schema still queries instead of
+waiting forever.

packages/plugin-tree/src/ObjectTree.settledSchemaKeying-6481.test.tsx

@@ -0,0 +1,213 @@
+/**
+ * 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.
+ */
+
+/**
+ * ObjectTree — the settled-schema gate must be KEYED to the bound object
+ * (objectui#6481), and must still settle on EVERY exit (objectui#6014).
+ *
+ * These two properties pull in opposite directions and that is the whole
+ * point of the file: one of them says "do not query until the schema is in",
+ * the other says "never wait for a schema that is not coming". A latch that
+ * satisfies only the first hangs forever on an adapter with no
+ * `getObjectSchema`; a latch that satisfies only the second — the bare
+ * boolean this card replaces — queries the NEW object with the OLD object's
+ * `$expand`.
+ *
+ * Measured on THIS component before the fix, not transferred from
+ * objectui#6453 / #6419: `ObjectTree`'s record effect has its own dependency
+ * set (`dataConfig`, `dataSource`, `schema.filter`, the schema resolution,
+ * `(rest as any).data`), and its gate sits INSIDE the object-provider branch
+ * rather than at the top of the effect.
+ */
+
+import React from 'react';
+import { render, waitFor, cleanup, act } from '@testing-library/react';
+import { describe, it, expect, afterEach, beforeEach, vi } from 'vitest';
+import { ObjectTree } from './ObjectTree';
+
+afterEach(cleanup);
+
+interface FindCall {
+  object: string;
+  options: any;
+}
+
+/** A promise a test resolves by hand, so "not settled yet" is a real state. */
+function deferred<T>() {
+  let resolve!: (value: T) => void;
+  let reject!: (reason?: unknown) => void;
+  const promise = new Promise<T>((res, rej) => {
+    resolve = res;
+    reject = rej;
+  });
+  return { promise, resolve, reject };
+}
+
+/**
+ * Two self-referencing objects whose EXPANDABLE field sets are disjoint apart
+ * from the parent pointer. `head` exists only on `business_unit` and `region`
+ * only on `territory`, so an `$expand` naming the wrong one is unambiguous
+ * evidence of which object's schema built it.
+ */
+const SCHEMAS: Record<string, any> = {
+  business_unit: {
+    name: 'business_unit',
+    fields: {
+      name: { type: 'text' },
+      parent_id: { type: 'tree', reference: 'business_unit' },
+      head: { type: 'lookup', reference: 'users' },
+    },
+  },
+  territory: {
+    name: 'territory',
+    fields: {
+      name: { type: 'text' },
+      parent_id: { type: 'tree', reference: 'territory' },
+      region: { type: 'lookup', reference: 'regions' },
+    },
+  },
+};
+
+function treeSchema(objectName: string) {
+  return {
+    type: 'object-tree',
+    objectName,
+    parentField: 'parent_id',
+    labelField: 'name',
+    fields: ['name'],
+  } as any;
+}
+
+let errorSpy: ReturnType<typeof vi.spyOn>;
+beforeEach(() => {
+  // The rejected-read path logs; keep the suite output honest without
+  // swallowing a real unexpected error (asserted on in that test).
+  errorSpy = vi.spyOn(console, 'error').mockImplementation(() => {});
+});
+afterEach(() => {
+  errorSpy.mockRestore();
+});
+
+describe('ObjectTree settled-schema gate is keyed to the bound object (objectui#6481)', () => {
+  it('never queries a switched-to object with the previous object\'s $expand', async () => {
+    const findCalls: FindCall[] = [];
+    const gates: Record<string, ReturnType<typeof deferred<any>>> = {
+      business_unit: deferred<any>(),
+      territory: deferred<any>(),
+    };
+    const schemaRequests: string[] = [];
+
+    const dataSource: any = {
+      getObjectSchema: (objectName: string) => {
+        schemaRequests.push(objectName);
+        return gates[objectName].promise;
+      },
+      find: async (object: string, options: any) => {
+        findCalls.push({ object, options });
+        return [];
+      },
+    };
+
+    const { rerender } = render(
+      <ObjectTree schema={treeSchema('business_unit')} dataSource={dataSource} />,
+    );
+
+    // Leg 1 — the first object settles and queries with ITS OWN expand set.
+    await act(async () => {
+      gates.business_unit.resolve(SCHEMAS.business_unit);
+      await gates.business_unit.promise;
+    });
+    await waitFor(() => expect(findCalls.length).toBe(1));
+    expect(findCalls[0].object).toBe('business_unit');
+    expect([...findCalls[0].options.$expand].sort()).toEqual(['head', 'parent_id']);
+
+    // Leg 2 — the host swaps the bound object. `territory`'s schema has NOT
+    // settled yet, so nothing is entitled to build an `$expand` for it.
+    await act(async () => {
+      rerender(<ObjectTree schema={treeSchema('territory')} dataSource={dataSource} />);
+    });
+
+    // The defect, stated as the query it emits: with an unkeyed boolean latch
+    // the gate reads "settled" from `business_unit`'s settle and the previous
+    // resolution is still in state, so this fires
+    //   find('territory', { $expand: ['parent_id', 'head'] })
+    // — `head` is not a field `territory` declares.
+    const territoryCalls = findCalls.filter((c) => c.object === 'territory');
+    const strayExpands = territoryCalls
+      .map((c) => (c.options?.$expand ?? []) as string[])
+      .filter((expand) => expand.some((f) => !(f in SCHEMAS.territory.fields)));
+    expect(strayExpands).toEqual([]);
+
+    // Stated the second way: no query at all may go out for an object whose
+    // schema has not settled. (The two assertions fail together on the bare
+    // boolean; keeping both says WHICH property broke if they ever diverge.)
+    expect(territoryCalls).toEqual([]);
+
+    // Leg 3 — once the new schema lands, the correct query follows. The gate
+    // must close, not deadlock.
+    await act(async () => {
+      gates.territory.resolve(SCHEMAS.territory);
+      await gates.territory.promise;
+    });
+    await waitFor(() => {
+      const calls = findCalls.filter((c) => c.object === 'territory');
+      expect(calls.length).toBeGreaterThan(0);
+    });
+    const settled = findCalls.filter((c) => c.object === 'territory');
+    expect([...settled[settled.length - 1].options.$expand].sort()).toEqual([
+      'parent_id',
+      'region',
+    ]);
+    // Exactly one query per object — the switch must not re-query the object
+    // it left, and must not double-query the one it arrived at.
+    expect(findCalls.map((c) => c.object)).toEqual(['business_unit', 'territory']);
+    expect(schemaRequests).toEqual(['business_unit', 'territory']);
+  });
+
+  it('still queries when the adapter exposes no getObjectSchema (every exit settles)', async () => {
+    const findCalls: FindCall[] = [];
+    // No `getObjectSchema` at all: the resolution has no source to read from.
+    // It must SETTLE with no definition rather than stay pending, or the gated
+    // record query waits forever — objectui#6014's `finally`, which this card
+    // must not trade away.
+    const dataSource: any = {
+      find: async (object: string, options: any) => {
+        findCalls.push({ object, options });
+        return [];
+      },
+    };
+
+    render(<ObjectTree schema={treeSchema('business_unit')} dataSource={dataSource} />);
+
+    await waitFor(() => expect(findCalls.length).toBe(1));
+    expect(findCalls[0].object).toBe('business_unit');
+    // No schema means no expand set to derive — the key must be absent, not
+    // an empty array.
+    expect('$expand' in findCalls[0].options).toBe(false);
+  });
+
+  it('still queries when the schema read rejects (every exit settles)', async () => {
+    const findCalls: FindCall[] = [];
+    const dataSource: any = {
+      getObjectSchema: async () => {
+        throw new Error('metadata endpoint down');
+      },
+      find: async (object: string, options: any) => {
+        findCalls.push({ object, options });
+        return [];
+      },
+    };
+
+    render(<ObjectTree schema={treeSchema('business_unit')} dataSource={dataSource} />);
+
+    await waitFor(() => expect(findCalls.length).toBe(1));
+    expect(findCalls[0].object).toBe('business_unit');
+    expect('$expand' in findCalls[0].options).toBe(false);
+    expect(errorSpy).toHaveBeenCalled();
+  });
+});

packages/plugin-tree/src/ObjectTree.tsx

@@ -21,7 +21,7 @@
 
 import React, { useEffect, useMemo, useState } from 'react';
 import type { DataSource, ViewData } from '@object-ui/types';
-import { useNavigationOverlay, useSafeFieldLabel } from '@object-ui/react';
+import { useNavigationOverlay, useSafeFieldLabel, useSettledSchema } from '@object-ui/react';
 import { NavigationOverlay, cn } from '@object-ui/components';
 import { createSafeTranslation } from '@object-ui/i18n';
 import {
@@ -338,65 +338,65 @@ export const ObjectTree: React.FC<ObjectTreeProps> = ({
   const [records, setRecords] = useState<any[]>([]);
   const [loading, setLoading] = useState(true);
   const [error, setError] = useState<Error | null>(null);
-  const [objectSchema, setObjectSchema] = useState<any>(null);
+  const dataConfig = useMemo(() => getDataConfig(schema), [schema]);
+
   /**
-   * Whether the object-schema fetch below has finished — settled, not
-   * successful: a dataSource that cannot serve a schema, an object with no
-   * name, and a rejected fetch all count, so the record fetch can never be
-   * blocked forever by a schema that is never going to arrive.
-   *
-   * A one-way latch on purpose. Re-arming it on every run of that effect would
-   * mean a `setState` in the effect body, and this component's dependency list
-   * includes `dataConfig` — a `useMemo` over the `schema` PROP object — so a
-   * host that rebuilds its schema each render would turn a benign re-run into a
-   * render loop. Not re-arming costs at most one fetch against a stale schema
-   * when `objectName` changes mid-life, which is exactly what happened on every
-   * fetch before this.
+   * The object THIS render is bound to, as a plain string — so the resolution
+   * below re-keys on the OBJECT rather than on `dataConfig`, a `useMemo` over
+   * the `schema` PROP object whose identity a host that rebuilds its schema
+   * each render changes without changing which object is bound.
    */
-  const [schemaSettled, setSchemaSettled] = useState(false);
+  const schemaKey =
+    (dataConfig?.provider === 'object' ? dataConfig.object : schema.objectName) ?? '';
 
-  const dataConfig = useMemo(() => getDataConfig(schema), [schema]);
-
-  // Fetch the object schema whenever the dataSource can serve one.
-  //
-  // It feeds FOUR things: parent-field auto-detection, column labels, the
-  // `$expand` list built below, and (objectui#6014) the per-field definitions
-  // the cell formatter reads to resolve select options and reference values.
-  //
-  // This used to be gated on "the host passed no inline data", which read as a
-  // cheap skip but disagreed with the record-fetch effect below: THAT branch
-  // prefers a live object dataSource over any inline `data`, so on the one
-  // mount shape `ListView` actually uses (objectName + dataSource + its own
-  // pre-fetched `data`) the tree ran its own query with
-  // `buildExpandFields(undefined)` → `[]` → no `$expand` at all, and had no
-  // field definitions to format cells with. That is the whole of objectui#6014:
-  // lookups rendered as bare ids and selects as raw stored values, on the very
-  // page whose flat-table tab rendered both correctly. The guard inside
-  // `fetchSchema` already no-ops without a dataSource, so dropping the gate
-  // costs nothing on the pure inline/static path.
-  useEffect(() => {
-    let cancelled = false;
-    const fetchSchema = async () => {
-      try {
-        if (!dataSource || typeof dataSource.getObjectSchema !== 'function') return;
-        const objectName =
-          dataConfig?.provider === 'object' ? dataConfig.object : schema.objectName;
-        if (!objectName) return;
-        const result = await dataSource.getObjectSchema(objectName);
-        if (!cancelled) setObjectSchema(result);
-      } catch (err) {
-        console.error('[ObjectTree] Failed to fetch object schema:', err);
-      } finally {
-        // `finally`, so the two early `return`s and a rejected fetch all settle
-        // too — see the latch's docstring.
-        if (!cancelled) setSchemaSettled(true);
-      }
-    };
-    fetchSchema();
-    return () => {
-      cancelled = true;
-    };
-  }, [schema.objectName, dataSource, dataConfig]);
+  /**
+   * The object schema, and whether it has settled FOR `schemaKey` — a single
+   * piece of state, from the shared hook ruled in objectui#6482.
+   *
+   * It feeds FOUR things: parent-field auto-detection, column labels, the
+   * `$expand` list built below, and (objectui#6014) the per-field definitions
+   * the cell formatter reads to resolve select options and reference values.
+   *
+   * ## Why the hook, and not the two `useState`s that were here
+   *
+   * This component used to carry the definition (`objectSchema`) and "has it
+   * settled" (`schemaSettled`) as two SEPARATE pieces of state, the second a
+   * one-way latch that nothing ever reset. Two independent values cannot
+   * express "settled, but for a DIFFERENT object" — so on an object switch the
+   * gate below read `schemaSettled === true` left over from the PREVIOUS
+   * object's settle, while `objectSchema` still held the previous object's
+   * fields, and the query went out as
+   * `find(newObject, { $expand: [ …previous object's relation fields… ] })`:
+   * rejected or silently ignored depending on the adapter, plus the transient
+   * it painted, before a correct second query followed (objectui#6481).
+   *
+   * `useSettledSchema` holds ONE value, `{ key, def } | null`, and derives
+   * readiness during render by comparing the settled key against `schemaKey`.
+   * The gate therefore closes in the SAME commit that changes the object
+   * rather than one commit later — and "ready for the wrong object" is not
+   * merely fixed but unwritable, because there is no second piece of state
+   * left to disagree with the first.
+   *
+   * ## The settle-on-every-exit guarantee, preserved
+   *
+   * The `finally` that used to live here existed so the two early `return`s
+   * (no `dataSource` / no `getObjectSchema`; no object name) and a rejected
+   * read all settled too — otherwise the gated record query below waits
+   * forever, and a tree whose adapter serves no schema never renders a row.
+   * The hook makes that structural rather than incidental: each of those exits
+   * settles explicitly with `def: null`, which is a DISTINCT outcome from "not
+   * ready yet". Both halves are pinned in
+   * `ObjectTree.settledSchemaKeying-6481.test.tsx`.
+   *
+   * Gate PLACEMENT stays this component's own, per that same ruling: it sits
+   * INSIDE the object-provider branch of the record effect, not at the top of
+   * it, because the inline/static branches issue no metadata read and must not
+   * be made to wait on one.
+   */
+  const { ready: schemaSettled, def: objectSchema } = useSettledSchema<any>(
+    schemaKey,
+    dataSource,
+  );
 
   // Fetch records.
   useEffect(() => {