Uh oh!
There was an error while loading. Please reload this page.
fix(plugin-form): honour the declared submitHandler seam in every form variant - #6299
Merged
Merged
Conversation
…m variant `ObjectFormSchema.submitHandler` is documented as the seam a host uses to own persistence: the form validates and hands the collected values over INSTEAD of calling dataSource.create / dataSource.update. `ObjectForm` forwards the key into every variant it routes to, but only `SimpleObjectForm` ever read it. TabbedForm, WizardForm, SplitForm, DrawerForm and ModalForm called dataSource.create directly. MasterDetailForm supplies `submitHandler: submitViaBatch` so the parent and its child collections commit as ONE batchTransaction (#2679 / ADR-0034 item 4). With the parent half rendered `tabbed`, the measured reading was `batchTransaction 0 / create 1` with args ["po", {"ref":"PO-1"}] — and the child leg was never attempted at all: the parent committed alone, the entered line items were discarded, no compensation ran, and a success toast confirmed it. `split` measured identically. Each variant now checks `schema.submitHandler` first, with the same precedence SimpleObjectForm uses, and declares the key on its own schema interface. The write payload is hoisted to one `writePayload` per handler so the host-owned route and the direct route cannot diverge. WizardForm additionally guards its default success arms with `!schema.submitHandler`, mirroring ObjectForm, so a host that owns the write also owns the outcome. The `object-master-detail-form.formType` vocabulary is deliberately unchanged and stays `simple | tabbed`. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Mn4BZ5AVDM81pvfij1WwM9
…ence, add changeset The five variant schemas restated `submitHandler`'s signature verbatim. Declaring it as `ObjectFormSchema['submitHandler']` instead makes the variant key and the canonical key `ObjectForm` forwards provably the same type — they cannot drift, and the restated `any`s (3 per site) stop being new lint warnings. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Mn4BZ5AVDM81pvfij1WwM9
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-litant
marked this pull request as ready for review
August 25, 2026 10:54
Uh oh!
There was an error while loading. Please reload this page.
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#6176
ObjectFormSchema.submitHandleris documented as the seam a host uses to own persistence — "the form validates and hands the collected values to the host INSTEAD of callingdataSource.create/dataSource.update".ObjectFormforwards the key into every variant it routes to (the{...schema}spread carries it), but onlySimpleObjectFormever read it.TabbedForm,WizardForm,SplitForm,DrawerFormandModalFormpersisted directly instead. This is a declared-vs-enforced restoration against ADR-0034 item 4 / #2679, which chose the atomic batch.Premise reproduced before any source was touched
Measured on
origin/main@c456d91f4, master-detail parent with two sections and one detail collection, driving the master-detail's own bottom Save bar. Counts are calls on the same stubdataSource:formTypebatchTransactiondataSource.createsimpletabbed["po", {"ref":"PO-1"}]wizardNextsplit["po", {"ref":"PO-1"}]drawer/modalThat is the card's measurement, including its observed argument, reproduced exactly.
0 / 0on every row —simpleincluded — because it clicked before the form had mounted. Thesimplepositive control is what caught it; the numbers above are from the corrected probe.The consequence is worse than "the batch is bypassed"
With
formType: tabbed(andsplit), the child leg is never attempted at all. Measured on the emulated path (adataSourcewith nobatchTransaction, whose child create fails):simple(onmain)tabbed/split(onmain)po,po_linepoonlydelete('po','po1')So the parent commits alone, the entered line items are silently discarded, nothing rolls back, and the save is confirmed as successful.
The fix
Each of the five variants now checks
schema.submitHandlerfirst, with the same precedenceSimpleObjectFormuses, and declares the key on its own schema interface. Two supporting choices:writePayloadper handler. The create-modeomitServerResolvedDefaultscall was hoisted so the host-owned route and the direct route cannot write different payloads. On the non-handler path the value is provably identical (in the edit branchmode === 'edit', sowritePayload === data).submitHandler?: ObjectFormSchema['submitHandler']rather than a copied signature, so the variant key and the canonical key can never drift. (This also removed the 15no-explicit-anywarnings the restated version introduced.)WizardFormadditionally guards its default success arms with!schema.submitHandler, mirroringObjectForm, so a host that owns the write also owns the outcome instead of double-confirming.Evidence
Per-point ablation — five repair points, each reverted alone to
c456d91f4, with the mutation proven on disk each leg (seam marker0in the ablated file,1in the other four) and restoration proven the same way. Restore ran undertrap … EXIT INT TERM;git diff HEAD --statwas empty afterwards.TabbedFormtabbedin all three blocksSplitFormsplitin all three blocks + the vocabulary file'ssplitcaseWizardFormwizardseam pinDrawerFormdrawerseam pinModalFormmodalseam pinsimplestayed green in every leg — the control that keeps the negative assertions from passing vacuously.TabbedFormleg: the entire pre-existing vocabulary file stayed green, including itstabbedcase. That case asserts only presentation, so it passes in both the broken and the fixed world — it was never an instrument for this defect. The new pins are.New file
submitHandlerSeam.test.tsx(12 cases) pins, in order of what matters:simple/tabbed/split, with a positive probe that the child leg was actually attempted, so the rollback assertion cannot pass on a form that never submitted;ObjectFormso the forwarding path is the real one — positive (handler received the values) and negative (create/updateuntouched);batchTransactioncarrying both legs, zero independent creates.Vocabulary deliberately unchanged — and one open question
The
object-master-detail-form.formTypevocabulary stayssimple | tabbed. Narrowing or widening it is a contract change and is not this card's to make.masterDetailFormTypeVocabulary.test.tsxexcludedsplitbecause it "renders inline but persists AROUND the atomic batch". The second half is no longer true —splitnow saves through the batch likesimple. That case is updated to pin the new true reading and to record that the persistence reason for excludingsplitis spent. Whethersplitshould now be admitted to the vocabulary is a contract question left open for triage — deliberately not decided here.Gates
Run from the repo root at
403488be2(the final commit):vitest run packages/plugin-form/plugin-form type-checkplugin-form lint0 errors, 673 warnings(basec456d91f4=0 errors, 663 warnings; the +10 areas anystubs in the new test file — zero new warnings in the five source files)check-changeset-presencecheck-changeset-no-major/-fixedmajor; all packages in the fixed groupcheck-control-bytescheck-vi-mock-specifiersConsumer sweep, downstream direction (dependents of
plugin-form):plugin-designer+plugin-view34 files / 297 tests;app-shell18 files / 262 tests;apps/console8 files / 203 tests — all passed.packages/app-shellsuite hit the container's ~10-minute foreground cap (exit 143, no failures in the partial log) and was narrowed to the 18 files that reachplugin-form. The narrowing rests on a measured invariance, not a guess: no code outsidepackages/plugin-formsupplies asubmitHandleranywhere inpackages/orapps/, so the new behavioural arm is unreachable from every consumer; the only reachable change is thewritePayloadhoist, which is value-identical. CI runs the full farm regardless.Generated by Claude Code
Generated by Claude Code