Skip to content

fix(plugin-form): consult a declared submitHandler before SimpleObjectForm's inline carve-out - #6406

Merged
os-support-ai merged 1 commit into
mainfrom
claude/issue-6388-simpleform-submithandler-order
Aug 25, 2026
Merged

fix(plugin-form): consult a declared submitHandler before SimpleObjectForm's inline carve-out#6406
os-support-ai merged 1 commit into
mainfrom
claude/issue-6388-simpleform-submithandler-order

Conversation

@os-support-ai

Copy link
Copy Markdown
Collaborator

Fixes#6388

ObjectFormSchema.submitHandler is documented as handing the collected values to the host instead of calling dataSource.create / dataSource.update — a seam that by construction needs no adapter of its own. SimpleObjectForm.handleSubmit nevertheless opened with the inline-fields carve-out, checked before the persistence chain, so a declared submitHandler was never reached: a host that had said it owns the write got a success signal for a write it was never asked to perform.

Re-derived on the merged tree (faa863dce), driving formType: 'simple' through ObjectForm with customFields, a submitHandler and no dataSource:

fixtureonSuccesssubmitHandler
simple + customFields + submitHandler, no dataSource10

The change

packages/plugin-form/src/ObjectForm.tsx, two edits, in the shape PR #6386 landed for the five variant renderers:

  1. the inline carve-out is gated on the absence of a declared seam — !dataSource && !schema.submitHandler && hasInlineFields;
  2. the "no submit target" refusal moved into the persistence chain, as the else if (!dataSource) branch after the submitHandler branch. It is now noSubmitTargetError() from submitTarget.ts — the family's shared refusal, whose message is the string this file already threw, verbatim — instead of a private literal. Expressing it there is also what lets TypeScript narrow dataSource for the create/edit routes with no assertion, and what routes the refusal through the container's catch, so schema.onError now learns why.

hasInlineFields vs hasInlineFieldSource — measured, and deliberately not swapped

They are not the same predicate, and the difference decides simple's behaviour:

  • limb (a) is the same fact.hasInlineFields is schema.customFields && schema.customFields.length > 0; hasInlineFieldSource's first limb is Array.isArray(customFields) && customFields.length > 0. Same shape, modulo a malformed non-array customFields carrying a length.
  • limb (b) is the divergence.hasInlineFieldSource also accepts sections whose every field is an inline runtime FormField — that is how the sectioned variants declare an inline field source, because TabbedFormSchema / SplitFormSchema / WizardFormSchema have no customFields of their own. SimpleObjectForm does not read section fields as a field source. In its sections path, section.fields builds a name-keyed map and then filterssourceFields — resolved from customFields or from the object schema — by those names, merging only visibleOn / span / colSpan overrides onto what it found. So with all-inline sections, no customFields and no adapter, the form resolves zero fields.

Adopting the shared predicate for simple while "aligning" it would therefore have widened the carve-out into a success signal for a form that collected nothing — #6300's own defect class in a narrower dress. simple keeps its own predicate; only the refusal is shared. The narrowing is not left to prose: block 3's simple BOUNDARY case pins that such a form refuses.

Ghost-assertion guard

The fix was committed first; then onlyObjectForm.tsx was reverted to faa863dce (git checkout faa863dce -- packages/plugin-form/src/ObjectForm.tsx) with the new test file left in place. The mutation was confirmed on disk by blob hash — b109875548f2… became 379c0a535c5b…, equal to the base blob — and restored with git checkout HEAD -- ... from a trap, verified by an empty git diff HEAD and a hash back at b109875548f2….

new caseblockagainst the unmodified faa863dce implementationafter the fix
simple: inline fields + a declared seam — the seam is what runs1FAILexpected "vi.fn()" to be called 1 times, but got 0 times (submitHandler)pass
simple: the seam wins over a PRESENT dataSource too (#6176)1pass, deliberately — the ordering defect was reachable only through the !dataSource carve-out, and this case is what says sopass
simple: CONTROL — no seam declared, the carve-out still fires3pass, deliberately (see below)pass
simple: BOUNDARY — sections of inline fields are NOT its field source3FAILexpected "vi.fn()" to be called 1 times, but got 0 times (onError)pass

Whole-file readings: against the base implementation Test Files 1 failed (1) / Tests 2 failed | 33 passed (35); after the fix Test Files 1 passed (1) / Tests 35 passed (35).

Degenerate control

The control fixture is simple + customFields + onSuccess, no submitHandler and no dataSource — block 3's formType 'simple': CONTROL — no seam declared, the carve-out still fires. It asserts that the carve-out still runs and that onSuccess receives the raw collected values (not an adapter result), i.e. simple's legitimate inline collector is untouched. It passes on the base tree and after the fix, deliberately: it is what makes the block 1 pin attributable to the declared seam rather than to the carve-out having been narrowed or removed.

No assertion in submitTargetRefusal.test.tsx was weakened or deleted — the diff on that file is 142 insertions, 0 deletions.

Checks

All at 3b4e72eec; heavy runs serialized through the container's shared verify lock.

  • pnpm exec vitest run packages/plugin-form/ (repo root, canonical invocation) — Test Files 70 passed (70), Tests 720 passed (720), VITEST_EXIT=0
  • pnpm --filter @object-ui/plugin-form run type-check = tsc --noEmit && tsc -p tsconfig.test.json (the second one is what covers the new test file) — TYPECHECK_EXIT=0, after building the dependency closure pnpm --filter '@object-ui/plugin-form^...' build (12 projects, Done)
  • pnpm --filter @object-ui/plugin-form run lint711 problems (0 errors, 711 warnings), LINT_EXIT=0; the warnings are pre-existing no-explicit-any across the package
  • node scripts/check-changeset-presence.mjs2 source file(s) of 1 released package(s) changed, and this change declares 1 changeset(s)
  • node scripts/check-changeset-no-major.mjsNo changeset declares a major bump
  • node scripts/check-control-bytes.mjsOK (scanned 5285 tracked text file(s))

Boundaries

The five variant renderers are untouched (correct as of PR #6386), packages/spec is untouched, and no accept set was widened — this restores a documented seam rather than admitting anything new. Changeset: patch on @object-ui/plugin-form.

Draft on purpose: the PM lands it.

Generated by Claude Code


Generated by Claude Code

…tForm's inline carve-out
SimpleObjectForm.handleSubmit opened with the inline-fields carve-out, ahead
of the persistence chain, so a declared submitHandler was never reached: a host
that owns the write got a success signal for a write it never performed
(measured onSuccess 1 / submitHandler 0). Gate the carve-out on the absence of
a declared seam and move the no-submit-target refusal into the persistence
chain after it, matching the five variant renderers and reusing the shared
refusal from submitTarget.ts.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011SfZeFWrhGLHmfq61xbz4q
@github-actions

Copy link
Copy Markdown
Contributor

✅ Console Performance Budget

MetricValueBudget
Eager closure (gzip, 52 chunks)3230.6 KB3266.6 KB
Main entry chunk (gzip)156.1 KB350 KB
Entry fileindex-BL6bc_w-.js
StatusPASS

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

PackageSizeGzipped
app-shell (consoleActionDispatch.js)0.20KB0.19KB
app-shell (index.js)11.30KB4.28KB
app-shell (runtime-config.js)18.10KB6.51KB
app-shell (types.js)0.01KB0.04KB
app-shell (urlParams.js)10.06KB3.86KB
auth (ActiveOrganizationStorage.js)25.05KB9.16KB
auth (AuthContext.js)0.31KB0.24KB
auth (AuthGuard.js)2.07KB1.00KB
auth (AuthProvider.js)40.18KB10.59KB
auth (AuthShell.js)3.49KB1.40KB
auth (ForgotPasswordForm.js)12.21KB3.45KB
auth (LoginForm.js)18.15KB5.39KB
auth (PreviewBanner.js)0.90KB0.50KB
auth (RegisterForm.js)6.65KB2.22KB
auth (SocialSignInButtons.js)9.61KB3.89KB
auth (UserMenu.js)3.41KB1.23KB
auth (auth-gate-events.js)1.29KB0.66KB
auth (authStyles.js)5.04KB1.72KB
auth (createAuthClient.js)40.21KB10.80KB
auth (createAuthenticatedFetch.js)8.46KB3.43KB
auth (index.js)3.19KB1.44KB
auth (invitation-status.js)1.22KB0.70KB
auth (org-roles.js)6.66KB2.78KB
auth (phone-identifier.js)1.11KB0.66KB
auth (types.js)0.59KB0.35KB
auth (useAuth.js)5.30KB1.02KB
auth (useWorkspaceAdminStatus.js)5.13KB2.35KB
collaboration (CommentThread.js)26.08KB7.56KB
collaboration (LiveCursors.js)3.17KB1.27KB
collaboration (PresenceAvatars.js)6.49KB2.64KB
collaboration (PresenceProvider.js)2.79KB1.13KB
collaboration (index.js)1.68KB0.73KB
collaboration (useCollaborationTranslation.js)6.05KB2.52KB
collaboration (useCommentSearch.js)1.98KB0.88KB
collaboration (useConflictResolution.js)7.75KB1.86KB
collaboration (useMentionNotifications.js)1.81KB0.68KB
collaboration (usePresence.js)6.33KB1.84KB
collaboration (useRealtimeSubscription.js)7.91KB2.01KB
components (index.js)505.84KB114.57KB
core (index.js)5.30KB2.13KB
create-plugin (index.js)10.08KB3.26KB
data-objectstack (index.js)173.18KB47.97KB
fields (index.js)238.89KB60.02KB
i18n (LocalizationContext.js)1.76KB0.96KB
i18n (currency.js)1.22KB0.64KB
i18n (fallbackInterpolation.js)6.25KB2.77KB
i18n (i18n.js)4.28KB1.75KB
i18n (index.js)3.44KB1.39KB
i18n (pickLocalized.js)7.62KB3.26KB
i18n (provider.js)26.89KB9.04KB
i18n (useDisplayLocale.js)2.85KB1.45KB
i18n (useObjectLabel.js)33.40KB8.71KB
i18n (useSafeTranslation.js)5.60KB2.33KB
layout (index.js)38.95KB10.97KB
mobile (MobileProvider.js)0.92KB0.49KB
mobile (ResponsiveContainer.js)0.94KB0.38KB
mobile (breakpoints.js)1.51KB0.70KB
mobile (createOfflineDataSource.js)5.61KB1.75KB
mobile (index.js)1.55KB0.62KB
mobile (offlineQueue.js)3.91KB1.35KB
mobile (pwa.js)0.97KB0.49KB
mobile (serviceWorker.js)1.48KB0.62KB
mobile (serviceWorkerSource.js)3.41KB1.48KB
mobile (useBreakpoint.js)1.54KB0.65KB
mobile (useGesture.js)6.96KB1.98KB
mobile (useOfflineSync.js)1.99KB0.72KB
mobile (usePullToRefresh.js)2.53KB0.85KB
mobile (useResponsive.js)0.72KB0.42KB
mobile (useResponsiveConfig.js)1.37KB0.63KB
mobile (useSpecGesture.js)4.32KB1.64KB
mobile (useTouchTarget.js)1.01KB0.54KB
permissions (MePermissionsProvider.js)9.53KB3.38KB
permissions (PermissionContext.js)0.31KB0.25KB
permissions (PermissionGuard.js)0.89KB0.45KB
permissions (PermissionProvider.js)4.64KB1.50KB
permissions (evaluator.js)5.12KB1.74KB
permissions (index.js)0.93KB0.41KB
permissions (store.js)0.91KB0.42KB
permissions (useFieldPermissions.js)1.28KB0.53KB
permissions (usePermissions.js)1.93KB0.88KB
plugin-ai (index.js)15.75KB3.80KB
plugin-calendar (index.js)46.66KB12.84KB
plugin-charts (index.js)64.66KB18.32KB
plugin-chatbot (index.js)188.60KB44.82KB
plugin-dashboard (index.js)133.46KB34.48KB
plugin-designer (index.js)211.95KB42.75KB
plugin-detail (index.js)245.10KB62.31KB
plugin-editor (index.js)2.46KB1.10KB
plugin-form (index.js)131.00KB31.91KB
plugin-gantt (index.js)164.14KB39.87KB
plugin-grid (index.js)201.79KB54.60KB
plugin-kanban (index.js)52.87KB14.57KB
plugin-list (index.js)112.63KB27.45KB
plugin-map (index.js)20.09KB6.62KB
plugin-markdown (index.js)13.72KB4.69KB
plugin-report (index.js)43.49KB11.93KB
plugin-timeline (index.js)26.70KB7.69KB
plugin-tree (index.js)9.26KB3.13KB
plugin-view (index.js)84.55KB20.74KB
providers (DataSourceProvider.js)0.75KB0.39KB
providers (MetadataProvider.js)1.37KB0.59KB
providers (ThemeProvider.js)1.90KB0.85KB
providers (UploadProvider.js)11.66KB3.50KB
providers (index.js)0.45KB0.23KB
providers (types.js)0.01KB0.04KB
react-runtime (index.js)5.62KB2.34KB
react (LazyPluginLoader.js)4.47KB1.63KB
react (SchemaRenderer.js)54.84KB18.43KB
react (data-invalidation.js)5.05KB2.08KB
react (index.js)1.35KB0.70KB
react (schema-input.js)2.32KB1.24KB
react (spec-input.js)0.20KB0.18KB
sdui-parser (codegen.js)5.41KB2.34KB
sdui-parser (dashboard-widget-options.js)3.08KB1.30KB
sdui-parser (index.js)4.93KB2.24KB
sdui-parser (input-type.js)2.84KB1.40KB
sdui-parser (parse.js)12.13KB3.65KB
sdui-parser (provenance.js)3.66KB1.82KB
sdui-parser (types.js)0.28KB0.23KB
sdui-parser (validate.js)7.54KB2.63KB
types (ai.js)0.20KB0.17KB
types (api-types.js)0.20KB0.18KB
types (app.js)2.87KB0.99KB
types (base.js)0.20KB0.18KB
types (blocks.js)0.20KB0.18KB
types (complex.js)2.74KB1.41KB
types (crud.js)0.20KB0.18KB
types (dashboard-filter-alias.js)6.23KB2.74KB
types (data-display.js)3.75KB1.85KB
types (data-protocol.js)0.20KB0.19KB
types (data.js)0.20KB0.18KB
types (designer.js)1.87KB0.85KB
types (disclosure.js)0.20KB0.18KB
types (error-code.js)1.54KB0.88KB
types (feedback.js)0.20KB0.18KB
types (field-types.js)0.20KB0.18KB
types (form.js)0.20KB0.18KB
types (http-inflight.js)8.87KB3.73KB
types (http-retry.js)4.32KB2.02KB
types (icon-key-migration.js)4.26KB1.63KB
types (index.js)4.72KB2.24KB
types (layout.js)0.20KB0.18KB
types (managed-by.js)0.19KB0.18KB
types (mobile.js)2.59KB1.31KB
types (navigation.js)0.20KB0.18KB
types (objectql.js)0.20KB0.18KB
types (overlay.js)0.20KB0.18KB
types (permissions.js)0.20KB0.18KB
types (plugin-scope.js)0.20KB0.18KB
types (record-components.js)0.20KB0.19KB
types (record-semantics.js)1.28KB0.67KB
types (registry.js)0.20KB0.18KB
types (reports.js)0.20KB0.18KB
types (spec-report.js)5.05KB1.93KB
types (spec-ui-namespace.js)0.20KB0.19KB
types (system-fields.js)3.33KB1.54KB
types (theme.js)6.28KB2.87KB
types (ui-action.js)3.40KB1.71KB
types (views.js)0.20KB0.18KB
types (widget.js)0.20KB0.18KB

Size Limits

  • ✅ Core packages should be < 50KB gzipped
  • ✅ Component packages should be < 100KB gzipped
  • ⚠️ Plugin packages should be < 150KB gzipped

@os-support-ai
os-support-ai marked this pull request as ready for review August 25, 2026 20:49
@os-support-ai
os-support-ai added this pull request to the merge queueAug 25, 2026
Merged via the queue into main with commit 43ca9d5Aug 25, 2026
28 checks passed
@os-support-ai
os-support-ai deleted the claude/issue-6388-simpleform-submithandler-order branch August 25, 2026 21:01
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

SimpleObjectForm's inline-fields carve-out bypasses a declared submitHandler — measured onSuccess 1 / submitHandler 0

1 participant

@os-support-ai