fix(plugin-form): render a childObject config hint for a declined master-detail collection - #6374
Merged
os-support-ai merged 1 commit intoAug 25, 2026
Conversation
`MasterDetailForm` already declines to fetch the schema of a detail collection whose `childObject` never resolved (objectui#5940) and returns the entry unresolved. The render branch it then fell into read `!d.columns?.length ? <p>Loading columns…</p>`, so the author was shown a spinner-shaped message that can never end — the decline is precisely the guarantee that those columns will not arrive — and that never named the key they had to set. The `!d.childObject` case now takes its own branch, checked BEFORE the columns arm because nothing is pending: there is no first paint where "loading" is honest. Copy and structure are `LineItemsPanel`'s, which took the same branch for the same key in objectui#6194 / PR #6359; the two components had disagreed about what an author sees for the identical authoring mistake. The hint carries its own `data-testid` (`md-detail-no-child-object`). Two source comments claimed "the grid card shows a config hint". Both were false. The one at the decline is now true and records that it was not. The one at the resolver's `catch` is corrected rather than made true: an entry whose schema fetch threw DOES name a child object, so it skips the new branch and still lands on `Loading columns…`. Separating that from "still in flight" needs per-entry error state this resolver does not keep, filed as objectui#6372. Extends `MasterDetailForm.detailChildObjectDecline.test.tsx`, which covered the fetch half only, with the render half. Both directions are pinned, matching the file's existing discipline: the declined detail shows the hint and no `Loading columns…`, and a detail that names its child object shows neither the hint nor anything but `Loading columns…` while it resolves — so deleting the loading branch outright cannot pass. Claude-Session: https://claude.ai/code/session_011SfZeFWrhGLHmfq61xbz4q Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Contributor
✅ Console Performance Budget
The eager closure is every chunk the entry reaches through static imports — what the browser fetches and parses before the app renders. The entry chunk on its own is a small fraction of it. 📦 Bundle Size Report
Size Limits
|
os-support-ai
marked this pull request as ready for review
August 25, 2026 18:20
os-support-ai
deleted the
claude/issue-6360-master-detail-declined-config-hint
branch
August 25, 2026 18:32
This was referenced Aug 25, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #6360
MasterDetailFormalready declines to fetch the schema of a detail collection whosechildObjectis missing (the decline landed in #5940) and returns the entry unresolved. The render branch it then fell into read!d.columns?.length ? <p>Loading columns…</p>, so the author was shown a spinner-shaped message that can never end — the decline is precisely the guarantee that those columns will not arrive — and that never named the key they had to set.The change
packages/plugin-form/src/MasterDetailForm.tsx— the!d.childObjectcase takes its own branch, checked before the columns arm because nothing is pending: there is no first paint where "loading" is honest.Copy and structure are
LineItemsPanel.tsx:327-334's (landed by #6359 for #6194) — same key, same register, same "childObject arm ahead of the loading arm", its owndata-testid. The two components had been disagreeing about what an author sees for the identical authoring mistake, and the weaker of the two was the one that read as the precedent.The two false comments
Both claimed "the grid card shows a config hint". Following them is what cost the reporting reader a run of the component.
catch— corrected rather than made true. An entry whose schema fetch threw does name a child object, so it skips the new branch and still lands onLoading columns…. Telling that apart from "still in flight" needs per-entry error state this resolver does not keep, which is a design call and not a rider on this card. Filed as [finding]MasterDetailFormshows a permanent "Loading columns…" for a detail whose schema fetch THREW — thecatcharm the #6360 config hint does not cover #6372; the comment now points there instead of promising a hint that is not rendered.Ghost-assertion guard (mandatory, both readings)
The new assertions were run against unmodified
origin/mainsource first — with the test file's additions in place andMasterDetailForm.tsxuntouched.Before —
pnpm exec vitest run packages/plugin-form/src/MasterDetailForm.detailChildObjectDecline.test.tsx, exit 1:The failure dump confirms the defect rather than a missing test id — the rendered section was:
After — same command, exit 0:
Tests
MasterDetailForm.detailChildObjectDecline.test.tsxalready existed and covered the fetch half only (the exact data-call list, theconsole.warn). No existing assertion was weakened or removed — the file goes 2 tests → 4.The two new tests pin both directions, matching the discipline the file already states for its fetch half:
childObject, contains noLoading columns…, and still shows its section title — with the existing exact-call-list assertion re-stated so the render claim is anchored to a genuinely declined detail;Loading columns…while it resolves. Without this, deleting the loading branch outright would pass test 1.Verification
Measured on
8a8a45b1b.git status/git diff HEADwere empty at the time of every reading below, so all of them were taken on this exact tree.pnpm exec vitest run packages/plugin-form/Test Files 66 passed (66)/Tests 671 passed (671), exit 0tsc -p packages/plugin-form/tsconfig.json --noEmittsc -p packages/plugin-form/tsconfig.test.json --noEmitnode scripts/check-control-bytes.mjs✅ check-control-bytes: OK (scanned 5247 tracked text file(s); skipped 85 binary)node scripts/check-changeset-presence.mjs✅ 2 source file(s) of 1 released package(s) changed, and this change declares 1 changeset(s)node scripts/check-changeset-no-major.mjs✅ No changeset declares amajorbump.apps/consolebinding-reach / record-reach / formType-manifest +examples/schema-cataloggallery render:Test Files 4 passed (4)/Tests 612 passed (612), exit 0Dependency closure built first (
turbo run build --filter='@object-ui/plugin-form^...', 11/11 successful) — before it, this fresh worktree's typecheck reported sixTS2307 Cannot find module '@object-ui/…', which is the missing-distartifact and not a finding.Typecheck coverage is real, not assumed:
tsc -p …/tsconfig.test.json --listFileslists both edited files, so the test-side typecheck does cover the file this PR extends.Lint — a narrowed run, declared as narrowed
Repo-wide
pnpm lintis CI's run. Locally, eslint was run on the two changed files only, and the narrowing is measured rather than assumed:--format jsonoutput, not estimated.MasterDetailForm.tsx0 errors / 25 warnings; the test file 0 errors / 4 warnings.origin/main'sMasterDetailForm.tsxpiped througheslint --stdin --stdin-filename …reports 0 errors / 25 warnings — identical, so this change introduces no new finding.eslint.config.jsdeclares noprojectService,project:,parserOptionsortsconfigRootDir— type-aware linting is off, so no edit in this diff can move the verdict on a file it does not touch.Out of scope — filed for triage, not addressed here
MasterDetailFormkeys a declined detail section onundefined-<index>— no collision (the index saves it), but the entry has no identity across reorder #6371 — the declined detail's React key. The card's dispatch order describedkey={${d.childObject}-${i}}as "collapses toundefined-0", implying a duplicate-key collision; that half is not true and the finding says so —imakes the key unique by construction, so there is no collision. What is true is that the key degrades to an index-only identity wearing anundefined-prefix, which matters because the row-state array is indexed the same way.MasterDetailFormshows a permanent "Loading columns…" for a detail whose schema fetch THREW — thecatcharm the #6360 config hint does not cover #6372 — thecatcharm above, the last place in this component whereloadingcan stay true forever.Scope
Single site.
"Loading columns…"appears exactly once in repo source, at the line this PR changes — confirmed bygit grepon this branch. Not widened into a family sweep.No spec or schema change:
childObjectis alreadyREQUIREDonMasterDetailDetailConfig. This is renderer-side reporting of an authoring error the type system cannot catch, because a detail entry reaches this renderer straight off an authored JSON schema.Changeset:
.changeset/6360-master-detail-declined-detail-config-hint.md,patch,@object-ui/plugin-form.Draft, and staying draft — the PM lands it.
Generated by Claude Code
Generated by Claude Code