Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 42 additions & 0 deletions .changeset/widget-context-catalog-load-state-5228.md
Original file line numberDiff line numberDiff line change
@@ -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.
Original file line numberDiff line numberDiff line change
Expand Up@@ -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' },
],
]),
};

/**
Expand Down
77 changes: 32 additions & 45 deletions packages/app-shell/src/views/metadata-admin/ResourceEditPage.tsx
Original file line numberDiff line numberDiff line change
Expand Up@@ -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,
Expand DownExpand Up@@ -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;
Expand DownExpand Up@@ -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
Expand DownExpand Up@@ -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
Expand All@@ -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
Expand All@@ -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<CatalogErrors>(() => {
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<WidgetContext>(
() => ({
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.
Expand Down
Original file line numberDiff line numberDiff line change
Expand Up@@ -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';

Expand DownExpand Up@@ -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']),
}}
/>,
);
Expand Down
7 changes: 7 additions & 0 deletions packages/app-shell/src/views/metadata-admin/SchemaForm.tsx
Original file line numberDiff line numberDiff line change
Expand Up@@ -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;

Expand Down
Original file line numberDiff line numberDiff line change
Expand Up@@ -63,6 +63,7 @@ import {
type RegisteredWidgetKey,
type WidgetContext,
} from './widgets';
import { loaded } from './loadState';

afterEach(cleanup);

Expand DownExpand Up@@ -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' },
Expand Down
Original file line numberDiff line numberDiff line change
Expand Up@@ -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);

Expand All@@ -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();
Expand All@@ -41,7 +42,7 @@ describe('view-ref widget', () => {
value={undefined}
onChange={() => {}}
schema={{ type: 'string' }}
context={{ objectViews: [] }}
context={{ objectViews: loaded([]) }}
/>,
);
expect(screen.getByRole('combobox')).toBeInTheDocument();
Expand All@@ -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();
Expand Down
Loading
Loading