Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 25 additions & 0 deletions .changeset/8738-fields-warn-route1.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
---
'@object-ui/plugin-form': patch
---

`object-form`'s top-level `fields` (and its `form` / `view:form` alias and
`object-master-detail-form`'s parent `fields`) now emit a named
`console.warn` when a member resolves to no field name, instead of silently
dropping it (objectui#8738 route 1, ruled after route 2 landed in a prior
release).

Top-level `fields` reads only bare field-name strings (`{ name }` tolerated)
— 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 into a
top-level `fields` array resolves to no name and used to vanish without a
word; it is now reported once per distinct offender via `console.warn`,
naming the skipped shape and the vocabulary difference, modelled on
`sectionFields.ts`'s existing `warnOnMixedVocabulary`.

The render outcome is unchanged — the member is still dropped, not resolved;
this is a diagnostic-only addition, not a lenient fallback. `form` /
`view:form` (the same `ObjectFormRenderer`) and `object-master-detail-form`'s
parent `fields` (routed through the same `SimpleObjectForm` read path via
`ObjectForm`) inherit the warning for free; both registrations' `description`
now also document the vocabulary (objectui#8847).
Original file line number Diff line number Diff line change
Expand Up @@ -2193,7 +2193,7 @@ const MEMBER_PINS: Record<string, MemberPin> = {
},
'object-form.fields': {
file: 'packages/plugin-form/src/__tests__/objectFormFieldsMembers-8071.test.tsx',
pins: 'Members are BARE FIELD NAMES resolved against the object schema — authored order preserved (against a control with no `fields`, whose order differs), a name the object does not declare dropped rather than rendered as an untyped stub, and the `{ name }` object spelling recorded as the read site\'s tolerance rather than a second contract. The sharp row is the one no other file can make: `object-form` carries a SECOND surface spelled `fields` (`sections[].fields`), whose canonical member is the spec `FormFieldSchema` object keyed on `field` — and that exact entry as a member of the TOP-LEVEL key resolves to no name and is dropped in SILENCE (no throw, no warning), with the same entry inside a section rendering as the live control so the negative cannot come from an object that never renders that field. Asserted through the real `ObjectForm`, because the sink is its own `fieldsToShow` loop rather than the `normalizeSectionField` chokepoint the sibling key uses. The spec row is `z.array(z.unknown())` and the registration declares no `of`, so the read site is the whole member contract (objectui#8071).',
pins: 'Members are BARE FIELD NAMES resolved against the object schema — authored order preserved (against a control with no `fields`, whose order differs), a name the object does not declare dropped rather than rendered as an untyped stub, and the `{ name }` object spelling recorded as the read site\'s tolerance rather than a second contract. The sharp row is the one no other file can make: `object-form` carries a SECOND surface spelled `fields` (`sections[].fields`), whose canonical member is the spec `FormFieldSchema` object keyed on `field` — and that exact entry as a member of the TOP-LEVEL key resolves to no name and is dropped from render (no throw) — SILENTLY until objectui#8738 route 1 added a named `console.warn` for exactly this case (also pinned in the same file), with the same entry inside a section rendering as the live control so the negative cannot come from an object that never renders that field. Asserted through the real `ObjectForm`, because the sink is its own `fieldsToShow` loop rather than the `normalizeSectionField` chokepoint the sibling key uses. The spec row is `z.array(z.unknown())` and the registration declares no `of`, so the read site is the whole member contract (objectui#8071).',
},
'object-grid.bulkActionDefs': {
file: 'packages/plugin-grid/src/__tests__/bulkActionMembers-8071.test.tsx',
Expand Down
10 changes: 9 additions & 1 deletion packages/plugin-form/src/ObjectForm.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import {
} from './submitRedirectNavigation';
import { usePermissions } from '@object-ui/permissions';
import { sectionPredicateUnsupportedWarning } from './sectionPredicateDiagnostic';
import { warnUnresolvedTopLevelField } from './sectionFields';
import { TabbedForm } from './TabbedForm';
import { WizardForm, NAVIGATE_ON_SUCCESS_REFUSED_NOTE } from './WizardForm';
import { SplitForm } from './SplitForm';
Expand Down Expand Up @@ -772,7 +773,14 @@ const SimpleObjectForm: React.FC<ObjectFormComponentProps> = ({
// If fieldsToShow is an array of strings, fieldName is the string
// If fieldsToShow is array of objects (unlikely but possible in some formats), we need to extract name
const name = typeof fieldName === 'string' ? fieldName : (fieldName as any).name;
if (!name) return;
if (!name) {
// objectui#8738 route 1: a member that resolves to no name (most
// commonly the spec `FormFieldSchema` object, identity key `field`,
// that IS legal in `sections[].fields`) used to vanish here without a
// word. Warn, don't accept — see `warnUnresolvedTopLevelField`.
warnUnresolvedTopLevelField(fieldName, schema.objectName);
return;
}

const field = objectSchema.fields?.[name];
if (!field && !hasInlineFields) return; // Skip if not found in object definition unless inline
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,18 @@
* `FormFieldSchema` object, whose identity key is `field` (see
* `sectionFields.ts`, and `sectionFields.spec-parity.test.ts` for its key set).
* That same object as a member of the TOP-LEVEL `fields` resolves to no name
* and is dropped in silence — no throw, no console warning, no empty-state.
* and is dropped from render — no throw, no empty-state.
*
* Until objectui#8738 route 1 that drop was also SILENT — no console warning
* either. The maintainer ruling at issuecomment-5603577602 required a named
* `console.warn` naming the skipped key and the vocabulary difference (same
* voice/dedupe discipline as `sectionFields.ts`'s `warnOnMixedVocabulary`,
* see `warnUnresolvedTopLevelField`). Row 4 below now pins that the warning
* fires; the block after row 6 pins both its legs (fires for an unresolvable
* member, does not fire for a legal one) with entries distinct from row 4's,
* since the warning's dedupe Set is module-scoped for the whole file run.
*

* ⛔ NOT a restatement of the declaration: the declaration says `array` and
* stops. Every row below is a render whose outcome would change if the read
* site changed — the ablation objectui#8071 ran on this file removed the
Expand Down Expand Up @@ -107,15 +117,24 @@ describe('object-form `fields` members are BARE FIELD NAMES (objectui#8071)', ()
expect(await renderedFieldLabels({ fields: ['status', 'not_a_field'] })).toEqual(['Status']);
});

it('4. the spec SECTION-field object is NOT a member of this key, and is dropped silently', async () => {
it('4. the spec SECTION-field object is NOT a member of this key — dropped from render, and (route 1) now WARNED', async () => {
// `{ field }` is the spec `FormFieldSchema` identity key — legal in
// `sections[].fields`, meaningless here. The renderer reads `.name` off a
// non-string member, finds nothing, and skips it without a word.
// non-string member, finds nothing, and skips it — but since objectui#8738
// route 1 (ruling: issuecomment-5603577602), no longer without a word: a
// named `console.warn` fires, naming the skipped shape and the vocabulary
// difference. The render outcome is UNCHANGED from before route 1 (still
// no throw, still an empty result — the ruling makes the failure audible,
// it does not make the wrong spelling work); only the silence is gone.
const warn = vi.spyOn(console, 'warn').mockImplementation(() => {});
const error = vi.spyOn(console, 'error').mockImplementation(() => {});
try {
expect(await renderedFieldLabels({ fields: [{ field: 'note' }] })).toEqual([]);
expect(warn).not.toHaveBeenCalled();
expect(warn).toHaveBeenCalledTimes(1);
const [message] = warn.mock.calls[0];
expect(String(message)).toContain("{ field: 'note' }");
expect(String(message)).toContain('sections[].fields');
expect(String(message)).toContain('FormFieldSchema');
expect(error).not.toHaveBeenCalled();
} finally {
warn.mockRestore();
Expand Down Expand Up @@ -147,3 +166,35 @@ describe('object-form `fields` members are BARE FIELD NAMES (objectui#8071)', ()
expect(await renderedFieldLabels({ fields: [{ name: 'note' }] })).toEqual(['Note']);
});
});

describe('object-form top-level `fields` WARNS on an unresolvable member (route 1, objectui#8738)', () => {
// Route 1 adds an observable where there was none, so it owes a pin of its
// own with BOTH legs: fires for the case that used to vanish, and does NOT
// fire for a legal bare name — without the second leg, the first leg would
// still pass on a warning that fires unconditionally on every render.
//
// Deliberately uses a DIFFERENT entry (`sent_at`, not row 4's `note`):
// `warnUnresolvedTopLevelField`'s dedupe Set is keyed by object name +
// skipped shape and module-scoped for this whole file's run, so reusing row
// 4's exact shape here would observe the dedupe, not the firing.
it('fires once for a member that resolves to no name', async () => {
const warn = vi.spyOn(console, 'warn').mockImplementation(() => {});
try {
expect(await renderedFieldLabels({ fields: [{ field: 'sent_at' }] })).toEqual([]);
expect(warn).toHaveBeenCalledTimes(1);
expect(String(warn.mock.calls[0][0])).toContain("{ field: 'sent_at' }");
} finally {
warn.mockRestore();
}
});

it('does NOT fire for a legal bare field name — the firing control', async () => {
const warn = vi.spyOn(console, 'warn').mockImplementation(() => {});
try {
expect(await renderedFieldLabels({ fields: ['status'] })).toEqual(['Status']);
expect(warn).not.toHaveBeenCalled();
} finally {
warn.mockRestore();
}
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,137 @@
/**
* 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.
*/

/**
* objectui#8847 — MEASURES, rather than assumes, whether route 1's warning
* (objectui#8738, `warnUnresolvedTopLevelField` in `sectionFields.ts`) reaches
* the two surfaces #8847 named beyond `object-form` itself:
*
* 1. `form` / `view:form` — registered on the exact same `ObjectFormRenderer`
* component as `object-form` (`index.tsx`), so it is the SAME code path
* by construction, not merely a similar one. No render test needed to
* "prove" a React component warns differently depending on the string
* key it happens to be registered under — it cannot.
* 2. `object-master-detail-form`'s parent `fields` — routed differently
* (`MasterDetailForm` builds a `parentSchema` and renders it through
* `<ObjectForm>`, `MasterDetailForm.tsx`), so THIS one is measured here.
*
* Result: route 1 covers both for free. `MasterDetailForm`'s `parentSchema`
* carries `fields: schema.fields` straight through with no `sections`
* (`fields` is documented as ignored once `sections` is given), so
* `ObjectForm`'s formType routing (`ObjectForm.tsx`, the `schema.sections?.length`
* guards on every sectioned branch) falls through to `SimpleObjectForm` — the
* exact read site route 1 patches — regardless of the master-detail
* `formType` ('simple' | 'tabbed'). ⇒ #8847's remaining work for both
* surfaces is documentation only (see the `description` added in `index.tsx`).
*/

import { describe, it, expect, vi } from 'vitest';
import { render, waitFor } from '@testing-library/react';
import React from 'react';
import { registerAllFields } from '@object-ui/fields';
import { MasterDetailForm } from '../MasterDetailForm';

registerAllFields();

const PARENT_SCHEMA = {
name: 'invoice',
fields: {
status: { type: 'text', label: 'Status' },
note: { type: 'text', label: 'Note' },
},
};

function makeDataSource() {
return {
getObjectSchema: vi.fn().mockResolvedValue(PARENT_SCHEMA),
find: vi.fn().mockResolvedValue({ data: [] }),
create: vi.fn(),
update: vi.fn(),
delete: vi.fn(),
bulk: vi.fn(),
} as any;
}

describe('object-master-detail-form parent `fields` inherits route 1 for free (objectui#8847)', () => {
it('warns when a parent-fields member resolves to no name — the spec FormFieldSchema object', async () => {
const warn = vi.spyOn(console, 'warn').mockImplementation(() => {});
try {
const { container } = render(
<MasterDetailForm
schema={
{
objectName: 'invoice',
mode: 'create',
fields: [{ field: 'note' }], // the #8738 trap shape — legal in sections[].fields, not here
details: [
{
childObject: 'invoice_line',
relationshipField: 'invoice',
columns: [{ key: 'qty', label: 'Qty', type: 'number' } as any],
},
],
} as any
}
dataSource={makeDataSource()}
/>,
);
await waitFor(() => expect(container.querySelector('form')).toBeTruthy());
// Filtered rather than an exact call count: this render also fires an
// unrelated `react-i18next` "no i18next instance" warning in this test
// harness (no I18nextProvider is mounted), which is not this pin's
// concern.
const said = warn.mock.calls
.map((c) => String(c[0]))
.filter((m) => m.includes('top-level `fields`'));
expect(said).toHaveLength(1);
expect(said[0]).toContain("{ field: 'note' }");
expect(said[0]).toContain('sections[].fields');
// The trapped field never rendered — route 1 warns, it does not resolve.
expect(container.querySelector('label')?.textContent ?? '').not.toContain('Note');
} finally {
warn.mockRestore();
}
});

it('does NOT warn for a legal bare parent field name — the firing control', async () => {
const warn = vi.spyOn(console, 'warn').mockImplementation(() => {});
try {
const { container } = render(
<MasterDetailForm
schema={
{
objectName: 'invoice',
mode: 'create',
fields: ['status'],
details: [
{
childObject: 'invoice_line',
relationshipField: 'invoice',
columns: [{ key: 'qty', label: 'Qty', type: 'number' } as any],
},
],
} as any
}
dataSource={makeDataSource()}
/>,
);
await waitFor(() => {
const el = container.querySelector('input[name="status"], label');
expect(el).toBeTruthy();
});
// Filtered for the same reason as the row above (unrelated i18next
// warning fires regardless of the `fields` shape under test).
const said = warn.mock.calls
.map((c) => String(c[0]))
.filter((m) => m.includes('top-level `fields`'));
expect(said).toHaveLength(0);
} finally {
warn.mockRestore();
}
});
});
42 changes: 41 additions & 1 deletion packages/plugin-form/src/flatFields.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
* no-sections drawer) lives in `flatFieldRules.test.tsx`.
*/

import { describe, it, expect } from 'vitest';
import { describe, it, expect, vi } from 'vitest';
import { buildFlatFields } from './flatFields';
import { normalizeSectionField } from './sectionFields';

Expand Down Expand Up @@ -82,6 +82,46 @@ describe('buildFlatFields — ADR-0036 rules reach the runtime FormField (#4755)
});
});

/**
* objectui#8738 route 1 — same shared read-site defect as `ObjectForm.tsx`'s
* `SimpleObjectForm` (pinned in `__tests__/objectFormFieldsMembers-8071.test.tsx`),
* measured directly here since `buildFlatFields` is the drawer/modal path with
* the identical shape: a member resolving to no name used to be dropped in
* total silence. `warnUnresolvedTopLevelField` (`sectionFields.ts`) now names
* the skipped shape and the vocabulary difference — same voice/dedupe
* discipline as `warnOnMixedVocabulary`. Both legs pinned: fires for the
* unresolvable case, and (the firing control) does NOT fire for a legal bare
* name — without the second leg the first would still pass on a warning that
* fires unconditionally.
*/
describe('buildFlatFields WARNS on an unresolvable top-level member (route 1, objectui#8738)', () => {
it('fires once for a member that resolves to no name — the spec FormFieldSchema object', () => {
const warn = vi.spyOn(console, 'warn').mockImplementation(() => {});
try {
const built = buildFlatFields({ ...ctx, fields: [{ field: 'note' } as any] });
expect(built).toEqual([]); // still dropped from render — route 1 warns, it does not resolve
expect(warn).toHaveBeenCalledTimes(1);
const [message] = warn.mock.calls[0];
expect(String(message)).toContain("{ field: 'note' }");
expect(String(message)).toContain('sections[].fields');
expect(String(message)).toContain('FormFieldSchema');
} finally {
warn.mockRestore();
}
});

it('does NOT fire for a legal bare field name — the firing control', () => {
const warn = vi.spyOn(console, 'warn').mockImplementation(() => {});
try {
const built = buildFlatFields({ ...ctx, fields: ['status'] });
expect(built.map((f) => f.name)).toEqual(['status']);
expect(warn).not.toHaveBeenCalled();
} finally {
warn.mockRestore();
}
});
});

/**
* PARITY PIN. The flat and sectioned producers now resolve every field through
* the same `fromObjectSchema`, so their output must agree on every key the
Expand Down
9 changes: 7 additions & 2 deletions packages/plugin-form/src/flatFields.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@
*/

import type { FormField } from '@object-ui/types';
import { fromObjectSchema, type SectionFieldsContext } from './sectionFields';
import { fromObjectSchema, warnUnresolvedTopLevelField, type SectionFieldsContext } from './sectionFields';

export interface FlatFieldsContext extends SectionFieldsContext {
/**
Expand All @@ -60,7 +60,12 @@ export function buildFlatFields(ctx: FlatFieldsContext): FormField[] {

for (const entry of fieldsToShow) {
const name = typeof entry === 'string' ? entry : (entry as any)?.name;
if (!name) continue;
if (!name) {
// objectui#8738 route 1: same read-site shape as `SimpleObjectForm`'s
// `fieldsToShow` loop in `ObjectForm.tsx` — warn, don't accept.
warnUnresolvedTopLevelField(entry, ctx.objectName);
continue;
}
const field = ctx.objectSchema?.fields?.[name];
if (!field) continue;

Expand Down
Loading
Loading