From 79424e1bfa17bb800b10056099fce8314eeb9964 Mon Sep 17 00:00:00 2001 From: os-support-ai Date: Wed, 19 Aug 2026 20:22:45 +0000 Subject: [PATCH] fix(app-shell): a WidgetContext option catalog carries its own load state (#5228) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Each option catalog on `WidgetContext` (`objectNames`, `objectFields`, `objectViews`, `objectActions`) becomes the four-arm `LoadState` the loaders already produce, instead of a plain array with the fault parked on a separate `catalogErrors` record and a `*Loading` flag beside it. The old shape let a FAILED load reach a picker as `[]`, byte-identical to a load that completed and found nothing, with the requirement to consult the failure channel living in a doc comment. `context?.objectFields ?? []` no longer compiles, and reading the list goes through `offeredOptions`, whose parameter type excludes the failure arm — so a call site that has not decided what a failure looks like does not compile. Fixes one live instance found by the compiler: the View variant inspector, a second host of `WidgetContext`, forwarded only the `fields` third of what its loader knows, so a failed field catalog rendered as "No object bound". --- .../widget-context-catalog-load-state-5228.md | 42 ++ .../metadata-admin/FilterModeWidget.test.tsx | 13 +- .../views/metadata-admin/ResourceEditPage.tsx | 77 ++-- .../SchemaForm.actionObjectPicker.test.tsx | 7 +- .../src/views/metadata-admin/SchemaForm.tsx | 7 + .../SchemaForm.widgetLabelling.test.tsx | 19 +- .../metadata-admin/ViewRefWidget.test.tsx | 9 +- .../WidgetContext.catalogUnion.test.tsx | 388 ++++++++++++++++++ .../inspectors/ViewVariantInspector.tsx | 56 ++- .../src/views/metadata-admin/loadState.ts | 101 ++++- .../selector-placeholder.i18n.test.tsx | 3 +- .../src/views/metadata-admin/widgets.tsx | 365 +++++++++------- 12 files changed, 864 insertions(+), 223 deletions(-) create mode 100644 .changeset/widget-context-catalog-load-state-5228.md create mode 100644 packages/app-shell/src/views/metadata-admin/WidgetContext.catalogUnion.test.tsx diff --git a/.changeset/widget-context-catalog-load-state-5228.md b/.changeset/widget-context-catalog-load-state-5228.md new file mode 100644 index 0000000000..5fee5b79bb --- /dev/null +++ b/.changeset/widget-context-catalog-load-state-5228.md @@ -0,0 +1,42 @@ +--- +'@object-ui/app-shell': patch +--- + +A metadata-admin option catalog now carries its own load state, so a picker cannot render a failed catalog as an empty one. + +`WidgetContext` spelled each option catalog (`objectNames`, `objectFields`, +`objectViews`, `objectActions`) as a plain array, with the fault travelling +alongside on a separate `catalogErrors` record and a `*Loading` flag beside +that. A FAILED load therefore arrived at the pickers as `[]` — byte-identical +to a load that completed and found nothing — and the rule that a picker must +consult the failure channel first lived in `CatalogErrors`' own doc comment. One +line was enough to ignore it: + +```ts +const fields = context?.objectFields ?? []; +``` + +That is type-correct, reads naturally, and renders a refusal, a dropped +connection or an expired session as the metadata graph's own answer of "this +object has no fields" — the defect objectui#5170 and objectui#5169 were filed +for, reintroduced at the boundary their fix stopped at. + +Each catalog is now the four-arm `LoadState` (`idle | loading | loaded | error`) +the loaders already produce, handed over intact instead of projected back down +into a pair. The naive read stops compiling, and reading the list at all goes +through an accessor whose parameter type excludes the failure arm — so a call +site that has not decided what a failure looks like does not compile, at exactly +the sites that must decide. Nothing widens: the set of authored metadata this +renderer accepts does not move. + +One real fault is fixed on the way, and it is why the tightening was worth more +than the churn: the View variant inspector is a second host of `WidgetContext`, +and it forwarded only the `fields` third of what its loader knows. A failed field +catalog reached its `field-ref` / `field-multi` pickers as an empty list and the +picker said "No object bound" about an object that is bound and whose catalog +simply could not be fetched. It now renders the same shared failure block the +other pickers use, with the server's own message. + +Behaviour is otherwise unchanged: the loading arms, the empty-state copy for a +load that genuinely found nothing, and every already-stored value staying +visible and editable on a failure all render exactly as before. diff --git a/packages/app-shell/src/views/metadata-admin/FilterModeWidget.test.tsx b/packages/app-shell/src/views/metadata-admin/FilterModeWidget.test.tsx index db1c99ab27..3c93aa6c8a 100644 --- a/packages/app-shell/src/views/metadata-admin/FilterModeWidget.test.tsx +++ b/packages/app-shell/src/views/metadata-admin/FilterModeWidget.test.tsx @@ -2,17 +2,22 @@ import { describe, it, expect, vi, afterEach } from 'vitest'; import { render, screen, fireEvent, cleanup } from '@testing-library/react'; -import { WIDGETS } from './widgets'; +import { WIDGETS, type WidgetContext } from './widgets'; +import { loaded } from './loadState'; afterEach(cleanup); const FilterMode = WIDGETS['filter-mode']; -const ctx = { - objectFields: [ +// A catalog that LOADED and holds three fields. `loaded(...)` rather than the +// bare array since objectui#5228: the catalog is a `LoadState`, so a fixture has +// to say which arm it is standing in — an empty list and a failed load are no +// longer the same value. +const ctx: WidgetContext = { + objectFields: loaded([ { name: 'status', label: 'Status' }, { name: 'priority', label: 'Priority' }, { name: 'owner', label: 'Owner' }, - ], + ]), }; /** diff --git a/packages/app-shell/src/views/metadata-admin/ResourceEditPage.tsx b/packages/app-shell/src/views/metadata-admin/ResourceEditPage.tsx index 0706a70622..f53084bb3a 100644 --- a/packages/app-shell/src/views/metadata-admin/ResourceEditPage.tsx +++ b/packages/app-shell/src/views/metadata-admin/ResourceEditPage.tsx @@ -93,13 +93,13 @@ import { DRAWER_METADATA_ID_SCOPE, type SchemaFormIssue, } from './SchemaForm'; -import { collectPageComponentIds, type CatalogErrors } from './widgets'; import { - loadErrorOf, - loadedData, - isLoading, - usePickerLoad, -} from './loadState'; + collectPageComponentIds, + type ObjectActionOption, + type ObjectFieldOption, + type WidgetContext, +} from './widgets'; +import { mapLoaded, usePickerLoad } from './loadState'; import { useMetadataClient, useMetadataTypes, @@ -282,17 +282,10 @@ type ReferencesState = /** The two catalogs the single `client.get('object', …)` call yields. */ type ObjectCatalog = { - fields: Array<{ name: string; label?: string; type?: string }>; - actions: Array<{ name: string; label?: string; locations?: string[] }>; + fields: ObjectFieldOption[]; + actions: ObjectActionOption[]; }; -// Module-level empties so the derived catalogs keep a stable identity across -// renders — `widgetContext` memoises on them, and a fresh `[]` per render would -// defeat that for every consumer downstream. -const EMPTY_OBJECT_NAMES: string[] = []; -const EMPTY_OBJECT_VIEWS: Array<{ name: string; label?: string }> = []; -const EMPTY_OBJECT_CATALOG: ObjectCatalog = { fields: [], actions: [] }; - interface MetadataResourceEditPageImplProps { type: string; name: string; @@ -659,8 +652,6 @@ function MetadataResourceEditPageImpl({ return list.map((x) => x?.name).filter((n): n is string => !!n).sort(); }, [client]), ); - const objectNames = loadedData(objectsState, EMPTY_OBJECT_NAMES); - const objectsLoading = isLoading(objectsState); // Field catalog of the draft's bound/source object — fuels field-picker // widgets (e.g. the interface-page filter-mode selector). For a page the // source is `interfaceConfig.source` (interface mode) or the bound @@ -695,9 +686,6 @@ function MetadataResourceEditPageImpl({ [client, sourceObjectName], ), ); - const objectFields = loadedData(objectCatalogState, EMPTY_OBJECT_CATALOG).fields; - const objectActions = loadedData(objectCatalogState, EMPTY_OBJECT_CATALOG).actions; - const objectFieldsLoading = isLoading(objectCatalogState); // View catalog of the source object — fuels the `view-ref` picker for // `interfaceConfig.sourceView` so the author chooses an existing view @@ -723,8 +711,6 @@ function MetadataResourceEditPageImpl({ [client, sourceObjectName], ), ); - const objectViews = loadedData(objectViewsState, EMPTY_OBJECT_VIEWS); - const objectViewsLoading = isLoading(objectViewsState); // Component ids placed on the page being edited — fuels the `ref:component` // picker so a page variable's `source` (the component that writes it) is @@ -736,30 +722,31 @@ function MetadataResourceEditPageImpl({ [type, draft], ); - // `catalogErrors` is the failure arm of the three loaders above, carried to - // the pickers (objectui#5170). A key is present ONLY when that catalog's load - // FAILED — never for a catalog that completed and found nothing, and never for - // one that was never asked (no source object bound). The catalog arrays stay - // empty on failure, which is exactly why the pickers must consult this first: - // an empty array can no longer be read as "the answer is none" without also - // checking whether the question was answered at all. + // Each loader's `LoadState` reaches the pickers WHOLE (objectui#5228). + // + // This used to project each one down into a pair — the catalog array plus a + // `*Loading` flag — with the failure arm sent alongside on a separate + // `catalogErrors` record. That projection is what let a failed load arrive as + // `[]`, byte-identical to a load that completed and found nothing, with the + // rule that a picker must consult the side channel first living in a doc + // comment. Handing the union over intact deletes both the projection and the + // rule: a consumer cannot reach the list without the compiler having seen it + // decide what a failure renders as. // - // `fields` covers `objectActions` too — both come from the single - // `client.get('object', …)` call, so there is one failure, not two. - const catalogErrors = React.useMemo(() => { - const errors: CatalogErrors = {}; - const objects = loadErrorOf(objectsState); - const fields = loadErrorOf(objectCatalogState); - const views = loadErrorOf(objectViewsState); - if (objects) errors.objects = objects; - if (fields) errors.fields = fields; - if (views) errors.views = views; - return errors; - }, [objectsState, objectCatalogState, objectViewsState]); - - const widgetContext = React.useMemo( - () => ({ objectNames, objectsLoading, objectFields, objectFieldsLoading, objectViews, objectViewsLoading, objectActions, componentIds, catalogErrors }), - [objectNames, objectsLoading, objectFields, objectFieldsLoading, objectViews, objectViewsLoading, objectActions, componentIds, catalogErrors], + // `objectFields` and `objectActions` are DERIVED FROM ONE STATE rather than + // loaded twice, because they come from one `client.get('object', …)` request: + // they succeed together and fail together, and `mapLoaded` is what makes that + // true by construction instead of by two independent states happening to + // agree. + const widgetContext = React.useMemo( + () => ({ + objectNames: objectsState, + objectFields: mapLoaded(objectCatalogState, (catalog) => catalog.fields), + objectActions: mapLoaded(objectCatalogState, (catalog) => catalog.actions), + objectViews: objectViewsState, + componentIds, + }), + [objectsState, objectCatalogState, objectViewsState, componentIds], ); // Load layered view + initial draft. diff --git a/packages/app-shell/src/views/metadata-admin/SchemaForm.actionObjectPicker.test.tsx b/packages/app-shell/src/views/metadata-admin/SchemaForm.actionObjectPicker.test.tsx index 433f239bfa..10be498403 100644 --- a/packages/app-shell/src/views/metadata-admin/SchemaForm.actionObjectPicker.test.tsx +++ b/packages/app-shell/src/views/metadata-admin/SchemaForm.actionObjectPicker.test.tsx @@ -3,6 +3,7 @@ import { describe, it, expect, afterEach } from 'vitest'; import { render, cleanup } from '@testing-library/react'; import { SchemaForm } from './SchemaForm'; +import { loaded } from './loadState'; import { registerBuiltinAnchors } from './anchors'; import { resolveResourceConfig } from './registry'; @@ -34,8 +35,10 @@ describe('SchemaForm — action objectName renders as an object selector (#2325) createMode onChange={() => {}} widgetContext={{ - objectNames: ['showcase_task', 'showcase_account'], - objectsLoading: false, + // objectui#5228: the object catalog carries its own load state, so + // the separate `objectsLoading: false` this fixture used to set is + // now expressed by the arm itself — a load that COMPLETED. + objectNames: loaded(['showcase_task', 'showcase_account']), }} />, ); diff --git a/packages/app-shell/src/views/metadata-admin/SchemaForm.tsx b/packages/app-shell/src/views/metadata-admin/SchemaForm.tsx index a9031e8165..0aa2ad479f 100644 --- a/packages/app-shell/src/views/metadata-admin/SchemaForm.tsx +++ b/packages/app-shell/src/views/metadata-admin/SchemaForm.tsx @@ -297,6 +297,13 @@ function detectFieldRefWidget( schema: JsonSchema | undefined, widgetContext?: WidgetContext, ): string | undefined { + // Tests WIRING, not contents: is a field catalog plumbed to this form at all? + // Deliberately NOT a read of the catalog — under objectui#5228's union every + // arm (`idle` / `loading` / `loaded` / `error`) answers yes, and it must, or a + // FAILED catalog would silently demote the picker back to the free-text input + // whose typos the picker exists to prevent. The picker itself renders the + // failure. (Before the union this line read the same way for a different + // reason: the array was `[]` on failure, and `[]` is truthy.) if (!widgetContext?.objectFields) return undefined; if (Array.isArray(schema?.enum)) return undefined; diff --git a/packages/app-shell/src/views/metadata-admin/SchemaForm.widgetLabelling.test.tsx b/packages/app-shell/src/views/metadata-admin/SchemaForm.widgetLabelling.test.tsx index 0b5cddabdd..7c1fc00c1e 100644 --- a/packages/app-shell/src/views/metadata-admin/SchemaForm.widgetLabelling.test.tsx +++ b/packages/app-shell/src/views/metadata-admin/SchemaForm.widgetLabelling.test.tsx @@ -63,6 +63,7 @@ import { type RegisteredWidgetKey, type WidgetContext, } from './widgets'; +import { loaded } from './loadState'; afterEach(cleanup); @@ -114,19 +115,19 @@ function renderCase(c: Case, readOnly: boolean) { * added without a probe fails here as well as at the declaration. */ const CASES: Case[] = [ - { key: 'ref:object', schema: { type: 'string', title: TITLE }, ctx: { objectNames: ['account'] }, value: 'account' }, + { key: 'ref:object', schema: { type: 'string', title: TITLE }, ctx: { objectNames: loaded(['account']) }, value: 'account' }, { key: 'ref:object', variant: 'no-objects', schema: { type: 'string', title: TITLE }, ctx: {}, value: 'x' }, { key: 'ref:component', schema: { type: 'string', title: TITLE }, ctx: { componentIds: [{ id: 'c1' }] }, value: 'c1' }, { key: 'ref:component', variant: 'no-components', schema: { type: 'string', title: TITLE }, ctx: {}, value: 'c1' }, - { key: 'filter-mode', schema: { type: 'object', title: TITLE }, ctx: { objectFields: [{ name: 'status' }] }, value: { element: 'dropdown' } }, - { key: 'object-selector', schema: { type: 'string', title: TITLE }, ctx: { objectNames: ['account'] }, value: 'account' }, - { key: 'object-selector', variant: 'multiple', schema: { type: 'array', title: TITLE }, spec: { multiple: true }, ctx: { objectNames: ['account', 'contact'] }, value: ['account'] }, + { key: 'filter-mode', schema: { type: 'object', title: TITLE }, ctx: { objectFields: loaded([{ name: 'status' }]) }, value: { element: 'dropdown' } }, + { key: 'object-selector', schema: { type: 'string', title: TITLE }, ctx: { objectNames: loaded(['account']) }, value: 'account' }, + { key: 'object-selector', variant: 'multiple', schema: { type: 'array', title: TITLE }, spec: { multiple: true }, ctx: { objectNames: loaded(['account', 'contact']) }, value: ['account'] }, { key: 'field-selector', schema: { type: 'string', title: TITLE }, spec: { dependsOn: 'objectName' }, formData: { objectName: '' }, value: '' }, - { key: 'field-ref', schema: { type: 'string', title: TITLE }, ctx: { objectFields: [{ name: 'status' }] }, value: 'status' }, - { key: 'field-multi', schema: { type: 'array', title: TITLE }, ctx: { objectFields: [{ name: 'status' }, { name: 'owner' }] }, value: ['status'] }, - { key: 'action-multi', schema: { type: 'array', title: TITLE }, ctx: { objectActions: [{ name: 'approve' }, { name: 'reject' }] }, value: ['approve'] }, - { key: 'filter-builder', schema: { type: 'array', title: TITLE }, ctx: { objectFields: [{ name: 'status' }] }, value: [] }, - { key: 'view-ref', schema: { type: 'string', title: TITLE }, ctx: { objectViews: [{ name: 'all' }] }, value: 'all' }, + { key: 'field-ref', schema: { type: 'string', title: TITLE }, ctx: { objectFields: loaded([{ name: 'status' }]) }, value: 'status' }, + { key: 'field-multi', schema: { type: 'array', title: TITLE }, ctx: { objectFields: loaded([{ name: 'status' }, { name: 'owner' }]) }, value: ['status'] }, + { key: 'action-multi', schema: { type: 'array', title: TITLE }, ctx: { objectActions: loaded([{ name: 'approve' }, { name: 'reject' }]) }, value: ['approve'] }, + { key: 'filter-builder', schema: { type: 'array', title: TITLE }, ctx: { objectFields: loaded([{ name: 'status' }]) }, value: [] }, + { key: 'view-ref', schema: { type: 'string', title: TITLE }, ctx: { objectViews: loaded([{ name: 'all' }]) }, value: 'all' }, { key: 'icon', schema: { type: 'string', title: TITLE }, value: 'check' }, { key: 'color-picker', schema: { type: 'string', title: TITLE, enum: ['default', 'blue'] }, value: 'blue' }, { key: 'color-input', schema: { type: 'string', title: TITLE }, value: '#112233' }, diff --git a/packages/app-shell/src/views/metadata-admin/ViewRefWidget.test.tsx b/packages/app-shell/src/views/metadata-admin/ViewRefWidget.test.tsx index a230bc3d7d..c0358cb941 100644 --- a/packages/app-shell/src/views/metadata-admin/ViewRefWidget.test.tsx +++ b/packages/app-shell/src/views/metadata-admin/ViewRefWidget.test.tsx @@ -3,6 +3,7 @@ import { describe, it, expect, afterEach } from 'vitest'; import { render, screen, cleanup } from '@testing-library/react'; import { WIDGETS, resolveStoredViewRef } from './widgets'; +import { loaded } from './loadState'; afterEach(cleanup); @@ -26,10 +27,10 @@ describe('view-ref widget', () => { value="default" onChange={() => {}} schema={{ type: 'string' }} - context={{ objectViews: [ + context={{ objectViews: loaded([ { name: 'default', label: 'All records' }, { name: 'mine', label: 'My records' }, - ] }} + ]) }} />, ); expect(screen.getByRole('combobox')).toBeInTheDocument(); @@ -41,7 +42,7 @@ describe('view-ref widget', () => { value={undefined} onChange={() => {}} schema={{ type: 'string' }} - context={{ objectViews: [] }} + context={{ objectViews: loaded([]) }} />, ); expect(screen.getByRole('combobox')).toBeInTheDocument(); @@ -53,7 +54,7 @@ describe('view-ref widget', () => { value="renamed_view" onChange={() => {}} schema={{ type: 'string' }} - context={{ objectViews: [{ name: 'default', label: 'All records' }] }} + context={{ objectViews: loaded([{ name: 'default', label: 'All records' }]) }} />, ); expect(screen.getByRole('combobox')).toBeInTheDocument(); diff --git a/packages/app-shell/src/views/metadata-admin/WidgetContext.catalogUnion.test.tsx b/packages/app-shell/src/views/metadata-admin/WidgetContext.catalogUnion.test.tsx new file mode 100644 index 0000000000..1078beb4a9 --- /dev/null +++ b/packages/app-shell/src/views/metadata-admin/WidgetContext.catalogUnion.test.tsx @@ -0,0 +1,388 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * A `WidgetContext` option catalog carries its own load state, and a picker + * cannot read the list without deciding what a failure looks like — + * objectui#5228. + * + * ## The residue this closes + * + * objectui#5170 / objectui#5169 stopped the three `ResourceEditPage` loaders + * spelling a fault as a measurement, but the fix was projected back down at the + * `WidgetContext` boundary: the catalogs stayed plain arrays — `[]` on a failed + * load, byte-identical to a load that completed and found nothing — and the + * fault travelled alongside them on a `catalogErrors` record. Nothing in the + * types said so. The requirement lived in `CatalogErrors`' own doc comment, + * which had to open with "a picker MUST consult this before it renders its + * catalog", and one line was enough to ignore it: + * + * const fields = context?.objectFields ?? []; + * + * — type-correct, natural to read, passes review, and renders a refusal or an + * expired session as the metadata graph's own answer of "this object has no + * fields". A comment doing a type's job. + * + * ## What is pinned here, and by which tool + * + * Two halves, and they fail in different places on purpose: + * + * • **`tsc`** — the `@ts-expect-error` block below. These assertions are + * erased at runtime, so vitest proves nothing about them; the package's + * `type-check` script (`tsc -p tsconfig.test.json`, which is the only + * project that compiles this directory's tests) is what judges them. Each + * one is a two-way pin: `@ts-expect-error` is itself an error (TS2578) when + * the line below it starts compiling, so loosening the contract turns this + * file red rather than quietly passing. + * • **vitest** — the render blocks. A COMPLETED-but-empty catalog and a + * FAILED catalog reach every migrated picker as different values and come + * out as different screens, asserted in both directions. The empty arm is + * what stops the failure assertions being tautologies: had the union been + * bought by deleting the empty rendering, "not an empty list" would pass + * for the wrong reason everywhere. + * + * Every failure arm renders the SHARED `PickerLoadFailure` block, the one + * objectui#5170 landed and objectui#5227 reused — asserted by its test id and + * its heading, so a second, bespoke failure presentation fails here. + * + * ## Why the observables are structural + * + * Measured by PR #5226 and re-confirmed: Radix `SelectValue` does not render + * its `placeholder` in jsdom, so asserting on "No object bound" would pass for + * every arm and pin nothing. Each arm is read by structure instead — the + * failure block by test id, the completed arms by the control they render and + * by the empty-state sentences that are plain text rather than placeholders. + */ + +import '@testing-library/jest-dom/vitest'; +import { describe, it, expect, afterEach, vi } from 'vitest'; +import { render, screen, cleanup, within } from '@testing-library/react'; +import { WIDGETS, type RegisteredWidgetKey, type WidgetProps } from './widgets'; +import { t } from './i18n'; +import { loaded, failed, NOT_ASKED, offeredOptions, type LoadState } from './loadState'; +import type { + ObjectActionOption, + ObjectFieldOption, + ObjectViewOption, + WidgetContext, +} from './widgets'; + +// `filter-builder` keeps its failure block inside a Radix popover, which does +// not open in jsdom. Rendering the popover parts inline is this repo's existing +// way of reaching such content in a unit test (see the `InboxPopover` suites); +// everything else in the barrel stays real. No JSX in the factory: `vi.mock` is +// hoisted above the imports, so the factory must not depend on them. +vi.mock('@object-ui/components', async () => { + const actual = await vi.importActual>('@object-ui/components'); + const Pass = ({ children }: { children?: unknown }) => children as never; + return { ...actual, Popover: Pass, PopoverTrigger: Pass, PopoverContent: Pass }; +}); + +afterEach(cleanup); + +const CAUSE = 'HTTP 503: metadata service unavailable'; +/** The heading of the one shared failure block every picker must use. */ +const LOAD_FAILED_TITLE = t('engine.form.optionsLoadFailedTitle', 'en-US'); +/** What `ref:object` says for a COMPLETED list of zero objects — a MEASUREMENT. */ +const NO_OBJECTS = t('engine.form.noObjects', 'en-US'); + +/* ========================================================================== */ +/* 1. Compile-time: the failure arm cannot be skipped */ +/* ========================================================================== */ + +/** + * These never run. They are the half of this card that a runtime test cannot + * express — "a call site that ignores the failure arm does not compile" is a + * statement about `tsc`, not about the DOM. + */ +export function catalogUnionTypePins(): void { + const failedFields: WidgetContext = { objectFields: failed(CAUSE) }; + + // ── PIN A — the naive read. This is the exact line objectui#5228 was filed + // about, and it is the one that must stop compiling. A `LoadState` is not an + // array, so `?? []` cannot produce a list. + // + // Two-way: revert `WidgetContext.objectFields` to `ObjectFieldOption[]` and + // this line compiles again, which makes the directive unused — TS2578 — so + // the pin fails on the loosening as well as holding on the tightening. + // @ts-expect-error — a catalog is a LoadState, not an array to default away + const naive: ObjectFieldOption[] = failedFields.objectFields ?? []; + void naive; + + // ── PIN B — the accessor refuses an undecided state. `offeredOptions` takes + // `FaultFreeLoadState`, so a caller still holding the `error` arm cannot read + // the catalog at all: the failure decision is a PRECONDITION checked by the + // compiler, not a rule asked for in prose. + // + // Two-way in the other direction: widen `FaultFreeLoadState` to include the + // error arm — the loosening that would restore the old "please remember to + // check" contract — and this call compiles, making the directive unused. + const anyArm: LoadState = failed(CAUSE); + // @ts-expect-error — the error arm has not been decided yet + offeredOptions(anyArm, [] as ObjectFieldOption[]); + + // ── PIN B, positive control. Without this, PIN B would also pass if + // `offeredOptions` were simply uncallable. Once the failure is decided, the + // very same state is accepted and yields the list. + if (anyArm.status !== 'error') { + const options: ObjectFieldOption[] = offeredOptions(anyArm, []); + void options; + } + + // ── PIN C — a fault and a measurement are different TYPES, not the same type + // with a flag beside it. This is the sentence the card leads with. + const completedEmpty: { status: 'loaded'; data: ObjectFieldOption[] } = { + status: 'loaded', + data: [], + }; + // @ts-expect-error — a failure cannot be spelled as a completed, empty load + const conflated: typeof completedEmpty = { status: 'error', message: CAUSE }; + void completedEmpty; + void conflated; + + // ── PIN D — the side channels are gone, not merely unused. A producer that + // still tries to route the fault alongside the catalog is refused, so the old + // shape cannot creep back one key at a time. + const legacy: WidgetContext = { + objectFields: loaded([]), + // @ts-expect-error — `catalogErrors` no longer exists: the arm carries it + catalogErrors: { fields: CAUSE }, + }; + void legacy; + + const legacyLoading: WidgetContext = { + objectNames: loaded([]), + // @ts-expect-error — `objectsLoading` no longer exists: `loading` is an arm + objectsLoading: true, + }; + void legacyLoading; + + // ── PIN E — absence is the `idle` arm, and it is a value of the union rather + // than a fabricated empty list. Compiles; it is here so the substitution every + // migrated picker performs is itself type-checked. + const unwiredContext: WidgetContext = {}; + const unwired: LoadState = unwiredContext.objectViews ?? NOT_ASKED; + void unwired; +} + +/* ========================================================================== */ +/* 2. Runtime: an empty catalog and a failed catalog render differently */ +/* ========================================================================== */ + +function renderWidget( + key: RegisteredWidgetKey, + context: WidgetContext, + props: Partial = {}, +) { + const Widget = WIDGETS[key]; + return render( + {}} + schema={{ type: 'string' }} + context={context} + {...props} + />, + ); +} + +/** + * The failure block is present, is the SHARED one, and carries the cause. + * + * The heading is looked up WITHIN the block rather than on the document: + * measured, the ordered-set pickers (`field-multi`, `action-multi`) also put + * this same sentence in their add-trigger placeholder, so a document-wide + * `getByText` finds two and throws. That duplication is intended — the trigger + * has to say why it is empty — and asserting inside the block is what keeps + * this helper pinning the BLOCK rather than either occurrence of the string. + */ +function expectSharedFailure(testId: string) { + const block = screen.getByTestId(testId); + expect(block).toBeInTheDocument(); + expect(within(block).getByText(LOAD_FAILED_TITLE)).toBeInTheDocument(); + expect(within(block).getByTestId(`${testId}-cause`)).toHaveTextContent(CAUSE); +} + +const EMPTY_FIELDS: ObjectFieldOption[] = []; +const EMPTY_VIEWS: ObjectViewOption[] = []; +const EMPTY_ACTIONS: ObjectActionOption[] = []; + +describe('ref:object — a failed object list is not "no objects detected"', () => { + it('a COMPLETED, empty list still says so (the measurement survives)', () => { + renderWidget('ref:object', { objectNames: loaded([]) }); + expect(screen.getByPlaceholderText(NO_OBJECTS)).toBeInTheDocument(); + expect(screen.queryByTestId('ref-object-load-failed')).toBeNull(); + }); + + it('a FAILED list renders the shared failure block and claims nothing', () => { + renderWidget('ref:object', { objectNames: failed(CAUSE) }); + expectSharedFailure('ref-object-load-failed'); + // The sentence that would be false is not on screen. + expect(screen.queryByPlaceholderText(NO_OBJECTS)).toBeNull(); + // …and authoring is not blocked: the freeform input survives. + expect(screen.getByRole('textbox')).toBeEnabled(); + }); + + it('the LOADING arm asserts neither', () => { + renderWidget('ref:object', { objectNames: { status: 'loading' } }); + expect(screen.queryByTestId('ref-object-load-failed')).toBeNull(); + expect(screen.queryByPlaceholderText(NO_OBJECTS)).toBeNull(); + }); + + it('an UNWIRED catalog is idle, not a failure', () => { + renderWidget('ref:object', {}); + expect(screen.queryByTestId('ref-object-load-failed')).toBeNull(); + }); +}); + +describe('object-selector — a failed object list is not an empty dropdown', () => { + const spec = { fieldSpec: { field: 'objectName', multiple: true } }; + + it('a COMPLETED, empty list still renders the picker', () => { + renderWidget('object-selector', { objectNames: loaded([]) }, spec); + expect(screen.getByRole('combobox')).toBeInTheDocument(); + expect(screen.queryByTestId('object-selector-load-failed')).toBeNull(); + }); + + it('a FAILED list replaces the picker with the shared failure block', () => { + renderWidget('object-selector', { objectNames: failed(CAUSE) }, spec); + expectSharedFailure('object-selector-load-failed'); + expect(screen.queryByRole('combobox')).toBeNull(); + }); + + it('a FAILED list still lets the author edit what is already stored', () => { + const onChange = vi.fn(); + renderWidget('object-selector', { objectNames: failed(CAUSE) }, { + ...spec, + value: ['showcase_account'], + onChange, + }); + expect(screen.getByText('showcase_account')).toBeInTheDocument(); + screen.getByRole('button', { name: '×' }).click(); + expect(onChange).toHaveBeenCalledWith([]); + }); +}); + +describe('field-ref — a failed field catalog is not an object without fields', () => { + it('a COMPLETED, empty catalog still renders the picker', () => { + renderWidget('field-ref', { objectFields: loaded(EMPTY_FIELDS) }); + expect(screen.getByRole('combobox')).toBeInTheDocument(); + expect(screen.queryByTestId('field-ref-load-failed')).toBeNull(); + }); + + it('a FAILED catalog replaces the picker with the shared failure block', () => { + renderWidget('field-ref', { objectFields: failed(CAUSE) }); + expectSharedFailure('field-ref-load-failed'); + expect(screen.queryByRole('combobox')).toBeNull(); + }); +}); + +describe('view-ref — a failed view catalog is not an object without views', () => { + it('a COMPLETED, empty catalog still renders the picker', () => { + renderWidget('view-ref', { objectViews: loaded(EMPTY_VIEWS) }); + expect(screen.getByRole('combobox')).toBeInTheDocument(); + expect(screen.queryByTestId('view-ref-load-failed')).toBeNull(); + }); + + it('a FAILED catalog replaces the picker with the shared failure block', () => { + renderWidget('view-ref', { objectViews: failed(CAUSE) }); + expectSharedFailure('view-ref-load-failed'); + expect(screen.queryByRole('combobox')).toBeNull(); + }); +}); + +describe('field-multi — a failed field catalog is not an empty add-list', () => { + it('a COMPLETED, empty catalog renders the add picker and no failure', () => { + renderWidget('field-multi', { objectFields: loaded(EMPTY_FIELDS) }, { value: [] }); + expect(screen.getByRole('combobox')).toBeInTheDocument(); + expect(screen.queryByTestId('field-multi-load-failed')).toBeNull(); + }); + + it('a FAILED catalog renders the shared failure block above the picker', () => { + renderWidget('field-multi', { objectFields: failed(CAUSE) }, { value: [] }); + expectSharedFailure('field-multi-load-failed'); + }); + + it('a FAILED catalog leaves already-chosen fields visible and reorderable', () => { + renderWidget('field-multi', { objectFields: failed(CAUSE) }, { value: ['status', 'owner'] }); + // Each chip prints the label AND the machine name, so the name is on screen + // twice by design — assert the chip's controls instead of a bare text match. + expect(screen.getByLabelText('Remove status')).toBeInTheDocument(); + expect(screen.getByLabelText('Remove owner')).toBeInTheDocument(); + }); +}); + +describe('action-multi — the action catalog carries its OWN fault', () => { + it('a COMPLETED, empty catalog renders the add picker and no failure', () => { + renderWidget('action-multi', { objectActions: loaded(EMPTY_ACTIONS) }, { value: [] }); + expect(screen.getByTestId('action-multi')).toBeInTheDocument(); + expect(screen.queryByTestId('action-multi-load-failed')).toBeNull(); + }); + + it('a FAILED catalog renders the shared failure block', () => { + renderWidget('action-multi', { objectActions: failed(CAUSE) }, { value: [] }); + expectSharedFailure('action-multi-load-failed'); + }); + + /** + * The old boundary made this picker read `catalogErrors.fields` to learn + * whether the ACTION catalog had failed, because the two ride one request. + * They still do — `ResourceEditPage` derives both from one `LoadState` — but + * the pairing is now the producer's job to state, and this picker reads only + * its own arm. Pinned so a field fault can no longer post a banner over a + * perfectly good action list by accident of key naming. + */ + it('a failed FIELD catalog does not put a failure on the ACTION picker', () => { + renderWidget( + 'action-multi', + { objectFields: failed(CAUSE), objectActions: loaded([{ name: 'approve' }]) }, + { value: [] }, + ); + expect(screen.queryByTestId('action-multi-load-failed')).toBeNull(); + }); +}); + +describe('filter-mode — a failed field catalog is not "bind a source object"', () => { + /** The empty-state sentence, which NAMES A CAUSE that is false on a failure. */ + const BIND_SOURCE = 'Bind a source object to pick filter fields.'; + + it('a COMPLETED, empty catalog still shows the empty-state sentence', () => { + renderWidget('filter-mode', { objectFields: loaded(EMPTY_FIELDS) }, { + value: { element: 'dropdown' }, + schema: { type: 'object' }, + }); + expect(screen.getByText(BIND_SOURCE)).toBeInTheDocument(); + expect(screen.queryByTestId('filter-mode-fields-load-failed')).toBeNull(); + }); + + it('a FAILED catalog replaces that sentence with the shared failure block', () => { + renderWidget('filter-mode', { objectFields: failed(CAUSE) }, { + value: { element: 'dropdown' }, + schema: { type: 'object' }, + }); + expectSharedFailure('filter-mode-fields-load-failed'); + expect(screen.queryByText(BIND_SOURCE)).toBeNull(); + }); +}); + +describe('filter-builder — a failed field catalog is not "no conditions to add"', () => { + const BIND_SOURCE = 'Bind a source object to add filter conditions.'; + + it('a COMPLETED, empty catalog still shows the empty-state sentence', () => { + renderWidget('filter-builder', { objectFields: loaded(EMPTY_FIELDS) }, { + value: undefined, + schema: { type: 'array' }, + }); + expect(screen.getByText(BIND_SOURCE)).toBeInTheDocument(); + expect(screen.queryByTestId('filter-builder-load-failed')).toBeNull(); + }); + + it('a FAILED catalog replaces that sentence with the shared failure block', () => { + renderWidget('filter-builder', { objectFields: failed(CAUSE) }, { + value: undefined, + schema: { type: 'array' }, + }); + expectSharedFailure('filter-builder-load-failed'); + expect(screen.queryByText(BIND_SOURCE)).toBeNull(); + }); +}); diff --git a/packages/app-shell/src/views/metadata-admin/inspectors/ViewVariantInspector.tsx b/packages/app-shell/src/views/metadata-admin/inspectors/ViewVariantInspector.tsx index 81a079da1a..47d7e8dacd 100644 --- a/packages/app-shell/src/views/metadata-admin/inspectors/ViewVariantInspector.tsx +++ b/packages/app-shell/src/views/metadata-admin/inspectors/ViewVariantInspector.tsx @@ -37,6 +37,8 @@ import { useObjectOptions } from './useDatasetFields'; import type { MetadataDefaultInspectorProps } from '../default-inspector-registry'; import { SchemaForm } from '../SchemaForm'; import { useObjectFields, type ObjectFieldInfo } from '../previews/useObjectFields'; +import { failed, loaded, type LoadState } from '../loadState'; +import type { ObjectFieldOption, WidgetContext } from '../widgets'; /** * Object picker for the view's binding — a searchable dropdown over the live @@ -233,10 +235,11 @@ export function ViewVariantInspector({ // Load the bound object's field catalog so field-reference config props // (groupByField, startDateField, xAxisField, visibleFields, …) render as // object-field pickers rather than free-text inputs. - const { fields: objectFields } = useObjectFields( - binding.value || undefined, - objectFieldsOverride, - ); + const { + fields: objectFields, + loading: objectFieldsLoading, + error: objectFieldsError, + } = useObjectFields(binding.value || undefined, objectFieldsOverride); // Locale-bound `t` for child components that take a bare `(key) => string` // (e.g. CelPredicateField / ConditionalFormattingEditor). const tLocal = React.useCallback((k: string) => t(k, locale), [locale]); @@ -278,16 +281,41 @@ export function ViewVariantInspector({ React.useEffect(() => { onBlockingIssuesChangeRef.current?.(blockingIssues); }, [blockingIssues]); - const widgetContext = React.useMemo( - () => ({ - objectFields: objectFields.map((f) => ({ - name: f.name, - label: f.label, - type: f.type, - })), - }), - [objectFields], - ); + /** + * objectui#5228 — the inspector is the SECOND host of `WidgetContext`, and it + * used to drop two thirds of what its loader knows. + * + * `useObjectFields` reports a triple (`fields` / `loading` / `error`), and + * this memo forwarded only `fields`. So a field catalog that FAILED to load + * arrived at the `field-ref` / `field-multi` pickers as `[]` — the same value + * a bound object with no fields sends — and the picker said "No object bound" + * about an object that is bound and whose catalog simply could not be + * fetched. That is the objectui#5170 defect class, still open on this host + * after the `ResourceEditPage` one was closed, and it is here because nothing + * in the old `WidgetContext` type required a producer to carry the fault at + * all: the array WAS the contract. + * + * It cannot be dropped any more — the catalog is a `LoadState`, so the three + * arms have to be spelled out. `loading` and `error` come straight off the + * hook; everything else is a load that completed, including "no object bound + * yet", which the hook already reports as an empty, error-free result and the + * pickers already render as their empty state. No `idle` arm is synthesized + * here: introducing one would change what an unbound inspector renders, which + * is a separate question from the fault this closes. + */ + const widgetContext = React.useMemo(() => { + let fieldCatalog: LoadState; + if (objectFieldsError) { + fieldCatalog = failed(objectFieldsError); + } else if (objectFieldsLoading) { + fieldCatalog = { status: 'loading' }; + } else { + fieldCatalog = loaded( + objectFields.map((f) => ({ name: f.name, label: f.label, type: f.type })), + ); + } + return { objectFields: fieldCatalog }; + }, [objectFields, objectFieldsLoading, objectFieldsError]); // Graft server-only fields onto the bundled variant form so new server // fields are editable even when the bundled spec lags (skew root-cure). A diff --git a/packages/app-shell/src/views/metadata-admin/loadState.ts b/packages/app-shell/src/views/metadata-admin/loadState.ts index 936f3b2ae4..a4e288be9a 100644 --- a/packages/app-shell/src/views/metadata-admin/loadState.ts +++ b/packages/app-shell/src/views/metadata-admin/loadState.ts @@ -50,11 +50,106 @@ import * as React from 'react'; * * ADR-0110 D3 — a miss and a fault are different facts. */ -export type LoadState = +export type LoadState = FaultFreeLoadState | { status: 'error'; message: string }; + +/** + * Every arm of {@link LoadState} EXCEPT the failure. + * + * This exists to make one sentence checkable by the compiler instead of asked + * for in a doc comment: **a picker may not read its catalog until it has + * decided what a failure looks like.** {@link offeredOptions} takes this type, + * so a call site that has not narrowed the `error` arm away cannot call it at + * all — the decision is a precondition of the read, not a convention around it. + * + * `LoadState` is declared in terms of this rather than the other way round so + * the two can never drift: adding an arm to the union adds it here, and an arm + * added HERE is automatically one a picker is allowed to reach the catalog + * from. Structurally the union is byte-identical to what it was before the + * split (`idle | loading | loaded | error`), so no consumer of `LoadState` + * sees a change. + * + * Note what is NOT in the name: these are not the "successful" arms. `idle` and + * `loading` are here because a picker renders its own copy for them (the + * "(Select an object first)" / "Loading…" states) — what they share with + * `loaded` is only that no fault has been reported, which is exactly the + * precondition being enforced. + */ +export type FaultFreeLoadState = | { status: 'idle' } | { status: 'loading' } - | { status: 'loaded'; data: T } - | { status: 'error'; message: string }; + | { status: 'loaded'; data: T }; + +/** + * The `idle` arm, as the value a consumer substitutes for a catalog slot its + * host never wired at all (objectui#5228). + * + * `WidgetContext`'s catalogs are optional: a host that does not fetch object + * views simply omits `objectViews`. That absence is a question never asked — + * the same fact `idle` carries — so it resolves to the same arm rather than to + * a fabricated empty list. One shared value so the substitution is also + * referentially stable: a fresh `{ status: 'idle' }` per render would defeat + * the memo on any consumer that keys off catalog identity. + */ +export const NOT_ASKED: FaultFreeLoadState = { status: 'idle' }; + +/** + * The options a picker may OFFER, for a state whose failure arm the caller has + * already handled (objectui#5228). + * + * The parameter type is {@link FaultFreeLoadState}, not {@link LoadState}, and + * that is the entire point: a caller still holding the `error` arm cannot pass + * it, so `const fields = offeredOptions(state, EMPTY)` does not compile until + * the line above it has decided what a failure renders as. That is the + * difference between this and {@link loadedData}, which accepts any arm and + * therefore can only *ask* (in prose) for the same discipline — the shape + * objectui#5228 was filed about, a comment doing a type's job. + * + * `empty` is what the `idle` and `loading` arms offer: nothing yet. It is NOT a + * failure fallback — no failure can reach this function. + */ +export function offeredOptions(state: FaultFreeLoadState, empty: T): T { + return state.status === 'loaded' ? state.data : empty; +} + +/** + * Project the data of a COMPLETED load through `select`, passing every other + * arm through untouched. + * + * Used where one request answers two questions: `ResourceEditPage` fetches an + * object's fields and its actions in a single `client.get('object', …)`, and + * hands the pickers one catalog each. Deriving both from the one state is what + * makes them agree by construction — two independently-built states could + * report a fault for the fields and success for the actions, which no single + * request can actually produce. + */ +export function mapLoaded(state: LoadState, select: (data: A) => B): LoadState { + return state.status === 'loaded' ? { status: 'loaded', data: select(state.data) } : state; +} + +/** + * A load that COMPLETED, carrying `data`. + * + * Mostly a fixture constructor: a test that means "this catalog loaded and + * holds these three fields" writes `loaded([...])` instead of restating the + * arm's shape, and — the reason it is worth a helper — cannot accidentally + * spell a *failed* catalog the same way, which is the defect objectui#5170 and + * objectui#5228 are both about. + */ +export function loaded(data: T): LoadState { + return { status: 'loaded', data }; +} + +/** + * A load that FAILED, carrying the cause. + * + * The counterpart to {@link loaded}, and the reason both exist: a suite that + * pins "an empty catalog and a failed catalog render differently" needs to + * write the two of them side by side, and they must be visibly different + * values rather than the same `[]` distinguished by a second argument. + */ +export function failed(message: string): LoadState { + return { status: 'error', message }; +} /** * Normalise a thrown value into the message the failure state shows. 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 f076f45bc8..907f0ea3f9 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 @@ -3,6 +3,7 @@ import { describe, it, expect, afterEach, vi } from 'vitest'; import { render, screen, cleanup } from '@testing-library/react'; import { WIDGETS } from './widgets'; +import { loaded } from './loadState'; import { t } from './i18n'; afterEach(() => { @@ -35,7 +36,7 @@ describe('metadata-admin selector placeholders (objectui#4387 key collapse)', () schema={{ type: 'string' }} value="" onChange={() => {}} - context={{ objectNames: ['account', 'contact'] }} + context={{ objectNames: loaded(['account', 'contact']) }} fieldSpec={{ field: 'objectName', multiple: false }} />, ); diff --git a/packages/app-shell/src/views/metadata-admin/widgets.tsx b/packages/app-shell/src/views/metadata-admin/widgets.tsx index 8e1f4c09d9..98a553d327 100644 --- a/packages/app-shell/src/views/metadata-admin/widgets.tsx +++ b/packages/app-shell/src/views/metadata-admin/widgets.tsx @@ -51,67 +51,104 @@ 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'; +import { + type LoadState, + isLoading, + loadErrorOf, + loadedData, + NOT_ASKED, + offeredOptions, + usePickerLoad, +} from './loadState'; + +/* -------------------------------------------------------------------------- */ +/* The option catalogs a picker reads off {@link WidgetContext} */ +/* -------------------------------------------------------------------------- */ + +/** One entry of the bound object's field catalog (`field-ref`, `field-multi`). */ +export interface ObjectFieldOption { + name: string; + label?: string; + type?: string; +} + +/** One entry of the source object's view catalog (`view-ref`). */ +export interface ObjectViewOption { + name: string; + label?: string; +} + +/** One entry of the source object's action catalog (`action-multi`). */ +export interface ObjectActionOption { + name: string; + label?: string; + locations?: string[]; +} /** - * Load failures for the option catalogs on {@link WidgetContext}, by catalog - * (objectui#5170). + * A catalog handed to the pickers, in whichever of the four states its load is + * actually in (objectui#5228). + * + * ## What this replaced, and why the replacement is a type and not a rule + * + * Until objectui#5228 this boundary spelled a catalog as a plain array plus two + * side channels — a `*Loading` flag and a `catalogErrors` record — which meant a + * FAILED load arrived here as `[]`, byte-identical to a load that completed and + * found nothing. Nothing in the types said the failure channel existed: the + * requirement lived in `CatalogErrors`' own doc comment, which had to open with + * "a picker MUST consult this before it renders its catalog". That is a comment + * doing a type's job, and it is one line away from being ignored — * - * A key is present **only** when that catalog's load FAILED. It is absent for a - * catalog that completed and found nothing, and absent for one that was never - * asked (no source object bound) — a miss and a fault are different facts - * (ADR-0110 D3). + * ```ts + * const fields = context?.objectFields ?? []; // type-correct, reads fine, + * ``` * - * ⚠️ A picker MUST consult this **before** it renders its catalog. The catalog - * arrays are still empty on failure, so `objectFields.length === 0` on its own - * cannot tell "this object has no fields" from "we could not ask" — that - * conflation is the defect this type exists to close. `ResourceEditPage` holds - * the authoritative four-arm `LoadState` (see `./loadState`) and projects the - * failure arm here; the empty array is the fallback for consumers that cannot - * render a failure, never a claim that the catalog is empty. + * — which renders a refusal, a dropped connection or an expired session as the + * metadata graph's own answer of "this object has no fields", the defect + * objectui#5170 and objectui#5169 were filed for. * - * `fields` covers the action catalog as well: both come from the one - * `client.get('object', …)` request, so they fail together. + * As a union that read does not compile: `LoadState` is not an array, so + * every site that wants the list has to go through {@link offeredOptions}, + * whose parameter type excludes the `error` arm. The failure decision stops + * being a rule a reviewer has to remember and becomes a precondition the + * compiler checks, at exactly the sites that must decide what a failure looks + * like. + * + * ## Absent still means something + * + * The catalogs stay optional — a host that never fetches views omits + * `objectViews` — and absence reads as {@link NOT_ASKED}, the `idle` arm. A + * question never asked is not a failure and must not render as one (ADR-0110 + * D3). */ -export interface CatalogErrors { - /** The object-name list could not be loaded (`ref:object`, `object-selector`). */ - objects?: string; - /** The bound object's field AND action catalogs could not be loaded. */ - fields?: string; - /** The bound object's view catalog could not be loaded (`view-ref`). */ - views?: string; -} - export interface WidgetContext { - /** Names of all object metadata records (for `ref:object`). */ - objectNames?: string[]; - /** Loading flag for the object list. */ - objectsLoading?: boolean; + /** Names of all object metadata records (for `ref:object`, `object-selector`). */ + objectNames?: LoadState; /** - * Field catalog of the bound object. Drives the `field-ref` / - * `field-multi` pickers so View config props that reference a field - * (kanban.groupByField, calendar.startDateField, chart.xAxisField, …) - * render as dropdowns of the object's real fields instead of free text. + * Field catalog of the bound object. Drives the `field-ref` / `field-multi` + * pickers so View config props that reference a field (kanban.groupByField, + * calendar.startDateField, chart.xAxisField, …) render as dropdowns of the + * object's real fields instead of free text. */ - objectFields?: Array<{ name: string; label?: string; type?: string }>; - /** Loading flag for the field catalog. */ - objectFieldsLoading?: boolean; + objectFields?: LoadState; /** - * View catalog of the bound/source object. Drives the `view-ref` picker - * so `interfaceConfig.sourceView` renders as a dropdown of the source - * object's real views instead of a free-text name the author can typo. + * View catalog of the bound/source object. Drives the `view-ref` picker so + * `interfaceConfig.sourceView` renders as a dropdown of the source object's + * real views instead of a free-text name the author can typo. */ - objectViews?: Array<{ name: string; label?: string }>; - /** Loading flag for the view catalog. */ - objectViewsLoading?: boolean; + objectViews?: LoadState; /** * Action catalog of the bound/source object. Drives the `action-multi` * picker so interface-page toolbar `buttons` reference the object's real * actions (ActionSchema) instead of free-text — correct-by-construction. + * + * Rides the same request as {@link WidgetContext.objectFields} today, and + * `ResourceEditPage` derives both from that one state (see `mapLoaded`), so + * the two agree by construction rather than by the reader knowing they share + * a fetch — which is what the old `catalogErrors.fields` key, consulted by + * the *action* picker to learn whether *actions* had failed, asked of them. */ - objectActions?: Array<{ name: string; label?: string; locations?: string[] }>; - /** Loading flag for the action catalog. */ - objectActionsLoading?: boolean; + objectActions?: LoadState; /** * Per-value sub-schemas for the `dynamic-config` widget: a map from a parent * field's value (e.g. the chosen driver id) to the JSON-Schema describing the @@ -127,13 +164,17 @@ export interface WidgetContext { * free-text id the author can typo. */ componentIds?: Array<{ id: string; type?: string; label?: string }>; - /** - * Which of the catalogs above FAILED to load. See {@link CatalogErrors} — - * a picker checks this before it renders an empty catalog as "none exist". - */ - catalogErrors?: CatalogErrors; } +/* Stable empties for the arms that have no catalog to offer yet — `idle` and + `loading`. Module-level so {@link offeredOptions} returns the SAME array + across renders and a memo downstream of it does not churn. Never a failure + fallback: no failure can reach `offeredOptions` at all. */ +const NO_OBJECT_NAMES: string[] = []; +const NO_OBJECT_FIELDS: ObjectFieldOption[] = []; +const NO_OBJECT_VIEWS: ObjectViewOption[] = []; +const NO_OBJECT_ACTIONS: ObjectActionOption[] = []; + export interface WidgetProps { /** * The host field id, handed down ONLY to a widget declared @@ -270,27 +311,20 @@ function RefObjectWidget({ context, }: WidgetProps) { const locale = useMetadataLocale(); - const names = context?.objectNames ?? []; + const objectsState = context?.objectNames ?? NOT_ASKED; const v = value == null ? '' : String(value); - if (context?.objectsLoading) { - return ( - - ); - } // The object list FAILED to load — not the same fact as "there are no // objects", which is what the empty branch below says out loud (objectui#5170). // The freeform input is kept, and enabled, so a failed catalog does not also // block authoring. - const loadError = context?.catalogErrors?.objects; - if (loadError) { + // + // objectui#5228: this arm is now checked because the compiler makes it be — + // `offeredOptions` below refuses a state that still carries the `error` arm, + // so the list cannot be read until this decision is written down. + if (objectsState.status === 'error') { return (
- + ); } + if (isLoading(objectsState)) { + return ( + + ); + } + const names = offeredOptions(objectsState, NO_OBJECT_NAMES); // If list is empty (e.g. no objects defined yet), fall back to a // freeform text input so the user can still type a value. if (names.length === 0) { @@ -465,7 +510,7 @@ function ObjectSelectorWidget({ fieldSpec, }: WidgetProps) { const locale = useMetadataLocale(); - const names = context?.objectNames ?? []; + const objectsState = context?.objectNames ?? NOT_ASKED; const multiple = fieldSpec?.multiple ?? false; // Parse value: string[], string (comma-separated), or empty @@ -496,7 +541,7 @@ function ObjectSelectorWidget({ onChange(multiple ? newSelection : ''); }; - if (context?.objectsLoading) { + if (isLoading(objectsState)) { return ; } @@ -504,55 +549,65 @@ function ObjectSelectorWidget({ // otherwise render as a completed, empty dropdown — indistinguishable from an // install that genuinely has no objects. Already-selected values stay visible // and removable; only the "add" picker is replaced. - const loadError = context?.catalogErrors?.objects; + /* Whatever is already selected, kept visible and removable in EVERY arm — + the shape objectui#5227 landed for `field-selector`: a failed catalog must + not also block authoring. */ + const selectedChips = selectedValues.length > 0 && ( +
+ {selectedValues.map(obj => ( +
+ {obj} + {!readOnly && ( + + )} +
+ ))} +
+ ); + + if (objectsState.status === 'error') { + return ( +
+ {selectedChips} + +
+ ); + } + + const names = offeredOptions(objectsState, NO_OBJECT_NAMES); return (
{/* Selected items */} - {selectedValues.length > 0 && ( -
- {selectedValues.map(obj => ( -
- {obj} - {!readOnly && ( - - )} -
- ))} -
- )} + {selectedChips} {/* Object picker */} - {loadError ? ( - - ) : ( - - )} +
); } @@ -577,8 +632,8 @@ const EMPTY_FIELD_SELECTOR_OPTIONS: FieldSelectorOption[] = []; * * 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 + * uses — which is why objectui#5170 missed it and why the option catalogs on + * {@link WidgetContext} do 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: * @@ -1182,24 +1237,24 @@ const NO_FIELD = '__none__'; */ function FieldRefWidget({ id, value, onChange, readOnly, context }: WidgetProps) { const locale = useMetadataLocale(); - const fields = context?.objectFields ?? []; + const fieldsState = context?.objectFields ?? NOT_ASKED; const current = value == null ? '' : String(value); - const inCatalog = !current || fields.some((f) => f.name === current); // Four structurally distinct arms, one per `LoadState` (objectui#5170), so a // fault can never be read as a measurement. Note what the empty placeholder // below claims: "No object bound" — on a failed load an object IS bound, so // that sentence names a cause that is not the real one, which is why the // failure arm replaces the picker rather than decorating it (the shape #5110 // landed for the References panel). - const loadError = context?.catalogErrors?.fields; - if (loadError) { - return ; + if (fieldsState.status === 'error') { + return ; } // Same in-file precedent as `ref:object` / `object-selector`: an unanswered // question renders as "asking", never as an answer of none. - if (context?.objectFieldsLoading) { + if (isLoading(fieldsState)) { return ; } + const fields = offeredOptions(fieldsState, NO_OBJECT_FIELDS); + const inCatalog = !current || fields.some((f) => f.name === current); return ( ; } + const views = offeredOptions(viewsState, NO_OBJECT_VIEWS); + // Mirror the runtime resolver (InterfaceListPage.resolveSourceView): a stored + // value resolves if it's an exact view name, OR a bare name matching a view's + // `.` suffix, OR the special `default`/`list` (→ object default + // view). Only a value that resolves to NOTHING gets the "(not in object)" tag — + // so a working bare value like `default` is no longer mislabelled. + const { suffixMatch, resolves, showStored } = resolveStoredViewRef(views, current); return (