Uh oh!
There was an error while loading. Please reload this page.
fix(plugin-form): decline to fetch a line-items child schema with no childObject - #6196
Merged
yinlianghui-tw merged 1 commit intoAug 25, 2026
Merged
Conversation
…childObject `record:line_items` read `schema.childObject` and handed it to `getObjectSchema` with a guard on the data source but none on the argument, so a panel authored without the key asked the backend for an object literally named `undefined`. The effect's `.catch` turned the answer into a null child schema, so the visible outcome was a silently unsanitized child grid rather than an error — which is why no assertion on resulting state could have caught it and the new test asserts the call list instead. The effect now declines and warns, naming the key and what to set it to, and clears the cached child schema so a later save is never sanitized against a previous object's fields. Matches the choice `RelatedList` makes for the same class of missing key and the one `object-master-detail-form` makes on this exact key. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019b5UBNMtTzKbVtZZGvFuxe
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
|
yinlianghui-tw
marked this pull request as ready for review
August 25, 2026 00:13
yinlianghui-tw
enabled auto-merge
August 25, 2026 00:14
Uh oh!
There was an error while loading. Please reload this page.
yinlianghui-tw
deleted the
claude/issue-6188-line-items-decline-to-fetch
branch
August 25, 2026 00:33
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 freeto 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#6188
The measurement this card was for, first
The card explicitly did not verify reachability. Both halves are now measured, and the answer is the stronger reading, not the weaker one:
Is
childObjectdeclared a required input forrecord:line_items? Yes —packages/plugin-form/src/index.tsx:404,{ name: 'childObject', type: 'string', label: 'Child Object', required: true }.Is that declaration enforced anywhere? No.
inputs[].requiredis designer metadata:WidgetRegistry.ts:180copies it onto theComponentRegistryentry and nothing parses an authored node against it. Grepped for every consumer of.requiredunderpackages/core/src— the hits are field-level rules (evaluator/fieldRules.ts), the record-level validators, and the schema builder; none of them sees a block's declared inputs.Does the spec's
RecordLineItemsPropsmake it required? There is noRecordLineItemsProps. Grep over@objectstack/spec@17.2.0(srcanddist) and over this whole repo returns zero hits. The spec names this block exactly once, as an example of the opposite: "typeis an open union (unregistered types likerecord:line_itemsare authored in the wild)" (src/ui/component.zod.ts:105). So the block has no spec schema at all, and the registry'srequired: trueis the only place the requirement is written down.Does a bare mount reproduce the call? Yes — through the registered authoring surface, not only through direct component use.
packages/plugin-form/src/LineItemsPanel.childObjectDecline.test.tsxrenders a plain authored node throughSchemaRenderer(so it goes throughComponentRegistryand theElementDataSourceGatewrapper, the real path), withchildObjectsimply absent. Recorded calls, by first argument, against unmodifiedorigin/main@b04646b:So the severity claim stands at its stronger reading: an author who omits
childObjecttriggers this. Nothing between the authored JSON and the effect rejects the node — the key is declared required in a place that only a designer UI reads.The fix
LineItemsPanel's child-schema effect guarded the data source (ds, and thatgetObjectSchemais callable) but never the argument. It now declines and warns.Following PR #6191 (card #5940), which landed the same decision on the same key in this package, with both of its deliberate choices checked against this component rather than copied:
LineItemsPanelis one panel with onechildSchemastate, so the analogous question is what that state should hold when the fetch is declined, and the answer isnull: an unresolvable panel has no child schema, and leaving a previous object's schema cached would sanitize a later save against the wrong object's fields.nullis also what the effect's own.catchalready produces, so the sanitize path needs no new case.The precedent it joins:
RelatedListdeclines when it cannot scope ("has no referenceField/parentId — refusing to fetch all rows",RelatedList.tsx:498),MasterDetailForm's child-schema cache has always spelled it.filter(Boolean), and after #6191 its detail-resolve effect declines on this exact key.Why the test asserts the call list
The symptom here is quieter than #5940's, and that shaped the test. The
.catchturns whatever a real backend returns for an object namedundefinedinto a null child schema — and a null child schema is exactly what the panel holds before any fetch resolves. So "it did not crash" and "the schema is null" were both true while the defect was live. No assertion on resulting state could have caught this. Only the calls distinguish the two worlds.Both directions are pinned, because a guard that declined everything would also make the bad call vanish and would pass an absence-only assertion:
childObjectunset — before['getObjectSchema(undefined)', 'find(undefined)']childObjectunset — after['find(undefined)'](see below)childObject: 'invoice_line'— after['getObjectSchema("invoice_line")', 'find("invoice_line")']The recorder deliberately captures
method(firstArgument)rather than the full argument list: the child fetch also carries$filter/$top, and pinning those would make the file fail for changes that have nothing to do with the object name.The entry still in that list — a further site, filed not fixed
find(undefined)is the sibling site in this same component:load()guardsdataSourceandparentIdbut notschema.childObject, so the row fetch still queries an object literally namedundefined. It is out of this card's scope by dispatch order, which scoped this to thegetObjectSchemacall and said to file any further unguarded sub-key site. Filed as #6194, which stays open — it is not addressed here.It is pinned in the test as current behaviour, with a comment pointing at #6194 and saying what to change the line to, rather than hidden behind a narrower fixture that omits
parentId. Hiding it would have made this PR's test read as if the component were clean.That card is not a one-line rider, which is the other reason it is separate: declining a schema fetch has no visible consequence, but declining the row fetch does —
loadownsloading, and the render branchesloading ? … : !parentId ? … : grid, so an unresolvable panel with a parent id bound would land on the grid branch and show an empty editable grid over an object that does not exist. Whether that wants its own config-hint branch is a question, not a mechanical fix.Lint note
The new
setChildSchema(null)raises onereact-hooks/set-state-in-effectwarning. It is the same warning the effect below it already carries (void load(), line 203), the rule is configured'warn'repo-wide (eslint.config.js:95), andlint.ymldeliberately does not set--max-warnings("warnings repo-wide run into the…", line 20). Package lint is 0 errors.Verification — all at
fd7525d9b, the commit this PR shipsExit codes captured by redirect before any pipe. Heavy runs went through the shared verify lock.
pnpm exec vitest run packages/plugin-form/src/LineItemsPanel.childObjectDecline.test.tsxTests 1 failed | 1 passed (2)— reproduced, exactly as predictedpnpm exec vitest run packages/plugin-form/ apps/console/src/__tests__/record-block-record-reach.test.tsx apps/console/src/__tests__/public-block-binding-reach.test.tsx apps/console/src/__tests__/public-contract.test.tsTest Files 64 passed (64)/Tests 655 passed (655)pnpm exec turbo run type-check --filter=@object-ui/plugin-form --concurrency=2Tasks: 13 successful, 13 total(includes the dependency build andtsconfig.test.json)eslint .inpackages/plugin-form641 problems (0 errors, 641 warnings)node scripts/check-control-bytes.mjsOK (scanned 5112 tracked text file(s))pnpm run changeset:checkHow the gate scope was derived — from each workflow's own configuration, not assumed.
ci.ymlandlint.ymlcarrypaths-ignorefor**/*.md/content/docs/.changesetonly, so apackages/**diff runs both in full;control-bytes.ymlandchangeset-presence.ymldeliberately carry no path filter at all;changeset-guard.ymlfilters to.changeset/**, which this PR touches.performance-budget.yml(Bundle Analysis) filters onpackages/**and so applies — it is a full turbo build plus a console build, a repo-level run CI owns; not reproduced locally.Why the lint narrowing is a measurement, not an omission: the universe comes from eslint's own resolution (
eslint .from the package directory, which is whatturbo run lintinvokes for this package —packages/plugin-form/package.json"lint": "eslint ."), and the rooteslint.config.jsdeclares noproject/projectService, so type-aware linting is off and this diff cannot move the verdict on any file it does not touch. Files outsidepackages/plugin-formare therefore invariant under this change.Docs are untouched, matching #6191: this is a guard on an existing read path, not a change to the authorable surface any document describes.
Generated by Claude Code
Generated by Claude Code