Uh oh!
There was an error while loading. Please reload this page.
refactor(plugin-form): retire the form-view section className/gridClassName reads - #7198
Merged
Merged
Conversation
…ssName reads The renderer reached `className` / `gridClassName` off the parsed form view through `as any` at seven sites, while `@objectstack/spec` deliberately does not declare either key on the form-view/section authorable surface — the boundary declared on one side and crossed on the other. Per the maintainer ruling of 2026-09-01 (objectstack#13626, director decision batch C): retire the reads. Declaring the keys was weighed and not adopted — it would formally invite free Tailwind strings into authored metadata, the exact class the boundary exists to keep out. Six sites in ObjectForm (the tabbed/wizard/split/drawer/modal section maps and the stacked section-divider) and one in DrawerForm (its own divider) stop copying the keys. A behavioural pin covers all seven arms; it is behavioural rather than a source grep because `ObjectFormSection` still declares both keys, so a plain uncast read would type-check and silently restore consumption.
…hangeset The README stated the renderer reads a section `className` "explicitly on exactly one pseudo-field, `type: 'section-divider'`" — the read this branch retires, so the sentence is now false. It is corrected in place and a new "Section styling is not authorable" section states the boundary, why declaring the keys was rejected, and what still works (the form ROOT `className`, section `columns`, host CSS). The changeset declares the behaviour narrowing. `minor`, not `major`, per this repo's version-alignment rule; the breaking semantics are spelled out in the body.
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 Sep 1, 2026
os-support-ai
marked this pull request as ready for review
September 1, 2026 10:51
os-support-ai
enabled auto-merge
September 1, 2026 10:51
Uh oh!
There was an error while loading. Please reload this page.
os-support-ai
deleted the
claude/issue-13626-formview-classname-reads
branch
September 1, 2026 11:11
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.
Part of objectstack-ai/objectstack#13626
Executes the maintainer ruling of 2026-09-01 (director decision batch C, verbatim 「同意」): retire the reads.
className/gridClassNameon the form-view section family sit on the SDUI-only side of the authorable boundary —@objectstack/specdeliberately does not declare either on the form-view/section surface (itscomponent.zod.tssays so in as many words) and the authorable-surface ledger carries no entry for them. The renderer nevertheless reached them off the parsed view throughas anyat seven sites: the boundary declared on one side and crossed on the other, with the two repos each deliberate and in opposite directions.Declaring the keys was weighed and not adopted: it would formally invite free Tailwind strings into authored metadata, the exact class the boundary exists to keep out — and per ADR-0065 / ADR-0080 (rev. 2026-06-30) utility classNames in runtime metadata are never scanned by the build-time Tailwind, so they silently produce no CSS anyway.
Clause-②: no— no contract-schema accept/reject change. Nothing in@objectstack/specmoves; this PR only stops the renderer reading two keys the spec already declines to declare, so no parse verdict changes in either direction. The path limb does not apply (objectui repo).Authored by Claude Code, session
https://claude.ai/code/session_01Mciyv38maJ6HYVMiaM26T1(recorded in prose because a body edit rewrites the footer link).① Census first — the ruling's own gate
The ruling ordered a census before any narrowing, with heavy usage being evidence back to the card rather than a change. Measured across all three corpora, zero authored uses:
className/gridClassNameon form-view section nodescolumns:in the same section literals)a7002ce5899730ea6be39aMethod. A structural walker bracket-matches each
sections:array literal and reports the two keys inside it, so the count is scoped to the form-view section family rather than to the stringclassName. The identical script ran against all three corpora.How authored metadata was told apart from React props — the distinction the ruling asked for:
className:) counts; JSX attribute syntax (className=) does not.UIComponent/PageComponent) trees, which have their own declared ledger entryui/PageComponent:className; component prop declarations insdui.manifest.json;buttonVariants({ className })cva calls in hotcrm's docs site; and two prose comments in objectstack examples that warn against authoring the key.src/metadata tree. objectui:gridClassNamehas zero authored occurrences repo-wide — every hit is a component prop declaration, renderer plumbing, or prose.Why the zeros are readings and not absences. Each corpus carries a positive control: the same walker finds 115 / 14 / 72
columns:keys inside the very same section literals, so it demonstrably reaches section nodes. hotcrm is the strongest case — 14 view files author formsections, the exact node type in question, and none of them carries either style key.② The retired reads
Seven cast sites, re-derived on
origin/mainat dispatch (line numbers had drifted from the triage comment):packages/plugin-form/src/ObjectForm.tsx293/294, wizard325/326, split359/360, drawer392, modal423/424, stacked section-divider1293packages/plugin-form/src/DrawerForm.tsx598Behaviour after the change is the natural result of the existing code path — the key is not read, so it does not take effect.
FormSectionalready falls back togridCols[columns]whengridClassNameis absent, so section layout is unchanged. No existing test asserted the styles flow through, so none needed updating (all 821 pass unchanged).The pin is behavioural, and that is load-bearing
ObjectFormSection(this repo's own@object-ui/types) still declares both keys, so a later "cleanup" writing a plainclassName: s.className— noas anyin sight — type-checks and silently restores consumption. A source grep foras anywould stay green through exactly the regression the pin exists to catch.packages/plugin-form/src/__tests__/sectionStyleKeysRetired-13626.test.tsxtherefore authors the keys and asserts the strings never reach the DOM, across all seven arms, each row waiting on the section's own label first so that "absent" is a verdict rather than a form that failed to render.Reverse verification (direction predicted before running)
Restoring the tabbed arm's two reads turned exactly one row red — the TabbedForm row — in the predicted PRESENT direction, with the other seven green. The assertion that fired was the sentinel-absence check reporting that the rendered markup DID contain
os13626-authored-grid-class: under the mutation the authored grid class reappears as theclassattribute of the wrapper element around the field grid. That is also a precise record of the pre-change behaviour this PR removes.The mutation was proven on disk before the run (injected marker = 1, restored cast read = 1, worktree blob
13197ac0differs from HEAD blob777bbed4), and the restore leg proven after it (git diff HEADempty, worktree blob back to777bbed4, 0 residual markers).③ Changeset and semver
minoron@object-ui/plugin-form, with the breaking semantics spelled out in the body — this repo's version-alignment rule reservesmajorfor the objectstack cross-major sync and labels its own breaking changesminor.No ADR-0087 disposition: no declared surface moves. The keys were never on the authorable-surface ledger, which is the whole premise of the finding;
check:changeset-presenceandcheck:changeset-no-majorboth pass.Scope boundaries held
component.zod.tswere already correct.className(ObjectFormSchema.className, read as a plain uncastschema.classNameand forwarded to the form wrapper) is a different key on a different node, was not part of the ruling, and is deliberately untouched and unpinned.Docs
The README stated the renderer reads a section
className"explicitly on exactly one pseudo-field,type: 'section-divider'" — the read this PR retires, so the sentence was false as of this change. Corrected in place, plus a new Section styling is not authorable section recording the boundary, why declaring was rejected, and what still works. Theplugin-form.mdxclassNamerows are about field-level and form-rootclassNameand remain true, so they are left alone.Verification
All on final head
4174960, aftergit merge origin/main(which broughtpackages/componentschanges, so the dependency closure was rebuilt before re-running):pnpm exec vitest run packages/plugin-form/pnpm --filter @object-ui/plugin-form type-checktsc --noEmitplustsc -p tsconfig.test.json)pnpm --filter @object-ui/plugin-form lintas anycasts)check:changeset-presence/check:changeset-no-majorcheck:control-bytescheck:doc-fencescheck:governed-queue-guard --test PATHSNotes on measurement honesty:
tsc --listFiles, so "typecheck clean" actually covers the new test rather than excluding it.check:readme-exportsexits 1 locally for a whole-repo build precondition (dist/index.d.tsmissing for ~30 packages this PR never touches, plus the derived floorpackagesRead: found 12, floor is 25). Isolated rather than assumed: buildingplugin-formalone took its README failures from 58 to 0. CI builds before linting.eslint .job — the same unitturbo run lintruns — not a narrowed subset. Type-aware linting is off (noparserOptions.project), so this diff cannot move verdicts on untouched files.