diff --git a/.changeset/field-selector-load-failure-5227.md b/.changeset/field-selector-load-failure-5227.md new file mode 100644 index 000000000..1a52e02a7 --- /dev/null +++ b/.changeset/field-selector-load-failure-5227.md @@ -0,0 +1,41 @@ +--- +'@object-ui/app-shell': patch +--- + +A failed field fetch in the metadata-admin `field-selector` picker now reads as a failure, not as "this object has no fields". + +`FieldSelectorWidget` was the fourth loader of the family objectui#5170 and +objectui#5169 closed, and the only one that does not go through +`MetadataClient`: a raw `fetch` to `/api/v1/objects/:name/fields`, its own +component-local `fields` / `loading` state, no `WidgetContext`. That is why the +`catalogErrors` channel added for the other pickers never reached it, and why it +kept the defect after they were fixed. + +It could reach a false empty two ways, and both are closed here: + +- the `catch` wrote `setFields([])` — the exact value a successful response with + no fields writes — and cleared the loading flag, so a dropped connection or an + expired session rendered as a completed, empty picker with a `console.error` + nobody reads; +- it never checked `res.ok`, so a 4xx/5xx whose body happens to parse as JSON + landed in the SUCCESS branch with `data.fields` undefined and `|| []` spelled + the refusal as an empty catalog. This mouth is the worse of the two: no error + was raised for the `catch` to swallow, so a union guarding only the `catch` + would have left it wide open. + +Both now leave through one door — a throw — and the loader is the four-arm +`LoadState` (`idle | loading | loaded | error`) the sibling pickers already use. +A failure renders the shared `PickerLoadFailure` block with the server's own +message, and the picker is replaced rather than decorated, so nothing on screen +can still be read as a measurement of zero. Whatever field is already stored +stays visible and removable: a failed catalog must not also block authoring. + +The "no fields" reading is deliberately kept for the case where it is true — a +load that COMPLETED and found nothing still renders the disabled picker, +unchanged and now reachable only from the `loaded` arm. No copy was added or +reworded. + +`usePickerLoad`, the shared loader hook, moves from `ResourceEditPage` into +`loadState` so this fourth loader reuses it instead of hand-rolling a fifth +union in a second file — which is exactly how this loader came to be missed. +Behaviour of the three existing callers is unchanged. diff --git a/packages/app-shell/src/views/metadata-admin/FieldSelectorWidget.loadFailure.test.tsx b/packages/app-shell/src/views/metadata-admin/FieldSelectorWidget.loadFailure.test.tsx new file mode 100644 index 000000000..21538d55f --- /dev/null +++ b/packages/app-shell/src/views/metadata-admin/FieldSelectorWidget.loadFailure.test.tsx @@ -0,0 +1,281 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * The FOURTH option-picker loader must tell a FAULT from a MEASUREMENT — + * objectui#5227. + * + * ## The defect, and why it had two mouths + * + * `FieldSelectorWidget` is the one loader in this family that does not go + * through `MetadataClient`: a raw `fetch` to `/api/v1/objects/:name/fields`, + * its own component-local `fields` / `loading` state, no `WidgetContext`. That + * is why objectui#5170 fixed the three `ResourceEditPage` pickers and left this + * one, and why the `catalogErrors` channel PR #5226 added never reached it. + * + * It could reach a false "no fields" two different ways: + * + * 1. the `catch` wrote `setFields([])` — the exact value a SUCCESSFUL empty + * response writes — and cleared the loading flag; + * 2. it never checked `res.ok`, so a non-ok response whose body happens to + * parse as JSON landed in the SUCCESS branch, where `data.fields || []` + * spelled the refusal as an empty catalog. No error was raised at all on + * this path, so a union guarding only mouth 1 would have left it open. + * + * Hence the triple is pinned PER MOUTH: a fix that only rewrote the `catch` + * passes the first failure case and fails the second. + * + * ## What is pinned + * + * 1. a FAILED load (network error) renders the failure state, NOT a picker; + * 2. a NON-OK response with a JSON body does the same — mouth 2; + * 3. a genuinely EMPTY SUCCESSFUL load still renders the picker, disabled. + * This is what stops (1) and (2) being tautologies: had the fix bought + * honesty by deleting the empty rendering, every "not an empty list" + * assertion would pass for the wrong reason; + * 4. the loading arm clears in every case, asserted in both directions — a + * held promise proves the arm is reachable, so "no longer loading" is not + * vacuous. + * + * ## Why these observables — measured, not assumed + * + * PR #5226 recorded that Radix `SelectValue` does not render its `placeholder` + * in jsdom. That measurement was taken on `field-ref`, and it does NOT carry + * here: `field-ref` holds `value={current || NO_FIELD}`, so an item always + * matches and its text wins over the placeholder. This widget holds `value=""` + * with no matching item, and the placeholder DOES render. Re-measured on this + * widget rather than inherited: + * + * completed, catalog empty → trigger reads "Add fields…" + * completed, catalog populated → trigger reads "Add fields…" (identical) + * single-select, catalog empty → trigger reads "Select field…" + * failed → no trigger at all + * + * So the placeholder cannot tell a MEASURED-empty catalog from a populated one + * — it is the same string — and an assertion on it would pass for both. What it + * can do is separate a completed load from a failed one, because the failure + * arm renders no picker; that direction is asserted below, in both polarities, + * alongside the structural reads: the failure block by its test id, the loading + * arm by its disabled input, the completed arms by the combobox they render. + */ + +import '@testing-library/jest-dom/vitest'; +import { describe, it, expect, vi, afterEach } from 'vitest'; +import { render, screen, cleanup, waitFor } from '@testing-library/react'; +import { WIDGETS } from './widgets'; +import { t } from './i18n'; + +const FieldSelector = WIDGETS['field-selector']; + +const OBJECT = 'showcase_account'; +const FIELDS_URL = `/api/v1/objects/${OBJECT}/fields`; + +/** Shown while the catalog request is genuinely in flight. */ +const LOADING_FIELDS = t('engine.form.loadingFields', 'en-US'); +/** The heading of the shared failure block (`PickerLoadFailure`). */ +const LOAD_FAILED_TITLE = t('engine.form.optionsLoadFailedTitle', 'en-US'); + +const FAILURE_TESTID = 'field-selector-load-failed'; +/** + * What the trigger reads on a COMPLETED load, measured (see the header). + * `multiple` is true for every case here except where stated, so this is the + * sentence a false empty used to show an operator. + */ +const ADD_FIELDS = t('engine.form.addFields', 'en-US'); + +function okResponse(body: unknown): Response { + return { ok: true, status: 200, json: async () => body } as unknown as Response; +} + +function jsonErrorResponse(status: number, body: unknown): Response { + return { ok: false, status, json: async () => body } as unknown as Response; +} + +/** A promise the test resolves by hand, so the loading arm is observable. */ +function deferred() { + let resolve!: (v: T) => void; + let reject!: (e: unknown) => void; + const promise = new Promise((res, rej) => { + resolve = res; + reject = rej; + }); + return { promise, resolve, reject }; +} + +function renderWidget(props: Record = {}) { + return render( + {}} + schema={{ type: 'string' }} + fieldSpec={{ field: 'fields', multiple: true }} + formData={{ objectName: OBJECT }} + {...props} + />, + ); +} + +afterEach(() => { + cleanup(); + vi.unstubAllGlobals(); + vi.restoreAllMocks(); +}); + +describe('field-selector — a failed field fetch is not an empty field list (objectui#5227)', () => { + it('mouth 1: a REJECTED fetch renders the failure block and no picker', async () => { + const fetchMock = vi.fn().mockRejectedValue(new Error('NetworkError: failed to fetch')); + vi.stubGlobal('fetch', fetchMock); + + renderWidget(); + + // The loading arm is reachable and then leaves — not vacuous, the widget + // starts in `loading` because an object IS bound on first paint. + expect(screen.getByDisplayValue(LOADING_FIELDS)).toBeDisabled(); + + await waitFor(() => expect(screen.getByTestId(FAILURE_TESTID)).toBeInTheDocument()); + expect(screen.getByText(LOAD_FAILED_TITLE)).toBeInTheDocument(); + // The cause reaches the operator instead of only `console.error`. + expect(screen.getByTestId(`${FAILURE_TESTID}-cause`)).toHaveTextContent( + 'NetworkError: failed to fetch', + ); + + // NOT an empty field list: the picker is absent, so neither the trigger + // nor its copy is on screen to be read as "this object has no fields". + expect(screen.queryByRole('combobox')).not.toBeInTheDocument(); + expect(screen.queryByText(ADD_FIELDS)).not.toBeInTheDocument(); + // …and the loading flag cleared. + expect(screen.queryByDisplayValue(LOADING_FIELDS)).not.toBeInTheDocument(); + + expect(fetchMock).toHaveBeenCalledWith(FIELDS_URL); + }); + + it('mouth 2: a NON-OK response with a parseable JSON body is a fault, not an empty catalog', async () => { + // The shape that used to land in the SUCCESS branch: `res.json()` resolves, + // `data.fields` is undefined, and `|| []` rendered the refusal as "no fields". + const fetchMock = vi + .fn() + .mockResolvedValue(jsonErrorResponse(403, { error: { message: 'Insufficient permissions' } })); + vi.stubGlobal('fetch', fetchMock); + + renderWidget(); + + await waitFor(() => expect(screen.getByTestId(FAILURE_TESTID)).toBeInTheDocument()); + expect(screen.queryByRole('combobox')).not.toBeInTheDocument(); + expect(screen.queryByText(ADD_FIELDS)).not.toBeInTheDocument(); + expect(screen.queryByDisplayValue(LOADING_FIELDS)).not.toBeInTheDocument(); + // The server's own sentence, not a generic one. + expect(screen.getByTestId(`${FAILURE_TESTID}-cause`)).toHaveTextContent( + 'Insufficient permissions', + ); + }); + + it('mouth 2: a NON-OK response with no usable message still fails with its status', async () => { + const fetchMock = vi.fn().mockResolvedValue({ + ok: false, + status: 500, + // A non-JSON body — the parse throws, and that must not be mistaken for + // a refusal that said nothing to report. + json: async () => { + throw new SyntaxError('Unexpected token < in JSON'); + }, + } as unknown as Response); + vi.stubGlobal('fetch', fetchMock); + + renderWidget(); + + await waitFor(() => expect(screen.getByTestId(FAILURE_TESTID)).toBeInTheDocument()); + expect(screen.queryByRole('combobox')).not.toBeInTheDocument(); + expect(screen.getByTestId(`${FAILURE_TESTID}-cause`)).toHaveTextContent('HTTP 500'); + }); + + it('a genuinely EMPTY successful load still renders the picker, and no failure block', async () => { + const fetchMock = vi.fn().mockResolvedValue(okResponse({ fields: [] })); + vi.stubGlobal('fetch', fetchMock); + + renderWidget(); + + // This arm is what stops the assertions above being tautologies: the empty + // measurement is still rendered, it is just no longer where a fault lands. + await waitFor(() => expect(screen.getByRole('combobox')).toBeInTheDocument()); + expect(screen.getByRole('combobox')).toBeDisabled(); + // The measurement is still spelled out — the fix bought honesty about the + // fault without deleting the case where "nothing here" is true. + expect(screen.getByText(ADD_FIELDS)).toBeInTheDocument(); + expect(screen.queryByTestId(FAILURE_TESTID)).not.toBeInTheDocument(); + expect(screen.queryByDisplayValue(LOADING_FIELDS)).not.toBeInTheDocument(); + }); + + it('a POPULATED successful load renders an enabled picker', async () => { + const fetchMock = vi.fn().mockResolvedValue( + okResponse({ fields: [{ name: 'name', label: 'Name', type: 'text' }] }), + ); + vi.stubGlobal('fetch', fetchMock); + + renderWidget(); + + await waitFor(() => expect(screen.getByRole('combobox')).toBeEnabled()); + expect(screen.queryByTestId(FAILURE_TESTID)).not.toBeInTheDocument(); + }); + + it('the loading arm is held, then clears — asserted in both directions', async () => { + const gate = deferred(); + const fetchMock = vi.fn().mockReturnValue(gate.promise); + vi.stubGlobal('fetch', fetchMock); + + renderWidget(); + + // Held: still asking, so neither an answer nor a failure is on screen. + expect(screen.getByDisplayValue(LOADING_FIELDS)).toBeDisabled(); + expect(screen.queryByRole('combobox')).not.toBeInTheDocument(); + expect(screen.queryByTestId(FAILURE_TESTID)).not.toBeInTheDocument(); + + gate.resolve(okResponse({ fields: [{ name: 'name', label: 'Name', type: 'text' }] })); + + await waitFor(() => expect(screen.getByRole('combobox')).toBeInTheDocument()); + expect(screen.queryByDisplayValue(LOADING_FIELDS)).not.toBeInTheDocument(); + }); + + it('the loading arm clears into the failure arm too (held, then rejected)', async () => { + const gate = deferred(); + const fetchMock = vi.fn().mockReturnValue(gate.promise); + vi.stubGlobal('fetch', fetchMock); + + renderWidget(); + + expect(screen.getByDisplayValue(LOADING_FIELDS)).toBeDisabled(); + + gate.reject(new Error('connection reset')); + + await waitFor(() => expect(screen.getByTestId(FAILURE_TESTID)).toBeInTheDocument()); + expect(screen.queryByDisplayValue(LOADING_FIELDS)).not.toBeInTheDocument(); + }); + + it('no object bound is IDLE, not a failure — nothing is fetched', () => { + const fetchMock = vi.fn(); + vi.stubGlobal('fetch', fetchMock); + + renderWidget({ formData: {} }); + + // A question never asked must not render as an answer of none, nor as a + // fault: the widget asks for an object first and stays silent. + expect( + screen.getByDisplayValue(t('engine.form.selectObjectFirst', 'en-US')), + ).toBeDisabled(); + expect(screen.queryByTestId(FAILURE_TESTID)).not.toBeInTheDocument(); + expect(fetchMock).not.toHaveBeenCalled(); + }); + + it('a failed catalog does not also block authoring — the stored value stays visible and removable', async () => { + const onChange = vi.fn(); + vi.stubGlobal('fetch', vi.fn().mockRejectedValue(new Error('offline'))); + + renderWidget({ value: ['amount'], onChange }); + + await waitFor(() => expect(screen.getByTestId(FAILURE_TESTID)).toBeInTheDocument()); + // The chip for the already-stored field is still there… + expect(screen.getByText('amount')).toBeInTheDocument(); + // …and still editable. + screen.getByRole('button', { name: '×' }).click(); + expect(onChange).toHaveBeenCalledWith([]); + }); +}); diff --git a/packages/app-shell/src/views/metadata-admin/ResourceEditPage.tsx b/packages/app-shell/src/views/metadata-admin/ResourceEditPage.tsx index 349b44a70..0706a7062 100644 --- a/packages/app-shell/src/views/metadata-admin/ResourceEditPage.tsx +++ b/packages/app-shell/src/views/metadata-admin/ResourceEditPage.tsx @@ -95,11 +95,10 @@ import { } from './SchemaForm'; import { collectPageComponentIds, type CatalogErrors } from './widgets'; import { - type LoadState, - loadErrorMessage, loadErrorOf, loadedData, isLoading, + usePickerLoad, } from './loadState'; import { useMetadataClient, @@ -294,65 +293,6 @@ const EMPTY_OBJECT_NAMES: string[] = []; const EMPTY_OBJECT_VIEWS: Array<{ name: string; label?: string }> = []; const EMPTY_OBJECT_CATALOG: ObjectCatalog = { fields: [], actions: [] }; -/** - * The one loader the three option-picker catalogs share (objectui#5170). - * - * All three used to hand-roll the same effect, and all three hand-rolled the - * same bug in it: the `catch` wrote the empty array — the value a *successful* - * response with nothing in it writes — and the `finally` flipped loading to - * false. The picker then rendered a completed, empty list, and an operator - * authoring a view, a permission row or an action read that as the metadata - * graph's answer ("this object has no fields") and went and created one. - * `client.list()` / `client.get()` throw for every non-ok status other than the - * 404s they map to an empty result, so refusals, dropped connections, expired - * sessions and unparseable bodies all took that path. - * - * Sharing one hook is what makes that unrepeatable: the catch is written once, - * and it can only produce the `error` arm. Three copy-pasted unions in one file - * is how the next drift starts. The three loaders differ only in what they - * fetch and in whether they are gated on a bound source object, and both - * differences fit through the argument: - * - * • `load` is the request, memoised by the caller — its identity is the - * dependency, so the caller keeps its own explicit dep list and this hook - * needs no dep-array passthrough. - * • `load === null` means "not applicable" (no source object is bound). That - * is the `idle` arm: a question never asked, which is NOT a failure and - * must not render as one. - * - * The initial state follows `load` rather than defaulting to `idle`, so an - * enabled loader is already `loading` on first paint. Starting at `idle` would - * flash one frame of "not loading, zero results" before the effect runs — the - * exact false reading this card is about, just briefly. - */ -function usePickerLoad(load: (() => Promise) | null): LoadState { - const [state, setState] = React.useState>(() => - load ? { status: 'loading' } : { status: 'idle' }, - ); - React.useEffect(() => { - if (!load) { - setState({ status: 'idle' }); - return; - } - let cancelled = false; - setState({ status: 'loading' }); - void (async () => { - try { - const data = await load(); - if (!cancelled) setState({ status: 'loaded', data }); - } catch (err) { - // NOT `{ status: 'loaded', data: [] }`. An unanswered question is not - // an answer of "nothing" — that substitution is the entire defect. - if (!cancelled) setState({ status: 'error', message: loadErrorMessage(err) }); - } - })(); - return () => { - cancelled = true; - }; - }, [load]); - return state; -} - interface MetadataResourceEditPageImplProps { type: string; name: string; diff --git a/packages/app-shell/src/views/metadata-admin/loadState.ts b/packages/app-shell/src/views/metadata-admin/loadState.ts index 123d19ea8..936f3b2ae 100644 --- a/packages/app-shell/src/views/metadata-admin/loadState.ts +++ b/packages/app-shell/src/views/metadata-admin/loadState.ts @@ -1,5 +1,7 @@ // Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. +import * as React from 'react'; + /** * `LoadState` — the one shape the metadata-admin loaders use to keep a * FAULT and a MEASUREMENT distinguishable (objectui#5170, objectui#5169). @@ -91,3 +93,71 @@ export function isLoading(state: LoadState): boolean { export function loadErrorOf(state: LoadState): string | undefined { return state.status === 'error' ? state.message : undefined; } + +/** + * The one loader every metadata-admin option-picker catalog shares + * (objectui#5170, objectui#5227). + * + * Each catalog used to hand-roll the same effect, and each hand-rolled the same + * bug in it: the `catch` wrote the empty array — the value a *successful* + * response with nothing in it writes — and the `finally` flipped loading to + * false. The picker then rendered a completed, empty list, and an operator + * authoring a view, a permission row or an action read that as the metadata + * graph's answer ("this object has no fields") and went and created one. + * + * Sharing one hook is what makes that unrepeatable: the catch is written once, + * and it can only produce the `error` arm. Copy-pasted unions are how the next + * drift starts — which is exactly what happened: `FieldSelectorWidget` in + * `widgets.tsx` was a fourth loader with the same swallow, missed by + * objectui#5170 because it does not go through `MetadataClient` at all. It now + * calls this hook too, so the hook lives here rather than private to + * `ResourceEditPage` (`widgets.tsx` cannot import that module — `ResourceEditPage` + * imports `widgets`). + * + * The loaders differ only in what they fetch and in whether they are gated on a + * bound source object, and both differences fit through the argument: + * + * • `load` is the request, memoised by the caller — its identity is the + * dependency, so the caller keeps its own explicit dep list and this hook + * needs no dep-array passthrough. + * • `load === null` means "not applicable" (no source object is bound). That + * is the `idle` arm: a question never asked, which is NOT a failure and + * must not render as one. + * + * The initial state follows `load` rather than defaulting to `idle`, so an + * enabled loader is already `loading` on first paint. Starting at `idle` would + * flash one frame of "not loading, zero results" before the effect runs — the + * exact false reading this card is about, just briefly. + * + * The `cancelled` guard is load-bearing, not hygiene: once a failure has its + * own arm, a late response for a PREVIOUS question can no longer only stale a + * list — it can post a failure banner over a catalog that loaded fine, or hide + * a real one behind a stale success. + */ +export function usePickerLoad(load: (() => Promise) | null): LoadState { + const [state, setState] = React.useState>(() => + load ? { status: 'loading' } : { status: 'idle' }, + ); + React.useEffect(() => { + if (!load) { + setState({ status: 'idle' }); + return; + } + let cancelled = false; + setState({ status: 'loading' }); + void (async () => { + try { + const data = await load(); + if (!cancelled) setState({ status: 'loaded', data }); + } catch (err) { + // NOT `{ status: 'loaded', data: [] }`. An unanswered question is not + // an answer of "nothing" — that substitution is the entire defect. + if (!cancelled) setState({ status: 'error', message: loadErrorMessage(err) }); + } + })(); + return () => { + cancelled = true; + }; + }, [load]); + return state; +} diff --git a/packages/app-shell/src/views/metadata-admin/selector-placeholder.i18n.test.tsx b/packages/app-shell/src/views/metadata-admin/selector-placeholder.i18n.test.tsx index 8569437ca..f076f45bc 100644 --- a/packages/app-shell/src/views/metadata-admin/selector-placeholder.i18n.test.tsx +++ b/packages/app-shell/src/views/metadata-admin/selector-placeholder.i18n.test.tsx @@ -45,10 +45,17 @@ describe('metadata-admin selector placeholders (objectui#4387 key collapse)', () it('the single-select field picker renders the surviving key value', async () => { // The picker only reaches its Select once a bound object's fields have // loaded — before that it is the "(Select an object first)" / "Loading - // fields…" input, neither of which carries the placeholder under test. + // fields…" input, neither of which carries the placeholder under test, and + // since objectui#5227 a load that FAILED renders the failure block instead. + // Hence `ok` / `status`: this stub stood in for a successful response but + // spelled neither, which was only expressible while the loader ignored + // `res.ok` — the second mouth of the defect #5227 closed. A real `Response` + // always carries both. vi.stubGlobal( 'fetch', vi.fn().mockResolvedValue({ + ok: true, + status: 200, json: async () => ({ fields: [{ name: 'name', label: 'Name', type: 'text' }] }), }), ); diff --git a/packages/app-shell/src/views/metadata-admin/widgets.tsx b/packages/app-shell/src/views/metadata-admin/widgets.tsx index 8664b6db9..8e1f4c09d 100644 --- a/packages/app-shell/src/views/metadata-admin/widgets.tsx +++ b/packages/app-shell/src/views/metadata-admin/widgets.tsx @@ -51,6 +51,7 @@ import { foldFilterGroupToSpecRules, FILTER_FOLD_REFUSAL_KEYS } from '../viewFil import { ColorVariantPicker } from './color-variant-field'; import { ConditionBuilder } from './inspectors/ConditionBuilder'; import { expressionSource, writeExpressionSource } from './inspectors/expression-envelope'; +import { isLoading, loadErrorOf, loadedData, usePickerLoad } from './loadState'; /** * Load failures for the option catalogs on {@link WidgetContext}, by catalog @@ -560,6 +561,63 @@ function ObjectSelectorWidget({ /* field-selector — smart field picker (depends on selected object) */ /* -------------------------------------------------------------------------- */ +/** One entry of the `field-selector` catalog, as the REST endpoint spells it. */ +interface FieldSelectorOption { + name: string; + label: string; + type: string; +} + +/** Stable identity for the non-`loaded` arms, so no consumer re-renders on a fresh `[]`. */ +const EMPTY_FIELD_SELECTOR_OPTIONS: FieldSelectorOption[] = []; + +/** + * Fetch the bound object's field catalog for {@link FieldSelectorWidget} + * (objectui#5227). + * + * This is the one loader in this family that does NOT go through + * `MetadataClient` — it is a raw `fetch` to a REST path no other widget here + * uses — which is why objectui#5170 missed it and why the `catalogErrors` + * channel on {@link WidgetContext} does not reach it. It had TWO ways to render + * a fault as a measurement, and a union that guarded only the first would have + * left the second wide open: + * + * 1. the `catch` wrote `setFields([])` — byte-identical to a successful + * response with no fields — and cleared the loading flag, so a dropped + * connection rendered as a completed, empty picker; + * 2. it never checked `res.ok`, so a 4xx/5xx whose body happens to parse as + * JSON landed in the SUCCESS branch with `data.fields` undefined, and the + * old `|| []` rendered that refusal as "no fields" — worse than (1), + * because no error was ever raised for the `catch` to swallow. + * + * Both now leave through the same door: a throw, which {@link usePickerLoad} + * can only turn into the `error` arm. The message follows the convention the + * other raw-`fetch` callers in this package already use (`useRecordApprovals`, + * `suggestedBindingsApi`, `studio-design/packages-io`): the server's own + * message when it sent one, otherwise the status. + */ +async function fetchFieldSelectorOptions(objectName: string): Promise { + const res = await fetch(`/api/v1/objects/${objectName}/fields`); + // Read the body BEFORE branching on `ok`: a refusal usually carries the more + // useful sentence, and a non-JSON body must not be mistaken for a refusal + // that said nothing (it still throws below — it just cannot add a cause). + let payload: any = null; + try { + payload = await res.json(); + } catch { + /* empty or non-JSON body */ + } + if (!res.ok) { + const detail = payload?.error?.message ?? payload?.error ?? payload?.message; + throw new Error( + typeof detail === 'string' && detail ? detail : `HTTP ${res.status}`, + ); + } + // `payload.fields` narrows to the list; anything else is an answer we cannot + // read, and the completed-load arm is the only one that reaches here. + return Array.isArray(payload?.fields) ? (payload.fields as FieldSelectorOption[]) : []; +} + function FieldSelectorWidget({ id, value, @@ -569,37 +627,26 @@ function FieldSelectorWidget({ formData, }: WidgetProps) { const locale = useMetadataLocale(); - const [fields, setFields] = React.useState>([]); - const [loading, setLoading] = React.useState(false); - + // Resolve dependency: fieldSpec.dependsOn or fieldSpec.reference or 'objectName' const dependsOnRaw = fieldSpec?.dependsOn || fieldSpec?.reference || 'objectName'; const dependsOnField = Array.isArray(dependsOnRaw) ? dependsOnRaw[0] : dependsOnRaw; const objectName = formData?.[dependsOnField] as string | undefined; - // Load fields when objectName changes - React.useEffect(() => { - if (!objectName) { - setFields([]); - return; - } - - setLoading(true); - fetch(`/api/v1/objects/${objectName}/fields`) - .then(r => r.json()) - .then(data => { - setFields(data.fields || []); - setLoading(false); - }) - .catch(err => { - console.error('Failed to load fields:', err); - setFields([]); - setLoading(false); - }); - }, [objectName]); + // Four structurally distinct arms, one per `LoadState` — the shape this + // directory landed for the other pickers (objectui#5170) and for the + // References panel before them (objectui#5110). `null` while no object is + // bound is the `idle` arm: a question never asked is not a failure. + const loadFields = React.useMemo( + () => (objectName ? () => fetchFieldSelectorOptions(objectName) : null), + [objectName], + ); + const fieldsState = usePickerLoad(loadFields); + const fields = loadedData(fieldsState, EMPTY_FIELD_SELECTOR_OPTIONS); + const loadError = loadErrorOf(fieldsState); const multiple = fieldSpec?.multiple ?? false; - + // Parse value const selectedValues = React.useMemo(() => { if (!value) return []; @@ -609,7 +656,7 @@ function FieldSelectorWidget({ const handleToggle = (fieldName: string) => { if (readOnly) return; - + if (!multiple) { onChange(fieldName); return; @@ -618,7 +665,7 @@ function FieldSelectorWidget({ const newSelection = selectedValues.includes(fieldName) ? selectedValues.filter(v => v !== fieldName) : [...selectedValues, fieldName]; - + onChange(newSelection); }; @@ -632,40 +679,60 @@ function FieldSelectorWidget({ return ; } - if (loading) { + if (isLoading(fieldsState)) { return ; } + /* Whatever is already stored, kept visible and removable in EVERY completed + arm — a failed catalog must not also block authoring. */ + const selectedChips = selectedValues.length > 0 && ( +
+ {selectedValues.map(field => { + const fieldMeta = fields.find(f => f.name === field); + return ( +
+ {fieldMeta?.label || field} + {fieldMeta?.type} + {!readOnly && ( + + )} +
+ ); + })} +
+ ); + + // The catalog FAILED to load — not the same fact as "this object has no + // fields". The picker below is REPLACED rather than decorated (the shape + // `field-ref` uses): with no options it could only render as a dead, + // disabled dropdown next to a banner saying the options are unknown, which + // is the very conflation this arm exists to end. + if (loadError) { + return ( +
+ {selectedChips} + +
+ ); + } + return (
{/* Selected fields */} - {selectedValues.length > 0 && ( -
- {selectedValues.map(field => { - const fieldMeta = fields.find(f => f.name === field); - return ( -
- {fieldMeta?.label || field} - {fieldMeta?.type} - {!readOnly && ( - - )} -
- ); - })} -
- )} + {selectedChips} - {/* Field picker */} + {/* Field picker. `fields.length === 0` here means a load that COMPLETED + and found nothing — the disabled trigger is that measurement, and it + is now reachable only from the `loaded` arm. */}