diff --git a/.changeset/13626-formview-section-style-keys-retired.md b/.changeset/13626-formview-section-style-keys-retired.md new file mode 100644 index 0000000000..55a47dc1a8 --- /dev/null +++ b/.changeset/13626-formview-section-style-keys-retired.md @@ -0,0 +1,39 @@ +--- +'@object-ui/plugin-form': minor +--- + +Retire the form-view section `className` / `gridClassName` reads (objectstack#13626, +maintainer ruling 2026-09-01, director decision batch C). + +**Breaking, deliberately.** A `className` or `gridClassName` authored on a form-view +section no longer has any effect. Before this change an authored `gridClassName` +reached the section's field-grid `
` and an authored `className` reached the +section wrapper / divider header; both are now dropped at the renderer. + +The two keys sit on the SDUI-only side of the authorable boundary: `@objectstack/spec` +deliberately does not declare either on the form-view/section surface (its +`component.zod.ts` says so in as many words) and the authorable-surface ledger carries +no entry for them. The renderer nevertheless reached them off the parsed view through +`as any` at 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 instead 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. Declaring them would have published a styling surface whose most obvious use +does nothing. If per-view styling becomes a real product need it gets an explicit +controlled token surface, not two leaked keys. + +**Migration.** Nothing in the measured corpora has to change. A census across the +objectstack corpus, this repo's corpus, and the hotcrm application found **zero** +authored uses of either key on a form-view section (201 authored section nodes reached, +0 carrying either key). If you author them in your own metadata, move the styling to +the host application's own CSS, or to the form ROOT `className` — which is a different +key on a different node and is **unaffected** by this change. + +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. The omission is pinned behaviourally across all seven arms rather than by a source +grep, because `ObjectFormSection` still declares both keys — so a later uncast +`className: s.className` would type-check and silently restore consumption. diff --git a/packages/plugin-form/README.md b/packages/plugin-form/README.md index a6d9d286f7..ac3a8213a5 100644 --- a/packages/plugin-form/README.md +++ b/packages/plugin-form/README.md @@ -229,7 +229,7 @@ here too. Two that a reader might expect, and that are **not** declared: | Not a `FormField` key | Write this instead | |---|---| | `defaultValue` | `FormSchema.defaultValues` at form level. An object-bound form seeds from the object field's own declared `defaultValue` — see [What a create form opens with](#what-a-create-form-opens-with) | -| `className` | `span` / `colSpan` for width, `FormSchema.fieldContainerClass` for the grid. (An undeclared key still rides the props spread down to whichever component the field resolves to, so a field-level `className` can visibly land on a built-in control — but nothing in the contract promises that, and a registered widget honours it only if it happens to spread its leftover props. The renderer reads it *explicitly* on exactly one pseudo-field, `type: 'section-divider'`, where it styles the inline section header.) | +| `className` | `span` / `colSpan` for width, `FormSchema.fieldContainerClass` for the grid. (An undeclared key still rides the props spread down to whichever component the field resolves to, so a field-level `className` can visibly land on a built-in control — but nothing in the contract promises that, and a registered widget honours it only if it happens to spread its leftover props. The renderer reads it *explicitly* nowhere: it used to stamp the `type: 'section-divider'` pseudo-field from an authored section's `className`, and that read was **retired** in objectstack#13626 — see [Section styling is not authorable](#section-styling-is-not-authorable).) | There is no `ValidationRule` type in this repo, under any spelling. @@ -376,6 +376,36 @@ The grid is applied to the field container **inside** the form, never wrapped around the `
` (which would put the whole form in cell 1 and leave the other columns empty — #2128). +### Section styling is not authorable + +`className` and `gridClassName` on a form-view **section** do nothing. Authoring +them is not an error and not a compile failure — the renderer simply does not +read them. + +This is deliberate. Both keys sit on the SDUI-only side of the authorable +boundary: `@objectstack/spec` does not declare either on the form-view/section +surface, and the authorable-surface ledger carries no entry for them. The +renderer nevertheless reached them through `as any` at seven sites until +objectstack#13626 (maintainer ruling 2026-09-01) retired those reads — the +boundary had been declared on one side and crossed on the other. + +Declaring the keys instead was weighed and rejected: it would invite free +Tailwind strings into authored metadata, which is the class the boundary exists +to keep out, and per ADR-0065 / ADR-0080 utility classNames in runtime metadata +are never scanned by the build-time Tailwind — so they silently produce no CSS +even when they are read. If per-view styling becomes a real product need it will +get an explicit controlled token surface rather than two leaked keys. + +**What still works:** the form ROOT `className` (`FormSchema.className`) is a +different key on a different node and is unaffected; section *layout* stays +authorable through `columns` (above); and a host application styles sections +through its own CSS. + +> Note for contributors: `ObjectFormSection` in `@object-ui/types` still declares +> both keys, so a plain `className: s.className` would type-check and silently +> restore consumption. The non-consumption is therefore pinned behaviourally, in +> `src/__tests__/sectionStyleKeysRetired-13626.test.tsx`, across all seven arms. + ### Tabbed field layout (`fieldTabs`) A sectioned form is **one** form. Instead of rendering a form per section — which diff --git a/packages/plugin-form/src/DrawerForm.tsx b/packages/plugin-form/src/DrawerForm.tsx index 9b36052621..38808768d6 100644 --- a/packages/plugin-form/src/DrawerForm.tsx +++ b/packages/plugin-form/src/DrawerForm.tsx @@ -595,7 +595,12 @@ export const DrawerForm: React.FC = ({ onToggle: section.collapsible ? () => setCollapsedSections(prev => ({ ...prev, [sectionKey]: !isCollapsed })) : undefined, - className: (section as any).className, + // ⛔ `className` deliberately not read — objectstack#13626, maintainer + // ruling 2026-09-01 (batch C) "retire the reads". The key is on the + // SDUI-only side of the authorable boundary; `ObjectForm`'s drawer map + // stops copying it in the same pass. Full rationale at the tabbed arm + // in `ObjectForm.tsx`; pinned by + // `__tests__/sectionStyleKeysRetired-13626.test.tsx`. } as any); if (isCollapsed) { diff --git a/packages/plugin-form/src/ObjectForm.tsx b/packages/plugin-form/src/ObjectForm.tsx index 82bc1bf602..777bbed4a3 100644 --- a/packages/plugin-form/src/ObjectForm.tsx +++ b/packages/plugin-form/src/ObjectForm.tsx @@ -290,8 +290,33 @@ export const ObjectForm: React.FC = ({ // compiling and silently copy `undefined` — the exact silent-drop // failure this line exists to fix. visibleWhen: s.visibleWhen, - className: (s as any).className, - gridClassName: (s as any).gridClassName, + // ⛔ `className` / `gridClassName` are DELIBERATELY not copied here + // or at any of the six sibling sites below — objectstack#13626, + // maintainer ruling 2026-09-01 (director decision batch C): + // "retire the reads". + // + // The two keys sit on the SDUI-only side of the authorable + // boundary: `@objectstack/spec` deliberately does NOT declare them + // on the form-view/section surface (see the "Deliberately NOT + // declared" note in its `component.zod.ts`), and the + // authorable-surface ledger carries no entry for them. They were + // nevertheless reached off the parsed view through `as any` — the + // boundary declared on one side and crossed on the other. + // + // Declaring them instead 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. If per-view styling becomes a + // real product need it gets an explicit controlled token surface, + // not two leaked keys. + // + // ⚠️ Re-adding the read does NOT require a cast to compile: + // `ObjectFormSection` (this repo's own `@object-ui/types`) still + // declares both keys, so a plain `className: s.className` type + // -checks. The omission is therefore pinned behaviourally, not by + // a source grep — `__tests__/sectionStyleKeysRetired-13626.test.tsx`. })), defaultTab: schema.defaultTab, tabPosition: schema.tabPosition, @@ -322,8 +347,8 @@ export const ObjectForm: React.FC = ({ description: s.description, columns: s.columns, fields: s.fields, - className: (s as any).className, - gridClassName: (s as any).gridClassName, + // `className` / `gridClassName`: deliberately not copied — see the + // tabbed arm above (objectstack#13626, ruled 2026-09-01). })), allowSkip: schema.allowSkip, showStepIndicator: schema.showStepIndicator, @@ -356,8 +381,8 @@ export const ObjectForm: React.FC = ({ // ADR-0089 section predicate (#6111) — same reason as `pane` above: // a key this map does not copy never reaches the layout at all. visibleWhen: (s as any).visibleWhen, - className: (s as any).className, - gridClassName: (s as any).gridClassName, + // `className` / `gridClassName`: deliberately not copied — see the + // tabbed arm above (objectstack#13626, ruled 2026-09-01). })), splitDirection: schema.splitDirection, splitSize: schema.splitSize, @@ -389,7 +414,9 @@ export const ObjectForm: React.FC = ({ // ADR-0089 section predicate (#6111) — key-by-key rebuild, so an // uncopied key is silently dropped before DrawerForm ever sees it. visibleWhen: (s as any).visibleWhen, - className: (s as any).className, + // `className`: deliberately not copied — see the tabbed arm above + // (objectstack#13626, ruled 2026-09-01). DrawerForm's own divider + // site stops reading it in the same pass. })), open: schema.open, onOpenChange: schema.onOpenChange, @@ -420,8 +447,8 @@ export const ObjectForm: React.FC = ({ // ADR-0089 section predicate (#6111) — key-by-key rebuild, so an // uncopied key is silently dropped before ModalForm ever sees it. visibleWhen: (s as any).visibleWhen, - className: (s as any).className, - gridClassName: (s as any).gridClassName, + // `className` / `gridClassName`: deliberately not copied — see the + // tabbed arm above (objectstack#13626, ruled 2026-09-01). })), open: schema.open, onOpenChange: schema.onOpenChange, @@ -1290,7 +1317,8 @@ const SimpleObjectForm: React.FC = ({ onToggle: section.collapsible ? () => setCollapsedSections(prev => ({ ...prev, [sectionKey]: !isCollapsed })) : undefined, - className: (section as any).className, + // `className`: deliberately not read — see the tabbed arm above + // (objectstack#13626, ruled 2026-09-01 "retire the reads"). } as FormField); } diff --git a/packages/plugin-form/src/__tests__/sectionStyleKeysRetired-13626.test.tsx b/packages/plugin-form/src/__tests__/sectionStyleKeysRetired-13626.test.tsx new file mode 100644 index 0000000000..4fa7c5f800 --- /dev/null +++ b/packages/plugin-form/src/__tests__/sectionStyleKeysRetired-13626.test.tsx @@ -0,0 +1,240 @@ +/** + * ObjectUI + * Copyright (c) 2024-present ObjectStack Inc. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +/** + * objectstack#13626 — the form-view section style keys are NOT consumed. + * + * ## The ruling this file pins + * + * `className` / `gridClassName` on the form-view section family sit on the + * SDUI-only side of the authorable boundary: `@objectstack/spec` deliberately + * does not declare them for these views (its `component.zod.ts` says so in + * as many words) and the authorable-surface ledger carries no entry for them. + * The renderer nevertheless reached them off the parsed view through `as any` + * at seven sites — the boundary declared on one side and crossed on the other. + * + * Maintainer ruling 2026-09-01 (director decision batch C, verbatim「同意」): + * **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, 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. + * + * ## Why this pin is BEHAVIOURAL and not a source grep + * + * ⚠️ The retired reads did not actually need their casts. `ObjectFormSection` + * (this repo's own `@object-ui/types`) still declares both keys, so a later + * "cleanup" writing a plain `className: s.className` — no `as any` in sight — + * type-checks and silently restores consumption. A grep for `as any` would stay + * green through exactly the regression this file exists to catch. So each row + * authors the keys and asserts the strings never reach the DOM. + * + * ## The liveness control — why every row asserts something PRESENT + * + * An "absent from the DOM" assertion is satisfied for free by a form that + * failed to render, by a section that was never processed, and by a typo in the + * mount. Each row therefore first waits on the section's own label: the + * sentinel's absence is a verdict only once the node carrying it demonstrably + * rendered. `columns: 2` is authored alongside for the same reason — it is a + * sibling key on the same node that IS still consumed, so the section object + * reaching the layout is not in question. + * + * ## Reverse verification (direction predicted BEFORE running) + * + * Restore the copy at any ONE of the seven sites and exactly the rows that + * mount that arm go red in the PRESENT direction (the sentinel class reappears + * in the markup); every other row stays green. Measured that way — see the PR. + * + * ## Scope boundary this file does NOT cross + * + * The form ROOT `className` (`ObjectFormSchema.className`, read as plain + * `schema.className` and forwarded to the form wrapper) is a different key on a + * different node and was NOT part of the ruling — it is deliberately unpinned + * here. Rows below assert only the SECTION-level keys. + */ + +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import { render, screen, cleanup, waitFor } from '@testing-library/react'; +import React from 'react'; +import { registerAllFields } from '@object-ui/fields'; +import { ObjectForm } from '../ObjectForm'; +import { DrawerForm } from '../DrawerForm'; + +registerAllFields(); + +/** + * Sentinels, not plausible Tailwind. A real utility string ('p-4') could reach + * the DOM from the layout's own chrome and make a row lie in either direction. + */ +const SECTION_CLASS = 'os13626-authored-section-class'; +const GRID_CLASS = 'os13626-authored-grid-class'; + +/** + * TWO sections, both authoring the style keys plus a consumed sibling + * (`columns`). Two rather than one on purpose: a form declaring a SINGLE + * section never engages the tab arm at all (the renderer needs more than one + * usable tab), so a one-section fixture would have scored the tabbed row + * against the stacked layout — measured, not assumed: it was a one-section + * fixture that made the tabbed and wizard rows fail their liveness wait here. + */ +const sections = () => [ + { + name: 'always', + label: 'Always', + columns: 2, + fields: ['subject'], + className: SECTION_CLASS, + gridClassName: GRID_CLASS, + }, + { + name: 'second', + label: 'Second', + columns: 2, + fields: ['detail'], + className: SECTION_CLASS, + gridClassName: GRID_CLASS, + }, +]; + +const objectSchema = { + name: 'crm_case', + fields: { + subject: { type: 'text', label: 'Subject' }, + detail: { type: 'text', label: 'Detail' }, + }, +}; + +let dataSource: any; + +beforeEach(() => { + dataSource = { + getObjectSchema: vi.fn().mockResolvedValue(objectSchema), + findOne: vi.fn(), + create: vi.fn(), + update: vi.fn(), + }; +}); + +afterEach(() => { + cleanup(); + vi.restoreAllMocks(); +}); + +/** Mount through `ObjectForm` — the entry `RecordFormPage` itself uses. */ +const renderObjectForm = async (extra: Record) => { + render( + , + ); + // Liveness control: the section demonstrably rendered before we judge absence. + // `getAllBy*`, not `getBy*`: the wizard legitimately renders the label twice + // (step-indicator entry + section heading) and `getByText` throws on the + // duplicate — an inability, which must not read as a verdict here. + await waitFor(() => expect(screen.getAllByText('Always').length).toBeGreaterThan(0)); +}; + +/** Mount `DrawerForm` directly — the shape `resolveFormViewLayout` produces. */ +const renderDrawerFormDirect = async () => { + render( + , + ); + // `getAllBy*`, not `getBy*`: the wizard legitimately renders the label twice + // (step-indicator entry + section heading) and `getByText` throws on the + // duplicate — an inability, which must not read as a verdict here. + await waitFor(() => expect(screen.getAllByText('Always').length).toBeGreaterThan(0)); +}; + +/** + * Every arm whose synthesis site this card edited, named individually: a + * restore applied to six of seven sites still passes a suite that exercises six. + */ +const LAYOUTS: { label: string; mount: () => Promise }[] = [ + { + label: "ObjectForm — stacked 'simple' sections (ObjectForm.tsx section-divider site)", + mount: () => renderObjectForm({ formType: 'simple' }), + }, + { + label: "TabbedForm — formType 'tabbed' via ObjectForm delegation (tabbed section map)", + mount: () => renderObjectForm({ formType: 'tabbed' }), + }, + { + label: "WizardForm — formType 'wizard' via ObjectForm delegation (wizard step map)", + mount: () => renderObjectForm({ formType: 'wizard' }), + }, + { + label: "SplitForm — formType 'split' via ObjectForm delegation (split section map)", + mount: () => renderObjectForm({ formType: 'split' }), + }, + { + label: "ModalForm — formType 'modal' via ObjectForm delegation (modal section map)", + mount: () => renderObjectForm({ formType: 'modal', open: true }), + }, + { + label: "DrawerForm — formType 'drawer' via ObjectForm delegation (drawer section map)", + mount: () => renderObjectForm({ formType: 'drawer', open: true }), + }, + { + // The seventh site: DrawerForm's OWN divider read, which the ObjectForm + // drawer map above cannot cover — mounting DrawerForm directly is the only + // route that reaches it with an authored section. + label: 'DrawerForm — mounted directly (DrawerForm.tsx section-divider site)', + mount: () => renderDrawerFormDirect(), + }, +]; + +/** Everything rendered anywhere in the document, chrome included. */ +const markup = () => document.body.innerHTML; + +describe('objectstack#13626 — authored section `className`/`gridClassName` are not consumed', () => { + describe('the authored strings never reach the DOM', () => { + for (const layout of LAYOUTS) { + it(layout.label, async () => { + await layout.mount(); + expect(markup()).not.toContain(SECTION_CLASS); + expect(markup()).not.toContain(GRID_CLASS); + // Same verdict through the DOM's own class index, so a row cannot pass + // on an HTML-escaping accident rather than on non-consumption. + expect(document.querySelector(`.${SECTION_CLASS}`)).toBeNull(); + expect(document.querySelector(`.${GRID_CLASS}`)).toBeNull(); + }); + } + }); + + describe('the sentinel harness itself can fail (control)', () => { + // Without this, every row above is satisfied by a `markup()` that never + // contains ANY class — the exact phantom-check the liveness wait is there + // to prevent. This row proves the assertion's subject is a live document + // whose classes this locator really reads. + it('markup() sees classes that ARE rendered, and querySelector finds them', async () => { + await renderObjectForm({ formType: 'simple' }); + const withClass = document.querySelector('[class]'); + expect(withClass).not.toBeNull(); + const cls = withClass!.className.toString().split(/\s+/).filter(Boolean)[0]; + expect(cls).toBeTruthy(); + expect(markup()).toContain(cls); + expect(document.querySelector(`.${CSS.escape(cls)}`)).not.toBeNull(); + }); + }); +});