diff --git a/.changeset/8738-object-form-fields-description.md b/.changeset/8738-object-form-fields-description.md index b209a5dc6a..9d70cf2653 100644 --- a/.changeset/8738-object-form-fields-description.md +++ b/.changeset/8738-object-form-fields-description.md @@ -3,16 +3,24 @@ --- `object-form`'s top-level `fields` input now documents its member vocabulary -(objectui#8738, route 2 of 2 — route 1, a diagnostic `console.warn`, is a -separate ruling still pending). +(objectui#8738, route 2 of 2 — route 1, a diagnostic `console.warn`, was a +separate ruling still pending when this was written; it landed 92 minutes +later, see the dated note below). The registration declared `{ name: 'fields', type: 'array' }` with no description, so an author had nowhere to read that this key's members are **bare field names** — a different vocabulary from `sections[].fields`, which also accepts the spec `FormFieldSchema` object (identity key `field`, e.g. `{ field: 'note', colSpan: 2 }`). Moving one of those objects to the top-level -`fields` resolves to no name and is silently skipped by `SimpleObjectForm` +`fields` resolves to no name and is skipped by `SimpleObjectForm` (`ObjectForm.tsx`) and by `buildFlatFields` (`flatFields.ts`, shared by the -drawer/modal presentations) — no throw, no warning, no empty-state. Behaviour -is unchanged; this only adds the description text an author would need to -avoid the drop before writing it. +drawer/modal presentations). Behaviour is unchanged by this entry; it only adds +the description text an author would need to avoid the drop before writing it. + +⚠️ Dated correction, so the original reading is not taken for the behaviour of +the release this publishes into. As written — `fd9bf26df0`, 2026-09-09T14:28:48Z +— that drop was SILENT: no throw, no warning, no empty-state. That reading was +true for 92 minutes. `8fda009057` (objectui#8859, route 1 of the same card) put +a de-duplicated `console.warn` at both named read sites at 2026-09-09T16:00:31Z, +so the drop is no longer silent. What did NOT change: the member is still +skipped, and there is still no throw and no empty-state. diff --git a/.changeset/9778-object-form-customfields-merge.md b/.changeset/9778-object-form-customfields-merge.md new file mode 100644 index 0000000000..3afc4d7c9b --- /dev/null +++ b/.changeset/9778-object-form-customfields-merge.md @@ -0,0 +1,42 @@ +--- +'@object-ui/plugin-form': minor +--- + +`object-form.customFields` now MERGES over the metadata-generated field set, as its +registered description always promised (objectui#9778, maintainer ruling 2026-09-18, +director seat batch #161 item 5). + +**Behaviour change, deliberately.** Hosts that used `customFields` as a full +replacement now see the metadata-generated members too. Before this change a +non-empty `customFields` replaced the generated set outright and the object's schema +was never even fetched; the registration has always described the other thing — +"Field definitions merged over the set generated from object metadata. With inline +definitions and no data source, this becomes the only field source." The per-member +merge the prose describes already existed as code in `ObjectForm` — the +`customFields?.find((f) => f.name === name)` lookup inside the metadata branch — and +was unreachable, because that branch ran only when `customFields` was absent or +empty, i.e. only when the lookup had nothing to find. + +The merge takes three directions, one pinned case each in +`objectFormCustomFieldsMembers-8071.test.tsx`: + +- **override** — a member naming a declared field supplies that field's whole + definition, in the generated set's position, inheriting nothing from it; +- **keep** — a declared field no member names still renders, from object metadata; +- **append** — a member naming a field the metadata never declares is added after + the generated set, in authored order. + +**Unchanged where there is nothing to merge over.** With no data source (or no +`objectName`) there is no generated set, so the members remain the only field +source — the registration's second sentence, and the shape `EmbeddableForm` uses, +which deliberately passes no data source once inline members are present. An object +the adapter cannot describe now falls back to that same members-only source rather +than replacing a form that used to render with an error panel. + +**Migration.** ⛔ No "replace mode" option is added: a host that wants a bespoke set +declares its own form. Two routes exist for a host that wants exactly its member +list against an object that HAS metadata, both through the existing `fields` +whitelist, which narrows the generated set the members merge over (measured): +`fields: []` leaves the members as the whole set, and `fields: ['customer']` renders +that one generated field plus the members. Authors who intended the documented merge +all along need to change nothing. diff --git a/packages/plugin-form/src/ObjectForm.tsx b/packages/plugin-form/src/ObjectForm.tsx index 85d6c1c400..cd4bd869a6 100644 --- a/packages/plugin-form/src/ObjectForm.tsx +++ b/packages/plugin-form/src/ObjectForm.tsx @@ -636,12 +636,26 @@ const SimpleObjectForm: React.FC = ({ useEffect(() => { if (hasInlineFields) { setInitialData(schema.initialData || schema.initialValues || {}); - setLoading(false); + // objectui#9778: inline members no longer short-circuit the metadata + // read — they MERGE over it — so the loading flag can only drop here + // when nothing is going to be fetched. Dropping it unconditionally put + // an empty `
` on screen for the duration of `getObjectSchema`. + // The metadata branch's own tail (`willFetchData`) clears it otherwise. + if (!(schema.objectName && dataSource)) { + setLoading(false); + } } - }, [hasInlineFields, schema.initialData, schema.initialValues]); + }, [hasInlineFields, schema.initialData, schema.initialValues, schema.objectName, dataSource]); - // Fetch object schema from ObjectQL/ObjectStack (skip if using inline fields) + // Fetch object schema from ObjectQL/ObjectStack (inline members merge OVER it) useEffect(() => { + // The field source when no object metadata is reachable: an object with no + // fields, over which the authored members are the whole set. + const inlineOnlySchema = { + name: schema.objectName, + fields: {} as Record, + }; + const fetchObjectSchema = async () => { try { if (!dataSource) { @@ -653,21 +667,34 @@ const SimpleObjectForm: React.FC = ({ } setObjectSchema(schemaData); } catch (err) { + // objectui#9778: for the inline path the metadata is an OVERLAY, not a + // prerequisite. A form that renders its authored members today must not + // become an error panel because the adapter cannot describe the object — + // fall back to the members-only source the registration promises for the + // no-data-source case. + if (hasInlineFields) { + setObjectSchema(inlineOnlySchema); + setLoading(false); + return; + } setError(err as Error); setLoading(false); } }; - // Skip fetching if we have inline fields - if (hasInlineFields) { - // Use a minimal schema for inline fields - setObjectSchema({ - name: schema.objectName, - fields: {} as Record, - }); - } else if (schema.objectName && dataSource) { + // objectui#9778: inline members are "merged over the set generated from + // object metadata" (the registered description of `customFields`), so the + // fetch is no longer skipped when they are present — without the generated + // set there is nothing to merge over and the merge lookup further down + // stays the dead code objectui#8071 measured. The members-only schema is + // what the registration's second sentence describes ("with inline + // definitions and no data source, this becomes the only field source"), so + // it is now the FALLBACK rather than the inline path's fixed answer. + if (schema.objectName && dataSource) { fetchObjectSchema(); - } else if (!hasInlineFields) { + } else if (hasInlineFields) { + setObjectSchema(inlineOnlySchema); + } else { // No objectName or dataSource and no inline fields — cannot proceed setLoading(false); } @@ -718,15 +745,26 @@ const SimpleObjectForm: React.FC = ({ // matcher, which is not a CEL evaluator and was never called downstream. const normalizeVisibility = useCallback((f: any): any => f, []); - // Generate form fields from object schema or inline fields + // Generate form fields from object schema, with inline members merged over it + // + // objectui#9778 — `customFields` MERGES, it does not replace. The registered + // description of the member ("Field definitions merged over the set generated + // from object metadata. With inline definitions and no data source, this + // becomes the only field source.") is the contract, and the per-member merge + // below (`schema.customFields?.find(...)`) was written for it — but a + // non-empty `customFields` used to return from here with + // `setFormFields(schema.customFields.map(normalizeVisibility))` BEFORE the + // generated set existed, so that lookup only ever ran over an empty array. + // The three directions the merge now takes, one case each in + // `objectFormCustomFieldsMembers-8071.test.tsx`: + // override — a member naming a declared field replaces that field's + // generated definition, in the generated set's position; + // keep — a declared field no member names still renders; + // append — a member naming a field the metadata never declares is added + // after the generated set, in authored order. + // With no data source the generated set is empty, so the members remain the + // only field source (the registration's second sentence) — unchanged. useEffect(() => { - // For inline fields, use them directly - if (hasInlineFields && schema.customFields) { - setFormFields(schema.customFields.map(normalizeVisibility)); - setLoading(false); - return; - } - if (!objectSchema) return; const generatedFields: FormField[] = []; @@ -973,6 +1011,20 @@ const SimpleObjectForm: React.FC = ({ } }); + // objectui#9778 — the APPEND direction. A member naming a field the + // generated set does not carry (an object metadata never declared, or one + // the `fields` whitelist left out) is added after it, in authored order; + // members that already overrode a generated field above are not repeated. + if (hasInlineFields && schema.customFields) { + const alreadyDrawn = new Set(generatedFields.map((f) => f.name)); + schema.customFields.forEach((customField: any) => { + const name = customField?.name; + if (!name || alreadyDrawn.has(name)) return; + alreadyDrawn.add(name); + generatedFields.push(normalizeVisibility(customField)); + }); + } + setFormFields(generatedFields); // Only set loading to false if we are not going to fetch data diff --git a/packages/plugin-form/src/__tests__/ObjectForm.mobileFullscreen.test.tsx b/packages/plugin-form/src/__tests__/ObjectForm.mobileFullscreen.test.tsx index f214781d3f..97084aefdc 100644 --- a/packages/plugin-form/src/__tests__/ObjectForm.mobileFullscreen.test.tsx +++ b/packages/plugin-form/src/__tests__/ObjectForm.mobileFullscreen.test.tsx @@ -46,7 +46,7 @@ * of racing RTL's 1000ms `findBy` budget against a cold Vite transform. */ import { describe, it, expect, vi } from 'vitest'; -import { render, screen, waitFor, fireEvent } from '@testing-library/react'; +import { render, screen, waitFor, fireEvent, within } from '@testing-library/react'; import React from 'react'; import { registerAllFields } from '@object-ui/fields'; import type { ObjectFormSchema } from '@object-ui/types'; @@ -239,7 +239,14 @@ describe('ObjectForm mobile.fullscreenLongText → RichTextField (objectui#3301) richTextObjectSchema, ); - const toggle = await screen.findByTestId('richtext-fullscreen-toggle'); + // objectui#9778 — `customFields` MERGES over the metadata-generated set, so + // this form draws the object's OTHER rich-text field (`changelog`) too and + // there are two expand affordances on screen. The case is about `summary`, + // so the lookup is scoped to it; before the merge this member was the whole + // field set and an unscoped `findByTestId` happened to be unambiguous. + await waitFor(() => expect(inlineControl('summary')).toBeInTheDocument()); + const summaryField = document.querySelector('[data-field="summary"]') as HTMLElement; + const toggle = within(summaryField).getByTestId('richtext-fullscreen-toggle'); fireEvent.click(toggle); fireEvent.change(screen.getByTestId('richtext-fullscreen-input'), { diff --git a/packages/plugin-form/src/__tests__/objectFormCustomFieldsMembers-8071.test.tsx b/packages/plugin-form/src/__tests__/objectFormCustomFieldsMembers-8071.test.tsx index 94e5edc600..da6b18246a 100644 --- a/packages/plugin-form/src/__tests__/objectFormCustomFieldsMembers-8071.test.tsx +++ b/packages/plugin-form/src/__tests__/objectFormCustomFieldsMembers-8071.test.tsx @@ -8,7 +8,8 @@ /** * `object-form.customFields` — the MEMBER shape this renderer reads - * (objectui#8071, criterion from objectui#8068). + * (objectui#8071, criterion from objectui#8068), now pinned on the MERGE + * behaviour ruled in objectui#9778. * * Declared on both sides without a member shape: the registration is * `{ type: 'array' }` with no `of`, and `@objectstack/spec`'s @@ -22,37 +23,44 @@ * With inline definitions and no data source, this becomes the only field * source." * - * ⇒ "merged over" and "with … no data source" both describe a merge. THE - * RENDERER NEVER MERGES. `ObjectForm.tsx` gates on - * `hasInlineFields = schema.customFields && schema.customFields.length > 0` and, - * when that holds, the field-generation effect does - * `setFormFields(schema.customFields.map(normalizeVisibility))` and RETURNS — - * above the metadata branch entirely, and whether or not an adapter was - * injected. A non-empty `customFields` therefore REPLACES the generated set on - * every path, and rows 1-3 pin that. + * ⇒ Both sentences describe a MERGE, and objectui#9778 ruled that the prose is + * the contract and the renderer was the defect: a non-empty `customFields` used + * to `setFormFields(schema.customFields.map(normalizeVisibility))` and return + * above the metadata branch entirely, so it REPLACED the generated set and the + * object's schema was never even fetched. Rows 1-4 pin the merge that replaced + * it, one row per direction: * - * ⚠️ The per-member merge the prose describes does exist as CODE — - * `schema.customFields?.find((f) => f.name === name)` inside the metadata - * branch — and it is unreachable: that branch runs only when `hasInlineFields` - * is false, i.e. when `customFields` is absent or EMPTY, so the `find` is - * always over nothing. Handed back as a finding on objectui#8071 rather than - * repaired here, because repairing it is a renderer (or a spec) change and this - * card writes pins only. + * OVERRIDE — a member naming a declared field supplies that field's whole + * definition, in the generated set's position (row 2), and the + * metadata IS fetched to have a position at all; + * KEEP — a declared field no member names still renders, with the label + * the object gave it (row 3); + * APPEND — a member naming a field the metadata never declares is added + * after the generated set, in authored order (row 4). * - * ⛔ Row 4 is the one a plausible "improvement" breaks. The gate reads + * ⚠️ Row 2's `getObjectSchema` assertion is the one that inverted: it used to + * read `toBe(0)` — "the renderer returns before that set is generated at all" — + * and the ruling made the fetch the precondition of the merge. It is asserted + * here rather than left implicit because a renderer that skipped the fetch + * again would still pass every drawn-field row on the no-data-source path. + * + * ⛔ Row 5 is the one a plausible "improvement" breaks. The gate reads * `.length > 0`, not truthiness, so an EMPTY `customFields: []` is UNAUTHORED: - * the metadata path runs and the object's own fields render. Simplifying that - * read to `!!schema.customFields` — which is what "has inline fields" reads - * like — turns a designer's not-configured-yet empty array into a form with no - * fields at all and no diagnostic, and every other row here stays green. + * the metadata path runs alone and the object's own fields render. Simplifying + * that read to `!!schema.customFields` — which is what "has inline fields" + * reads like — makes a designer's not-configured-yet empty array take the + * inline path; under the merge that no longer empties the form, but it does + * suppress the no-adapter panel row 6 pins, so the row stays. * - * Row 5 pins the SECOND read site of the same member count, one layer up: + * Row 6 pins the SECOND read site of the same member count, one layer up: * `index.tsx` spells `requiresDataSource={!(schema?.customFields?.length > 0) …}`, * so the members are what exempt this block from the gate's no-adapter panel. * `ElementDataSourceGate`'s own docblock states the contract in words ("an * `object-form` with inline `customFields` needs none"); this is that sentence * as behaviour, driven through the REGISTERED block with its control in the - * same test. + * same test. It doubles as the control for the registration's second sentence: + * with no data source there is no generated set to merge over, so the members + * are the only field source — the shape hosts authored before the ruling. */ import { describe, it, expect, vi } from 'vitest'; @@ -103,54 +111,75 @@ const labelOf = (c: HTMLElement, name: string): string | null => c.querySelector(`[data-field="${name}"] label`)?.textContent ?? null; describe('`object-form` — the member shape of `customFields`', () => { - it('1. members are whole FIELD DEFINITIONS keyed by `name`, rendered in AUTHORED order', async () => { + it('1. members are whole FIELD DEFINITIONS keyed by `name`, MERGED over the generated set', async () => { const { container } = await mount({ customFields: [ { name: 'zz', label: 'Brand new', type: 'text' }, { name: 'note', label: 'INLINE NOTE', type: 'text' }, ], }); - expect(drawnFields(container)).toEqual(['zz', 'note']); + expect( + drawnFields(container), + 'the generated set keeps its own order and its unnamed members; the member naming a ' + + 'declared field lands in that field\'s position, the one naming nothing declared is appended', + ).toEqual(['customer', 'note', 'amount', 'zz']); expect(labelOf(container, 'zz'), 'a member naming a field the object never declares still renders').toBe( 'Brand new', ); + expect(labelOf(container, 'note'), 'and a member naming a declared field is what renders for it').toBe( + 'INLINE NOTE', + ); }); - it('2. a non-empty `customFields` REPLACES the generated set — the object’s metadata is never even fetched', async () => { + it('2. OVERRIDE — a member replaces the declared field IN PLACE, and the metadata IS fetched', async () => { const { container, adapter } = await mount({ customFields: [{ name: 'note', label: 'INLINE NOTE', type: 'text' }], }); expect( adapter.getObjectSchema.mock.calls.length, - 'the registration promises a merge "over the set generated from object metadata"; the ' + - 'renderer returns before that set is generated at all', - ).toBe(0); + 'the registration promises a merge "over the set generated from object metadata"; there is ' + + 'no set to merge over unless the renderer goes and generates it (objectui#9778)', + ).toBe(1); + expect(adapter.getObjectSchema).toHaveBeenCalledWith('invoice'); expect( drawnFields(container), - 'the object declares `customer` / `note` / `amount`; only the member survives', - ).toEqual(['note']); + 'one member, three declared fields: the member takes `note`\'s position, it does not become the set', + ).toEqual(['customer', 'note', 'amount']); + expect( + labelOf(container, 'note'), + 'the object declares this field as "Note"; the member supplies the whole definition, inheriting nothing', + ).toBe('INLINE NOTE'); }); - it('3. a member naming a declared field inherits NOTHING from it — its own `label` is what renders', async () => { + it('3. KEEP — a declared field no member names renders with the definition the OBJECT gave it', async () => { const { container } = await mount({ customFields: [{ name: 'note', label: 'INLINE NOTE', type: 'text' }], }); - expect( - labelOf(container, 'note'), - 'the object declares this field as "Note"; a merge would have to reach it', - ).toBe('INLINE NOTE'); + expect(labelOf(container, 'customer'), 'untouched by the members, so the object\'s own label').toBe('Customer'); + expect(labelOf(container, 'amount'), 'same, for a field after the overridden one').toBe('Amount'); + }); + + it('4. APPEND — members naming nothing declared come after the generated set, in AUTHORED order', async () => { + const { container } = await mount({ + customFields: [ + { name: 'zz', label: 'Brand new', type: 'text' }, + { name: 'yy', label: 'Also new', type: 'text' }, + ], + }); + expect(drawnFields(container)).toEqual(['customer', 'note', 'amount', 'zz', 'yy']); + expect(labelOf(container, 'yy'), 'the appended member carries its own definition too').toBe('Also new'); }); - it('4. an EMPTY `customFields` is UNAUTHORED — the read is `.length > 0`, ⛔ not truthiness', async () => { + it('5. an EMPTY `customFields` is UNAUTHORED — the read is `.length > 0`, ⛔ not truthiness', async () => { const { container, adapter } = await mount({ customFields: [] }); expect(adapter.getObjectSchema).toHaveBeenCalledWith('invoice'); expect( drawnFields(container), - 'a truthiness read would make `[]` an inline source and render a form with no fields', + 'nothing is merged over the generated set, so the object\'s own fields are all of it', ).toEqual(['customer', 'note', 'amount']); }); - it('5. the same member count is what exempts the block from the no-adapter panel', async () => { + it('6. the same member count is what exempts the block from the no-adapter panel', async () => { const withMembers = render( { if (!withMembers.container.querySelector('form')) throw new Error('form not ready'); }); expect(withMembers.container.querySelector('[data-testid="object-form-no-data-source"]')).toBeNull(); - expect(drawnFields(withMembers.container as HTMLElement)).toEqual(['zz']); + expect( + drawnFields(withMembers.container as HTMLElement), + 'the registration\'s second sentence: with no data source there is no generated set, so the ' + + 'members are the only field source', + ).toEqual(['zz']); const withoutMembers = render(