Uh oh!
There was an error while loading. Please reload this page.
fix(plugin-form): refuse a submit with no target instead of reporting success - #6386
Merged
os-support-ai merged 1 commit intoAug 25, 2026
Merged
Conversation
… success
TabbedForm, WizardForm, SplitForm, DrawerForm and ModalForm each opened
handleSubmit with `if (!dataSource) { await schema.onSuccess?.(data); return
data; }` — a success signal emitted without consulting a declared
`submitHandler` and without persisting anything.
All five now share one answer with SimpleObjectForm and the `object-form`
element gate: a form has a submit target when it has a `dataSource` or a
declared `submitHandler`; with neither, the one legitimate shape is inline
fields, whose `onSuccess` is the write. Anything else throws, reaching
`schema.onError` and rethrowing.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011SfZeFWrhGLHmfq61xbz4qContributor
✅ 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 19:29
Uh oh!
There was an error while loading. Please reload this page.
os-support-ai
deleted the
claude/issue-6300-variant-form-missing-datasource
branch
August 25, 2026 19:40
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#6300
TabbedForm,WizardForm,SplitForm,DrawerFormandModalFormeach openedhandleSubmitwith— a success signal emitted without consulting a declared
submitHandlerand without persisting anything.Re-derived measurement — both sides, not inherited
The card states a limit on its own table and asks for both sides to be re-derived. Done, on the merged tree (
2e11c8c5b), driving eachformTypethroughObjectFormwith asubmitHandlerdeclared and nodataSource:tabbedwizardsplitdrawermodalsimpleWhere the numbers differ from the card
1. The
simplecontrast is "refuses at LOAD", not "throws at submit". With nodataSourceand no inlinecustomFields,SimpleObjectForm's schema effect takes the branch it comments as "No objectName or dataSource and no inline fields — cannot proceed", and no submittable form is mounted — measuredrendered=false,onSuccess=0. Its'DataSource is required for form submission (inline mode not configured)'throw is not reached on this path. That is very likely why the #6176 dev'ssimpleleg "failed in its own harness": there was nothing to submit.This changes where the fix has to live, not whether it is needed. The five variants build their fields from
sectionsand therefore do render, so their refusal has to be at submit time.2.
SimpleObjectForm's own inline carve-out bypasses a declaredsubmitHandlertoo — measuredsimple+customFields+submitHandler+ nodataSource→onSuccess 1 / submitHandler 0. Its carve-out is checked before the seam. The triage direction ("when asubmitHandleris declared, a missingdataSourcemust never bypass it") is followed here for the five, which meanssimplenow differs from them in that one cell. Deliberately left alone in this PR —ObjectForm.tsxis outside this card's five files, and it is #6176's defect class rather than this one's. Recorded as its own card, #6388, and reported to the PM.3.
MasterDetailFormreachability, measured rather than inferred: with nodataSource, the parent form's early return reachedhandleSavedand producedtoast.success("Created"),toastError 0, and aformKeybump that remounts and clears the parent form. Exactly the described failure.The fix
One rule, in one place (
submitTarget.ts), consumed by all five containers. A form has a submit target when it has either adataSourceor a declaredsubmitHandler(the host owns the write and needs no adapter of its own). With neither, one shape is still legitimate — fields authored inline, whoseonSuccessis the write:customFields—SimpleObjectForm's own test, and the one theobject-formelement gate already keysrequiresDataSourceon, under the comment "The one escape hatch is inlinecustomFields, which is exactly whathasInlineFieldsgates on inside the component, so the two stay in step."sectionswhose fields are all inline runtimeFormFieldobjects — the sectioned variants' inline mode, since they have nocustomFieldsof their own.Everything else throws. The throw sits inside the persistence chain, as the branch taken when none of the real routes applies: that is where the fact belongs, it narrows
dataSourcefor the routes that use it with no assertion, and each container'scatchrunsschema.onErrorand rethrows — which is what turns theMasterDetailFormreading fromtoast.success("Created")into a reported failure with the form left intact.No accept set is widened and no key is added.
packages/specis untouched. #6176 / PR #6299's work on the normal path is untouched — the seam check it added is what limb-first precedence now routes through.Limb (b) is not decoration — a
customFields-only rule broke this repo's own published examplepackages/plugin-form/README.mddocuments and ships aWizardFormelement rendered withschema={wizard}and nodataSourceprop, annotated// dataSource omitted: every step lists inline fieldsand introduced by "The inline shape is what lets a wizard run with no data source at all". Its inline fields live insections[].fields, notcustomFields. Measured against an intermediatecustomFields-only version of this guard:That is the over-broad refusal the card warned about, caught before it shipped. Limb (b) is all-or-nothing on purpose — it keeps
normalizeSectionField's own taxonomy, where only shape (3) is self-describing and shapes (1)/(2) name fields that only an object schema can resolve. One bare name anywhere means the form did need metadata it could not get, and it refuses.Ghost-assertion guard — every new assertion captured failing, then passing
Ablation: the five variant files reverted to unmodified
origin/mainbytes —git checkout 2e11c8c5b --against the five variant paths — with the test file left as written; measured, then restored. Both legs proved on disk by blob hash rather than by exit code, and the restore leg proved by an emptygit diff HEAD. No rebuild is involved: these tests import the containers by relative path and run from source under the root vitest config.BEFORE — unmodified
origin/mainsource:Tests 18 failed | 13 passed (31)·VITEST_EXIT=1. Every failure readsAssertionError: expected "vi.fn()" to be called 1 times, but got 0 times— the false-success signature:submitHandler,onErrorandtoast.errorall silent.AFTER — with the fix:
Test Files 1 passed (1)·Tests 31 passed (31)·VITEST_EXIT=0.Which fixture is which
The 13 that pass in both readings are deliberately the non-discriminating half — they are what makes the 18 failures attributable to the absence of a submit target rather than to a blanket change:
dataSourcepresent,createreally called (5 cases)drawer/modal,customFieldsonly (2 cases)MasterDetailFormwith adataSource, batch commits, save confirmed (1 case)MasterDetailFormwith no adapter: no success toast, no reset (1)All six paths named in the dispatch are pinned: the five variants, and the
MasterDetailFormreachability path.Verification
All readings below are from the tree of the pushed commit
c33ddf2fa(verified at3107b210f, whose tree68cbc1a34is byte-identical — same tree, collapsed history), after mergingorigin/main(631d81dbf) and rebuilding the dependency closure.pnpm --filter @object-ui/plugin-form type-checkTYPECHECK_EXIT=0pnpm --filter @object-ui/plugin-form lint✖ 690 problems (0 errors, 690 warnings)·LINT_EXIT=0pnpm exec vitest run packages/plugin-form/Test Files 68 passed (68)·Tests 705 passed (705)·PKGTESTS_EXIT=0Test Files 53 passed (53)·Tests 745 passed (745)·DOWNSTREAM_EXIT=0node scripts/check-changeset-presence.mjs✅ 7 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 a major bumpThe
type-checkscript's second project (tsconfig.test.json) demonstrably compiles the new test file —tsc -p tsconfig.test.json --listFiles | grep -c submitTargetRefusal.test.tsx→1— so "typecheck is clean" is a statement about the new assertions too, not only aboutsrc.Declared narrowing: the repo-wide
pnpm lint(turbo run lint) was narrowed to the one package whose files changed. Evidence, all three parts: (1) the population is eslint's own —eslint .over the package, not a hand-picked file list; (2) the count is read from--format json: 98 files, 0 errors, 690 warnings; (3)eslint.config.jsdeclares noparserOptions.project/projectService, so linting is not type-aware and this diff cannot move a verdict in any file it does not touch. CI runs the full farm regardless.Notes for the PM
packages/specuntouched, as instructed. No spec note is needed:ObjectFormSchema.customFieldsalready glosses itself as "when used with inline field definitions (without dataSource), this becomes the primary field source", which is the contract this restores rather than changes.SimpleObjectForm's inline-fields carve-out bypasses a declaredsubmitHandler— measured onSuccess 1 / submitHandler 0 #6388 —SimpleObjectForm's inline carve-out skips a declaredsubmitHandler. It remains open and this branch does not touchObjectForm.tsx.Generated by Claude Code