fix(plugin-form): decline the row load and save of a line-items panel with no childObject - #6359
Merged
Merged
Conversation
… with no childObject
`LineItemsPanel` read `schema.childObject` at two sites. objectui#6188 guarded
the child-schema fetch; the row load still asked the data layer to `find` an
object literally named `undefined`, scoped by `{ [relationshipField]: parentId }`.
Declining that fetch is not safe on its own. `load` owns `loading`, and the
panel branched `loading ? "Loading…" : !parentId ? "Save the record first…" :
<grid>`, so a declining panel with a parent id bound fell to the third branch
and showed an empty EDITABLE grid with an Add button over an object that does
not exist. Measured on the pre-fix component: one keystroke in the grid's
always-present ghost row materialised a row, enabling Save, which reached
`batchTransaction([{ object: undefined, action: 'create', … }])` — so the bad
read was one keystroke from a bad write.
An unresolvable panel now gets its own render branch, a config hint naming the
key, checked ahead of `loading` because nothing is pending. `save` takes the
same one-line guard for the route the render branch cannot close: a schema
edited to drop `childObject` while rows are already dirty.
The pinned expectation in `LineItemsPanel.childObjectDecline.test.tsx` is
flipped to `toEqual([])` per its own annotation, and the render branch and the
closed save path are pinned alongside it.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012CZgmFFzqA9cX8tBMhvpFe
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
|
This was referenced Aug 25, 2026
os-warren
marked this pull request as ready for review
August 25, 2026 15:38
akarma-synetal
pushed a commit
to akarma-synetal/objectui
that referenced
this pull request
Sep 1, 2026
…ail (objectstack-ai#6374) `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 objectstack-ai#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 <noreply@anthropic.com>
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 #6194
The second of
LineItemsPanel's two reads ofschema.childObject. #6188 guarded the first (the child-schema cache); the row load still asked the data layer tofindan object literally namedundefined, scoped by{ [relationshipField]: parentId }.Re-anchored on my own base
ed859aa35— the card cites:133-137, the site was at:175.The judgement this card existed for
Declining the fetch is not safe on its own, and the card was right to make that its real question.
loadownsloading, and the panel branchedloading ? "Loading…" : !parentId ? "Save the record first…" :the line-items grid element. The moment the row fetch declines, an unresolvable panel with aparentIdbound falls to the third branch: an empty editable grid with an Add button, over an object that does not exist.I landed on the config-hint branch — triage's default, and it is load-bearing rather than cosmetic. An unresolvable panel now renders its own branch naming the key, checked ahead of
loadingbecause nothing is pending: the schema itself already says the panel can never resolve, so there is no honest moment at which "Loading…" is true.loadingis never left true to hide the state.Two precedents, and I followed the shape of the first while deliberately not copying its copy:
object-master-detail-formissuesgetObjectSchema(undefined)for a detail collection whose object it never resolved, instead of declining to fetch #5940 /object-master-detail-form, which declines on this exact key and whose comment says "the grid card shows a config hint".AdvancedChartImpl's refusal placeholders — "This chart cannot plot its category axis: no row has axfield" — a block that says what it cannot do instead of painting an empty frame.The save path — measured, not assumed, and the card's open question closes
The card left this explicitly unmeasured: "The Save button is disabled until the grid is dirty, so reaching it requires editing rows of the nonexistent object first — plausible but not measured."
Measured on the pre-fix component, this fixture,
childObjectunset:So it was reachable, and the hedge was too generous: it took one keystroke, not a deliberate campaign of "editing rows of the nonexistent object" — and it reached a write, through the very affordance the empty grid advertises ("No items yet — click Add to begin"). The bad read was one keystroke from a bad write.
Removing the grid removes the only producer of
dirty, which is the only thing that enables Save — so the render branch closes that route structurally. I folded the remaining one-line guard intosaverather than filing it, on the PM's stated condition ("fold it only if it is the same one-line guard on the same code path — argue it"). The argument: it is one token on the existingif (!dataSource || !parentId) return;at the component's other data-layer entry point, the same defect class, the same gate family, no new verification surface. Depending on a render branch to keep an undefined object name out of the data layer is the same reasoning that left this second site unguarded in the first place; a write contract is not the render tree's to keep. It also covers the one route the render branch cannot — a schema edited to dropchildObjectwhile rows are already dirty (a live designer edit), which is what the third new test exercises.The pin: flipped, not deleted
Per its own annotation. Red against the old expectation, green against the new one — both on this branch:
It is now
expect(calls).toEqual([]), still an exact-list assertion — the only shape that catches a third unguarded read of this key, which is how the second one arrived. Three tests added alongside it, each keeping a direction honest:childObjectfrom a panel with dirty rows leaves Save enabled and still writes nothing — the only test that exercises thesaveguard.Gates
Run on the committed head
69c7e6cc5, exit codes captured by redirect before any pipe, each quoted from the gate's own verdict line.vitest run packages/plugin-form/Test Files 65 passed (65)/Tests 653 passed (653)plugin-form run type-checktsc --noEmit && tsc -p tsconfig.test.json)plugin-form run lint674 problems (0 errors, 674 warnings)— 94 files, 0 errors; every warning pre-existing, none on a line this diff addscheck-changeset-presence.mjs✅ 2 source file(s) of 1 released package(s) changed, and this change declares 1 changeset(s)check-changeset-no-major.mjs✅ No changeset declares a major bump.check-changeset-fixed.mjs✅ All workspace packages are in the changeset fixed group.check-control-bytes.mjs✅ OK (scanned 5230 tracked text file(s); skipped 85 binary)check-entry-guard.mjscheck-phantom-dependencies.mjs✅ Every in-scope import is declared by the package that publishes it.check-vi-mock-specifiers.mjs✅ OKcheck-i18n-call-site-keys/-en-drift/-dead-keysNo en value changed in this range.check-shell-escape-residue.mjs✅ OKtypecheck covers the editis asserted, not assumed:tsc --listFilesputsLineItemsPanel.tsxin the main project (1 hit) andLineItemsPanel.childObjectDecline.test.tsxin the test project (1 hit, correctly 0 in the main one).Declared narrowing — repo-wide
pnpm lint(turbo run lint). I ran@object-ui/plugin-form's own leg in full, which is the exact command turbo runs for this package, and not the other packages' legs. Three readings make that a measurement rather than a skip: ① the population comes from eslint's own config (files: ['**/*.{ts,tsx}']) over the whole package, not from my guess about which files count; ② the file count is--format json's own — 94 files, 0 errors, 674 warnings, matching the human-format verdict line; ③eslint.config.jssets noparserOptions.project, noprojectServiceand no type-checked preset, so linting is not type-aware and this diff cannot move the verdict of any file it does not touch — in this package or another. CI runs the full farm regardless.Not measured, and not failures.
check-readme-exports.mjs(exit 1) andcheck-eager-closure-budget.mjs(exit 2) both report a missing build artifact — "its type entry./dist/index.d.tsis not on disk — runpnpm buildfirst" and "This is a broken gauge, not a passing budget" — andscripts/pm/check-half-states.mjsexits 3 with "it is no reading at all" (the container holds the proxy placeholder, not a GitHub credential). None is implicated by this diff: no README, no export, no console bundle, no board sweep. Recorded as prerequisite-not-met rather than as passes.Out-of-scope finding, filed not fixed
While reading #5940's precedent I found that
MasterDetailForm's own "config hint" branch renders the string "Loading columns…" for a detail whosechildObjectnever resolved — a spinner-shaped message that can never stop being wrong, since the decline above it guarantees the columns never arrive. That is the exact outcome this card told me not to reproduce here, which is why this PR follows the shape of #5940's precedent and not its copy. Filed separately; not touched in this PR.Scope
packages/plugin-form/**only, as dispatched — nothing near.npmrcor workflow images. No edit tocontent/docs/releases/.Generated by Claude Code