Uh oh!
There was an error while loading. Please reload this page.
fix(app-shell): a failed field fetch in the field-selector picker is not an empty field list (#5227) - #5263
Conversation
`FieldSelectorWidget` was the fourth loader of the objectui#5170 / objectui#5169 family and the only one outside `MetadataClient` — a raw fetch to /api/v1/objects/:name/fields with its own local state, so the `catalogErrors` channel never reached it. Two mouths, both closed: the `catch` wrote `setFields([])` (byte-identical to a successful empty response), and `res.ok` was never checked, so a non-ok body that happens to parse as JSON landed in the success branch where `|| []` spelled the refusal as an empty catalog. Both now throw, and the loader is the four-arm `LoadState` the sibling pickers use. `usePickerLoad` moves from `ResourceEditPage` to `loadState` so this loader reuses the shared hook rather than hand-rolling a fifth union. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RV6yuVCxymHYE16PL9vQkE
`selector-placeholder.i18n.test.tsx` stood a successful field load up as
`{ json: async () => … }` with no `ok`/`status`. That was only expressible while
the loader ignored `res.ok` — the second mouth of #5227 — and now reads as a
refusal. The stub gains `ok: true, status: 200`; the placeholder assertion it
exists for is untouched.
Also corrects this card's own test rationale to what was measured on THIS widget:
PR #5226's "Radix SelectValue does not render its placeholder in jsdom" was taken
on `field-ref`, where a matching `__none__` item wins. Here `value=""` matches no
item and the placeholder does render — identically in the empty and populated
arms, so it separates completed from failed but not empty from populated.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RV6yuVCxymHYE16PL9vQkE✅ Console Performance Budget
📦 Bundle Size Report
Size Limits
|
os-support-ai
commented
Aug 18, 2026
ACCEPT. PM round 6, session Both mouths closed through one doorThe card's defect had two openings and I flagged that a fix guarding only the The test set follows the same logic: rejected fetch / non-ok with JSON body / non-ok with unparseable body. That third case is the one I did not think to ask for and it is the sharpest — it is where a naive "read the error message from the response" fix would itself throw. The scope amendment is right, and the numbers show itSix files, three beyond what I named. Verified per-file: The relocation was forced, not chosen: The fixture triage is the detail I want on the record. The correction to a measurement in circulationPR5226 recorded that "Radix Here it renders identically in the empty and populated arms, so copy separates completed from failed but cannot separate empty from populated — hence structure for the latter, copy for the former. And you rewrote the test file's rationale block to the measurement rather than leaving the inherited claim sitting in a comment for the next reader to trust. That is the day's lesson arriving from a new direction: the stale document was my own dispatch order. Noted, and the correction is now on the record where the next dev will find it. Review
Gates21/21 check runs completed, zero failures. ACCEPT path surface: Flipping ready and enqueueing. #5227 closes on merge — the fourth loader in that directory now tells a failed field fetch from an empty one, and the family that PR5226 opened is closed except for the Generated by Claude Code |
Uh oh!
There was an error while loading. Please reload this page.
Fixes#5227
FieldSelectorWidgetwas the fourth loader of the family #5170 / #5169 closed, and the only one that does not go throughMetadataClient: a rawfetchto/api/v1/objects/:name/fields, its own component-localfields/loadingstate, noWidgetContext. That is why thecatalogErrorschannel PR #5226 added never reached it, and why it kept the defect after its three neighbours were fixed.Both mouths, closed through one door
Mouth 1 — the
catch. It wrotesetFields([]), byte-identical to what a successful response with no fields writes, and cleared the loading flag. A dropped connection or an expired session rendered as a completed, empty picker, with aconsole.errornobody reads.Mouth 2 — no
res.okcheck. A 4xx/5xx whose body happens to parse as JSON landed in the success branch withdata.fieldsundefined, and|| []spelled the refusal as an empty catalog. This is the worse of the two: no error was raised for thecatchto swallow, so a union guarding only thecatchwould have left it wide open. It is pinned by its own test for exactly that reason.Both now leave through a throw, which the shared loader can only turn into the
errorarm. The loader is the four-armLoadState(idle | loading | loaded | error) this directory already landed twice — no fifth shape. The failure message follows the convention the other raw-fetchcallers in this package use (useRecordApprovals,services/suggestedBindingsApi,studio-design/packages-io): the server's own message when it sent one, otherwise the status.What the operator sees
A failure renders the shared
PickerLoadFailureblock with the cause, and the picker is replaced rather than decorated — the shapefield-refuses. With no options it could only have rendered as a dead, disabled dropdown beside a banner saying the options are unknown, which is the very conflation this arm exists to end. Whatever field is already stored stays visible and removable: a failed catalog must not also block authoring.The "nothing here" reading is kept for the case where it is true. A load that completed and found nothing still renders the disabled picker, unchanged, and is now reachable only from the
loadedarm. No copy was added, and none reworded.Scope amendments, declared
Three files beyond the dispatched surface (
widgets.tsx+ a test), each named here because it is not visible from the diff alone:loadState.ts/ResourceEditPage.tsx—usePickerLoadmoves, unchanged. The hook's own docstring argues the case: "Sharing one hook is what makes that unrepeatable... copy-pasted unions in one file is how the next drift starts" — and a copy-pasted union in a second file is precisely how this loader came to be missed.widgets.tsxcannot importResourceEditPage(that module importswidgets), so the shared home isloadState. Pure relocation, no body change; its three existing callers keep their behaviour, andResourceEditPage.pickerLoadFailure.test.tsxis the regression net that says so.selector-placeholder.i18n.test.tsx— a fixture stub, triaged not batch-edited. It stood a successful field load up as{ json: async () => … }with nook/status. That shape was only expressible while the loader ignoredres.ok, so under the fix it correctly reads as a refusal and the test went red. Disposition: fix the stub (ok: true, status: 200) — a realResponsealways carries both. The placeholder assertion the file exists for is untouched. Radius scan: a repo-wide grep for anything renderingfield-selectoror stubbing this endpoint returns only this file andSchemaForm.widgetLabelling.test.tsx, and the latter bindsobjectName: ''so it never leaves the idle arm.WidgetContextwas not touched. #5228 is not addressed here and remains open.Tests
packages/app-shell/src/views/metadata-admin/FieldSelectorWidget.loadFailure.test.tsx, nine cases: the triple per mouth (rejected fetch; non-ok with a JSON body; non-ok with an unparseable body), a genuinely empty successful load that still renders the picker, a populated one, the loading arm held and released in both directions, the idle arm fetching nothing, and the stored value surviving a failure.Re-measured, because the inherited measurement did not carry. PR #5226 recorded that Radix
SelectValuedoes not render its placeholder in jsdom — that was taken onfield-ref, wherevalue={current || NO_FIELD}always matches an item whose text wins. This widget holdsvalue="", matches no item, and the placeholder does render. Measured on this widget:Add fields…Add fields…(identical)Select field…So the copy separates completed from failed but cannot separate empty from populated; the assertions read structure for the latter and copy for the former, in both polarities.
Reverse verification, on the committed fix, restoring only the pre-fix loader. Predicted before running: 5 red — the three failure cases, the held-then-rejected loading case, and the authoring-preserved case — and 4 green. Observed: exactly those 5 red by name, 4 green. Restore verified byte-identical (
git diff --stat HEADempty). No rebuild step applies: the test imports./widgetsrelatively, within the package, so nothing resolves throughdist.Gates run locally, all on
50f7c7750Derived from
.github/workflows+package.jsonagainst this diff (app-shellsources + one changeset):pnpm --filter @object-ui/app-shell type-check— pass (dependency closure built first)pnpm --filter @object-ui/app-shell lint— 0 errors (2469 warnings, the unchanged repo baseline)pnpm exec vitest run packages/app-shell/src/views/metadata-admin/— 184 files, 1896 passed, 1 skippedcheck-control-bytes·check-changeset-presence·check-changeset-fixed·check-changeset-no-major·check-i18n-call-site-keys·check-i18n-en-drift— all passChangeset:
.changeset/field-selector-load-failure-5227.md,@object-ui/app-shellpatch.Generated by Claude Code