diff --git a/.changeset/inputs-reverse-parity-3808.md b/.changeset/inputs-reverse-parity-3808.md new file mode 100644 index 0000000000..05aafc1fdf --- /dev/null +++ b/.changeset/inputs-reverse-parity-3808.md @@ -0,0 +1,53 @@ +--- +"@object-ui/plugin-detail": patch +"@object-ui/components": patch +--- + +Four spec keys the renderers already honoured are now discoverable from the published `inputs` + +`record:details.hideFields`, `record:related_list.relationshipValueField`, +`record:related_list.add` and `element:text_input.defaultValue` were declared by +`@objectstack/spec` and read by their renderers, while the registry `inputs` — +the surface `gen-manifest.ts` serializes into `sdui.manifest.json` and +`sdui-intrinsics.d.ts` — never mentioned them. Nothing anywhere reported the +mismatch, and every layer that reads a manifest said the opposite of the +runtime: the keys were in no designer panel and no generated `.d.ts`, +`sdui-parser`'s prop walk returned `unknown-prop` for an author who wrote one, +and the renderer honoured it regardless. That is objectui#3407's original +complaint (`readonly` was enforced and honoured, the description just never said +so) on four more keys. + +Each description is derived from what the renderer actually does, not from +restating the spec's one-liner, because the two can differ and the published +text is what an AI author reads: + +- `hideFields` documents bare field names only — the renderer tolerates + `{name}` / `{field}` entries but the spec is `z.array(z.string())` and rejects + them, so teaching that spelling would publish a dialect the contract refuses; +- `relationshipValueField` publishes the renderer's `'id'` default and says that + the resolved value drives the list filter, the Add-picker link value and the + pre-filled create form together; +- `add` publishes its member shape in prose (`ComponentInput` is flat and has no + member-shape slot) with each default taken from the renderer — including + `picker.labelField`, where the renderer defaults to `name` while the spec's + own wording says "the object title field". It also names `picker.filter` as a + KNOWN GAP rather than documenting it as a restriction: the spec declares it + and nothing reads it, so an author would otherwise believe their picker is + scoped when it offers every record (objectui#3831); +- `defaultValue` distinguishes the two behaviours an author can get — seeding a + bound page variable once while it is still empty, versus the native + uncontrolled initial value with no variable bound. + +`element:text_input` is not in the public tier, so its gap was not in +`sdui.manifest.json` at all — it was in the JSX-page compiler's prop whitelist, +which `renderers/layout/page.tsx` builds from `getKnownTypes()` plus these same +`inputs`, making the undeclared `defaultValue` a live `unknown-prop` warning. + +The repo-wide parity gate now runs in both directions over one covered set and +one exemption discipline, so neither direction can be forgotten again the way +the reverse half was after PR #3806. Nine spec keys stay deliberately +unpublished, each with a written reason and a tracking issue: two the renderers +do not read at all (objectui#3829), three retired upstream by ADR-0087 +tombstones, `page:tabs.type` (a carrier collision, objectstack#6776), two +`targetVariable` declarative hints (objectui#3834), and +`element:record_picker.filter` (objectui#3830). diff --git a/apps/console/src/__tests__/public-block-binding-reach.test.tsx b/apps/console/src/__tests__/public-block-binding-reach.test.tsx index 5b85ff5fc6..28f452f7ec 100644 --- a/apps/console/src/__tests__/public-block-binding-reach.test.tsx +++ b/apps/console/src/__tests__/public-block-binding-reach.test.tsx @@ -233,6 +233,15 @@ const SUPERSEDES_BINDING = new Set(['data']); */ const sampleFor = (input: any): unknown => { if (input.name === 'objectName') return PROBE_OBJECT; + // `record:related_list.add` — the generic `object` sample below is `{}`, and + // `{}` is not a valid `add`: the spec makes `picker` required. An invalid one + // does not merely under-configure this block, it CRASHES it + // (`RelatedList.tsx:1299` dereferences `add.picker.object`, objectui#3838) — + // and a crashed block makes no data calls, which is indistinguishable from the + // "declines to fetch" verdict this block is ledgered for below. That is a green + // for the wrong reason, so the sample is spec-valid at the source instead. + // Arrived with objectui#3808, which is when `add` became a declared input. + if (input.name === 'add') return { picker: { object: PROBE_OBJECT } }; if (input.defaultValue !== undefined) return input.defaultValue; switch (input.type) { case 'number': @@ -253,13 +262,29 @@ const sampleFor = (input: any): unknown => { }; /** - * Mount one block bare and report every data-layer call it made. + * What one probe mount observed: every data-layer call the block made, and the + * DOM it produced. + * + * The html half is here because a crash is invisible in `calls` alone — + * `SchemaRenderer` catches a renderer's throw and paints an error card, so a + * crashed block simply makes no calls, which is the pass condition on the + * ledgered branch below. Deliberately the same shape and field names as the + * sibling probe's `Mount` (`record-block-record-reach.test.tsx:310-313`), which + * has captured both halves from the start for the same reason. + */ +interface Mount { + calls: string[]; + html: string; +} + +/** + * Mount one block bare and report every data-layer call it made, plus the DOM. * * The data source is a Proxy so ANY method a block reaches for is recorded * rather than crashing it — a block that calls `dataSource.aggregate` must not * fail the probe merely because a hand-written stub didn't anticipate it. */ -async function dataCallsFor(cfg: any): Promise { +async function dataCallsFor(cfg: any): Promise { const calls: string[] = []; const record = (key: string) => (...args: unknown[]) => { @@ -309,12 +334,21 @@ async function dataCallsFor(cfg: any): Promise { // its object. Every call it made is already recorded above, so swallow the // unmount and let the assertion speak to the data reach. Deliberately scoped // to unmount: an error thrown during RENDER still propagates and fails. + // Read the DOM before unmounting: `SchemaRenderer` CATCHES a renderer's throw + // and paints an error card, so a crash never propagates here — it just makes + // the block produce nothing, including no data calls. For a ledgered block + // ("declines to fetch") that is a green earned by crashing, which is why this + // is captured and asserted rather than left to the calls alone. Same guard the + // sibling probe carries as `assertRendered` + // (`record-block-record-reach.test.tsx`), added here after objectui#3808 made + // an invalid `add` sample able to trigger exactly that. + const html = view.container.innerHTML; try { view.unmount(); } catch { /* see above */ } - return calls; + return { calls, html }; } const candidates = ComponentRegistry.getPublicConfigs().filter(declaresObjectName); @@ -335,9 +369,40 @@ describe('public blocks — a declared objectName reaches the data layer (object for (const cfg of candidates) { const ledgered = cfg.type in NO_DATA_REACH; it(`${cfg.type} ${ledgered ? 'does not reach the data layer (ledgered)' : 'asks the data layer for its objectName'}`, async () => { - const calls = await dataCallsFor(cfg); + const { calls, html } = await dataCallsFor(cfg); const reached = calls.filter((c) => c.includes(PROBE_OBJECT)); if (ledgered) { + // A crash is not a binding answer, and on THIS branch it is + // indistinguishable from one: "made no data call" is the pass condition, + // and a block that threw during render made none either. `SchemaRenderer` + // catches the throw and paints an error card, so nothing propagates — + // without this the ledger entry would be confirmed by the block being + // broken. + // + // Added with objectui#3808, and DEFENSIVE rather than load-bearing today: + // that change made an invalid `add` sample crash `record:related_list` + // (objectui#3838), which is what it does in the sibling probe, but not + // here — `renderers/record-related-list.tsx:185` passes + // `dataSource={ctx?.dataSource}`, this probe mounts with no RecordContext, + // so `RelatedList`'s `add && dataSource` guard short-circuits before the + // unguarded read. Checked, not assumed: reverting the sample to `{}` keeps + // all 16 green. The predicate itself is known to work — applied to both + // branches it reports the two crashes in objectui#3840 — so this is a + // cheap standing guard on the one branch where a crash IS the pass + // condition, not a claim that it fires today. + // + // Deliberately NOT applied to the other branch: `object-form` and + // `object-master-detail-form` do paint an error card under this fixture + // ("Cannot read properties of undefined (reading 'map')") while still + // making their data calls, so their verdicts are earned rather than + // vacuous. Whether that card is a product bug or this fixture handing + // them an implausible configuration — the lesson the header records four + // instances of — is objectui#3840, not something to decide by widening a + // guard here. + expect( + html.includes('failed to render'), + `<${cfg.type}> threw during render, so "made no data call" proves nothing:\n${html.slice(0, 600)}`, + ).toBe(false); // Asserted, not skipped: the day this block starts binding, this fails // and the ledger entry has to go — a ledger nobody is forced to update // decays into the accepted-baseline problem this whole test exists for. diff --git a/apps/console/src/__tests__/record-block-record-reach.test.tsx b/apps/console/src/__tests__/record-block-record-reach.test.tsx index 4228c12816..d2e0fe77e3 100644 --- a/apps/console/src/__tests__/record-block-record-reach.test.tsx +++ b/apps/console/src/__tests__/record-block-record-reach.test.tsx @@ -196,6 +196,19 @@ const DATA_SOURCE_METHODS = [ * CONFIGURATION. It is also why {@link assertRendered} exists — a crash must * fail loudly rather than land in the "no difference" bucket and read as a * finding about the block. + * + * `add` is the FIFTH instance, and arrived the moment `record:related_list` + * started publishing it (objectui#3808). The generic `object` sample is `{}`, + * and `{}` is not a valid `add`: the spec makes `picker` required, so the sample + * has to carry `picker.object` or the fixture is exercising metadata no author + * could publish. Filled here rather than by loosening the generic `object` + * sample, which would put an unspecified bag on every future `object` input. + * + * That `{}` did not merely under-exercise the block, it CRASHED it — + * `RelatedList.tsx:1299` dereferences `add.picker.object` where `:378` / `:390` + * optional-chain the same path — and the crash is filed as objectui#3838 rather + * than papered over here: this fixture's job is to be spec-valid, not to steer + * clear of the renderer's unguarded reads. */ const SAMPLE_BY_INPUT: Readonly> = { // On `record:*` this names the RELATED object, not the page's object — @@ -223,6 +236,11 @@ const SAMPLE_BY_INPUT: Readonly> = { visible: "record.stage === 'qualified'", title: 'Probe Alert', body: 'Probe alert body', + // `record:related_list.add` — spec-valid minimum, i.e. `picker.object` present. + // Points at the same child object the rest of this fixture uses, so the Add + // affordance is configured against something that exists rather than at a + // dangling name. + add: { picker: { object: PROBE_CHILD_OBJECT } }, }; /** Fill one declared input. */ diff --git a/apps/console/src/__tests__/registry-inputs-spec-parity.test.ts b/apps/console/src/__tests__/registry-inputs-spec-parity.test.ts index 13a2d55d38..5137fcb038 100644 --- a/apps/console/src/__tests__/registry-inputs-spec-parity.test.ts +++ b/apps/console/src/__tests__/registry-inputs-spec-parity.test.ts @@ -6,18 +6,27 @@ * LICENSE file in the root directory of this source tree. * * Registry `inputs` <-> `@objectstack/spec` `ComponentPropsMap` parity, for - * EVERY block that has both (objectui#3797). + * EVERY block that has both, in BOTH directions (objectui#3797, objectui#3808). * - * PR #3795 landed this check on one block (`record:highlights`, see + * PR #3795 landed both directions on one block (`record:highlights`, see * `packages/plugin-detail/src/__tests__/recordHighlightsInputs.spec-parity.test.ts`). - * This file is the repo-wide half: it asserts the same direction — a block may - * not DECLARE a top-level input its spec props schema does not accept — for - * every entry of `ComponentPropsMap` this repo registers with a non-empty - * `inputs`. + * This file is the repo-wide half, and it carries the same two: * - * WHY THE DIRECTION MATTERS. `inputs` is not documentation, it is the published - * authoring surface, and four layers are silent about a key that only exists - * there: + * FORWARD (objectui#3797, PR #3806) — a block may not DECLARE a top-level + * input its spec props schema does not accept. + * + * REVERSE (objectui#3808) — a top-level key the spec DOES declare must be + * discoverable from that block's `inputs`. + * + * Both live in one file on purpose. #3808 exists because PR #3806 shipped only + * the forward half repo-wide and the reverse half stayed on the single block + * PR #3795 had covered; keeping them side by side, over one `covered` set and + * one exemption discipline, is what stops a direction from being forgotten + * again. + * + * WHY THE FORWARD DIRECTION MATTERS. `inputs` is not documentation, it is the + * published authoring surface, and four layers are silent about a key that only + * exists there: * * 1. `packages/sdui-parser/scripts/gen-manifest.ts` serializes `inputs` into * `sdui.manifest.json` (the save-gate + parser whitelist) and into @@ -45,6 +54,26 @@ * always paired with an issue that resolves the disagreement, never left as a * standing licence. * + * WHY THE REVERSE DIRECTION MATTERS JUST AS MUCH. A key the spec declares, the + * renderer honours, and `inputs` omits does not exist as far as an author can + * tell — and the same four layers are just as quiet, only inverted: + * `gen-manifest.ts` leaves it out of `sdui.manifest.json` and + * `sdui-intrinsics.d.ts`, so it is in no designer panel and no `.d.ts`; + * `validate.ts:74` does not find it in `comp.inputs` and reports `unknown-prop` + * on it; and the renderer honours it anyway. An author who writes it is warned + * off a key that works, and an author who doesn't never learns it is there. + * That is objectui#3407's original complaint verbatim (`readonly` was enforced + * by the HeaderHighlight gate and honoured by the renderer — the description + * just never mentioned it), and objectui#3808 found it on three more keys plus + * one this gate now covers as an exemption. + * + * The reverse direction bites non-public blocks too, which is the other reason + * coverage is not limited to `PUBLIC_BLOCKS`: `element:text_input` never reaches + * `sdui.manifest.json`, but `page.tsx:462` builds the JSX-page compiler's prop + * whitelist from `getKnownTypes()` + these same `inputs`, so its undeclared + * `defaultValue` was a live `unknown-prop` warning on a key the renderer seeded + * page variables from. + * * WHY IT LIVES HERE. The check needs the FULL registration graph — the same one * that produces the published artifacts. `dev/manifest-dump.tsx` builds them * from `src/register-plugins.ts` plus `@object-ui/components`, so this file @@ -68,12 +97,29 @@ * DELETED rather than kept — the last test in this file turns a no-longer-needed * exemption red, so the list cannot rot into a permanent allowlist. * - * LIMIT — worth knowing before trusting a pass. This gate can only see TOP-LEVEL - * keys. An `inputs` entry of type `array`/`object` declares no member shape - * (`ComponentInput` has no slot for one), so a drifted key INSIDE an array - * element is invisible here; making that machine-readable is its own change - * across types/core/sdui-parser and is tracked separately (PR #3795's open - * question). A pass means the top-level surface is in parity, nothing more. + * LIMIT — worth knowing before trusting a pass. This gate compares TOP-LEVEL + * KEY NAMES and nothing else. Three things it therefore cannot see, all of them + * real and all filed: + * + * - member shapes. An `inputs` entry of type `array`/`object` declares no + * member shape (`ComponentInput` has no slot for one), so a drifted key + * INSIDE an array element or nested object is invisible here — which is why + * `record:details.sections`, `record:highlights.fields` and + * `record:related_list.add` publish their members in prose and are pinned by + * per-block tests next to their renderers. PR #3795's open question; + * - types. `ComponentInput.type` is one coarse control kind and cannot spell a + * spec union, so a key can be in perfect NAME parity while publishing a + * narrower type than the contract accepts (objectui#3832); + * - `retiredKey()` tombstones. `Object.keys(shape)` still contains a key the + * spec rejects BY NAME, and the two directions then fail opposite ways — + * forward reads the tombstone as "accepted" and goes falsely GREEN, reverse + * reads it as "declared" and would demand the block publish it, going + * falsely RED. Dormant today (zero tombstones in the pinned rc.5) and fixed + * in one place — narrowing `specTopLevelKeys` — for both directions at once: + * objectui#3809. Until then the reverse direction's exemptions for the + * `element:record_picker` trio are what absorb the red, and they say so. + * + * A pass means the top-level key names are in parity, nothing more. */ import { describe, it, expect } from 'vitest'; @@ -115,6 +161,35 @@ function offSpecInputs(type: string): string[] { return (declaredInputs(type) ?? []).filter((name) => !allowed.has(name)); } +/** + * Spec keys that no block is expected to publish, with the reason — applied to + * every block rather than repeated as one exemption entry per block. + * + * Only `aria` qualifies, and only because the reason is genuinely uniform: it is + * an accessibility escape hatch, not a layout choice, and the blocks that omit + * it say so in the same words at their registration sites + * (`plugin-detail/src/index.tsx:335-337`, verbatim: "`aria` is omitted for the + * same reason it is omitted on `record:details` above"). Publishing it would put + * an `aria` object in every designer panel and every generated `.d.ts` as though + * hand-writing ARIA were the normal way to configure a block, when the renderers + * derive their accessible names from labels and object metadata. A key whose + * reason is per-block belongs in `UNPUBLISHED_EXEMPTIONS` below, not here. + */ +const GLOBALLY_UNPUBLISHED_SPEC_KEYS: Record = { + aria: 'Accessibility escape hatch, not a layout choice — renderers derive accessible names from labels and object metadata, and every block omits it for this one reason (plugin-detail/src/index.tsx:335-337). objectui#3808.', +}; + +/** + * Top-level keys this block's spec props schema declares that its `inputs` do + * not publish — the reverse direction (objectui#3808). + */ +function undiscoverableSpecKeys(type: string): string[] { + const declared = new Set(declaredInputs(type) ?? []); + return specTopLevelKeys(type).filter( + (key) => !declared.has(key) && !(key in GLOBALLY_UNPUBLISHED_SPEC_KEYS), + ); +} + /** * The blocks this gate judges: an entry of `ComponentPropsMap` that this repo * registers with at least one `inputs` entry. A block with no `inputs` — or one @@ -270,11 +345,124 @@ const OFF_SPEC_EXEMPTIONS: Record = { 'Already declared upstream by objectstack#5775; flagged only because the pinned @objectstack/spec@17.0.0-rc.5 predates it. Delete this entry when the pin moves.', }; +/** + * Spec-declared top-level keys deliberately NOT published, each with the reason. + * Key format: `BLOCK.KEY`. The reverse direction's half of the same discipline + * as `OFF_SPEC_EXEMPTIONS` above: explicit, reasoned, issue-backed, and deleted + * by a failing test once it stops describing anything. + * + * The bar for an entry is NOT "we haven't got round to it". A spec key the + * renderer HONOURS and `inputs` omits is a plain defect and gets declared — + * that is what objectui#3808 did to `record:details.hideFields`, + * `record:related_list.relationshipValueField`, `record:related_list.add` and + * `element:text_input.defaultValue`. The bar is that publishing the key would + * itself be wrong or premature, and WHICH of those it is has to be named: + * + * - the renderer does not read it, so publishing it would advertise + * configuration the platform silently drops (the objectui#3797 direction, in + * reverse) — the choice between wiring it and declaring it with a KNOWN GAP + * is a contract decision, not an implementation detail; + * - the spec rejects it by name upstream already and only a stale pin still + * lists it; + * - the key is out of the dispatched scope of the change that added this gate, + * and its own issue owns it. + * + * Every reason cites an issue, which `references a tracking issue` asserts. + * Verified against renderer read sites at objectui `origin/main` @ `c85268256` + * with `@objectstack/spec@17.0.0-rc.5` — not assumed from the spec's wording. + */ +const UNPUBLISHED_EXEMPTIONS: Record = { + // ── B class — spec declares it, NO renderer read point at all (2 keys) ───── + // The instinct here is to add an input, and it is wrong: that publishes a key + // the platform drops on the floor, which is exactly the defect objectui#3797 + // spent a repo-wide gate closing. The other instinct — wire it — is a visual + // decision (where an icon sits next to RecordTitleChip; whether a card grows + // an actions area, which reaches into `renderers/action/**`). The third option + // is the `record:activity.showSubscriptionToggle` precedent: declare it and + // say NOT IMPLEMENTED in the description, so both directions are in parity and + // the author is told. Three viable shapes, one public contract — filed as + // objectui#3829 rather than guessed at here. + 'page:header.icon': + 'Spec declares it; PageHeaderRenderer has NO read point — `icon` in containers.tsx:822-1570 is only ever per-action (`action.icon`, :1321/:1365) or a nav item (:604). Wire it, or declare it with a KNOWN GAP per the showSubscriptionToggle precedent: objectui#3829.', + 'page:card.actions': + 'Spec declares it; PageCardRenderer (containers.tsx:666-695) renders title/body/footer only and never reads `actions`. Wire it, or declare it with a KNOWN GAP per the showSubscriptionToggle precedent: objectui#3829.', + + // ── page:tabs.type — the carrier collision, from the other side ──────────── + // The mirror image of the `page:tabs.tabStyle` exemption in + // `OFF_SPEC_EXEMPTIONS` above, and the same single fact seen twice: the spec + // spells this concept `type`, the flat SDUI carrier cannot express it (a flat + // node is `{ type: 'page:tabs', … }` where `type` is the dispatch tag, and + // `SchemaRenderer.tsx:251-270` deliberately refuses to hoist + // `properties.type`), and `validate.ts` lists `'type'` in `BASE_PROPS` so it + // is skipped as a base prop and could not be validated as an input even if + // declared. Publishing it would advertise a key this repo's own parser cannot + // check, on a spelling the carrier cannot carry. Convergence is upstream. + 'page:tabs.type': + "Spec's spelling of the tabStyle concept; unpublishable in the flat carrier (`type` is the dispatch key, SchemaRenderer.tsx:251-270) and unvalidatable as an input (validate.ts BASE_PROPS). The renderer does read it when it survives as `properties.type` (containers.tsx:381). Upstream contract decision: objectstack#6776.", + + // ── element:record_picker — retired upstream, stale pin only (3 keys) ────── + // objectstack#5775 (ADR-0087 D2) turned these three into `retiredKey()` + // tombstones, converging on the `labelField` / `valueField` this renderer + // actually reads (`renderers/basic/record-picker.tsx:80-81`). Declaring a key + // the spec has retired is the objectui#3797 direction again. + // + // TWO THINGS THE PIN BUMP WILL DO HERE, and objectui#3808 got the first of + // them wrong, so it is written out: + // 1. these three do NOT vanish from `Object.keys(shape)`. ADR-0087 D2 + // retirement REPLACES the entry with `z.never().optional()`, it does not + // delete it — so they stay "declared" to this gate and these exemptions + // stay live rather than going stale. They resolve when objectui#3809's + // tombstone recognition narrows `specTopLevelKeys`, not when the pin + // moves; + // 2. `sort` / `limit` / `emptyText` — which #5775 ADDS and this renderer + // already reads (`record-picker.tsx:79/80` and `:170`) — become brand-new + // A-class gaps, and this gate will go RED demanding them. That red is + // correct and wanted: it is the pin bump's own reminder to declare them, + // the way `record:details.hideFields` was declared here. + 'element:record_picker.displayField': + 'Retired upstream by objectstack#5775 (ADR-0087 D2 tombstone, converging on the `labelField` this renderer reads); declaring it would publish a key the spec rejects by name. Listed here only because the pinned @objectstack/spec@17.0.0-rc.5 predates the retirement. Resolves via objectui#3809, not via the pin bump.', + 'element:record_picker.searchFields': + 'Retired upstream by objectstack#5775 (ADR-0087 D2 tombstone); declaring it would publish a key the spec rejects by name. Listed here only because the pinned @objectstack/spec@17.0.0-rc.5 predates the retirement. Resolves via objectui#3809, not via the pin bump.', + 'element:record_picker.multiple': + 'Retired upstream by objectstack#5775 (ADR-0087 D2 tombstone); declaring it would publish a key the spec rejects by name. Listed here only because the pinned @objectstack/spec@17.0.0-rc.5 predates the retirement. Resolves via objectui#3809, not via the pin bump.', + + // ── element:record_picker.filter — a real A-class gap, out of scope here ─── + // The renderer DOES read it (`record-picker.tsx:78`, `ds.filter ?? props.filter`, + // into `query.$filter` at :103) and the spec DOES declare it, so by the bar + // above this key should be declared, not exempted. It is exempted because + // objectui#3808's own three-class triage never sorted it into A, B or C — it + // appears in that issue's raw key dump and then in none of the three lists — + // so it fell outside the dispatched scope of the change that added this gate. + // Filed as objectui#3830 with the same evidence, rather than widened into a + // PR nobody reviewed for it. + 'element:record_picker.filter': + 'A genuine A-class gap (renderer reads it at record-picker.tsx:78 → query.$filter at :103), not a deliberate omission — it fell out of objectui#3808\'s three-class triage and so out of that PR\'s scope. Owned by objectui#3830; delete this entry when it declares the input.', + + // ── targetVariable — the spec's own "declarative hint" (2 keys) ──────────── + // Zero read points repo-wide (`grep -rn targetVariable packages/ apps/` is + // empty), and that is by design, not drift: the spec's describe says the live + // binding resolves via the variable whose `source` equals the component id, + // which is exactly what `usePageVariableBinding(schema?.id)` does + // (`text-input.tsx:60`). So publishing it is neither a fix nor a defect — it + // is a judgement about whether to publish an intent-only key, with a concrete + // risk on the publish side (an author who writes only `targetVariable` and no + // variable `source` gets an input that writes nowhere, silently). + 'element:text_input.targetVariable': + "Spec's own declarative hint with zero read points repo-wide; the live binding is the reverse lookup in usePageVariableBinding(schema.id) (text-input.tsx:60). Whether to publish an intent-only key is an open judgement: objectui#3834.", + 'element:record_picker.targetVariable': + "Spec's own declarative hint with zero read points repo-wide; the live binding is the reverse lookup by component id, as on element:text_input. Whether to publish an intent-only key is an open judgement: objectui#3834.", +}; + const exemptedFor = (type: string): string[] => Object.keys(OFF_SPEC_EXEMPTIONS) .filter((key) => key.startsWith(`${type}.`)) .map((key) => key.slice(type.length + 1)); +const unpublishedExemptedFor = (type: string): string[] => + Object.keys(UNPUBLISHED_EXEMPTIONS) + .filter((key) => key.startsWith(`${type}.`)) + .map((key) => key.slice(type.length + 1)); + describe('registry `inputs` vs `@objectstack/spec` ComponentPropsMap (repo-wide)', () => { it('judges every spec-carried block that declares an authoring surface', () => { // Non-vacuity guard. Every per-block assertion below is generated from @@ -336,4 +524,96 @@ describe('registry `inputs` vs `@objectstack/spec` ComponentPropsMap (repo-wide) }); expect(stale).toEqual([]); }); + + // ── the REVERSE direction (objectui#3808) ────────────────────────────────── + // + // Same `covered` set, same derived-not-restated expectations, same exemption + // discipline — only the subtraction is turned round: spec keys minus declared + // inputs, instead of declared inputs minus spec keys. + + it('every globally unpublished key is a real spec key on a covered block', () => { + // Non-vacuity for the blanket exclusion. `aria` is subtracted from EVERY + // block's expected surface, so a typo there (or the spec renaming the key) + // would quietly stop excluding anything while still reading as a documented + // decision — and, worse, would make the per-block assertion below start + // demanding an `aria` input on fifteen blocks for a reason nobody wrote down. + // + // Non-empty FIRST: a `for` over an emptied map — and the reason check below — + // both pass on nothing, so the map's own existence is the first assertion. + expect(Object.keys(GLOBALLY_UNPUBLISHED_SPEC_KEYS).length).toBeGreaterThan(0); + for (const key of Object.keys(GLOBALLY_UNPUBLISHED_SPEC_KEYS)) { + const carriers = covered.filter((type) => specTopLevelKeys(type).includes(key)); + expect(carriers.length, `no covered block's spec declares "${key}"`).toBeGreaterThan(0); + } + }); + + it('every globally unpublished key states a reason and references a tracking issue', () => { + const unjustified = Object.entries(GLOBALLY_UNPUBLISHED_SPEC_KEYS) + .filter(([, reason]) => !/#\d+/.test(reason)) + .map(([key]) => key); + expect(unjustified).toEqual([]); + }); + + it.each(covered)('%s publishes every top-level key its spec props schema declares', (type) => { + const exempt = new Set(unpublishedExemptedFor(type)); + const undiscoverable = undiscoverableSpecKeys(type).filter((key) => !exempt.has(key)); + expect(undiscoverable).toEqual([]); + }); + + it('every unpublished-key exemption names a key the spec really declares', () => { + // The dangling check, in the reverse direction. Two ways to be wrong here, + // and both read as deliberate cover while licensing nothing: a typo'd key, + // and an entry for a block this gate does not judge. + const dangling = Object.keys(UNPUBLISHED_EXEMPTIONS).filter((key) => { + const dot = key.indexOf('.'); + const type = key.slice(0, dot); + const specKey = key.slice(dot + 1); + return !covered.includes(type) || !specTopLevelKeys(type).includes(specKey); + }); + expect(dangling).toEqual([]); + }); + + it('every unpublished-key exemption states a reason and references a tracking issue', () => { + // The discipline that separates "deliberately not published, and here is who + // owns the decision" from "we forgot". Four of the nine entries below exist + // only because objectui#3829 / #3830 / #3834 were opened to own them. + const unjustified = Object.entries(UNPUBLISHED_EXEMPTIONS) + .filter(([, reason]) => !/#\d+/.test(reason)) + .map(([key]) => key); + expect(unjustified).toEqual([]); + }); + + it('carries no stale unpublished-key exemption — a published key must lose its entry', () => { + // Keeps the reverse list from rotting the same way. An entry goes stale when + // the block declares the input (objectui#3829/#3830/#3834 landing) or when + // the spec genuinely deletes the key — note that ADR-0087 D2 retirement is + // NOT a deletion, so the `element:record_picker` trio does not go stale on + // the pin bump; objectui#3809 is what resolves those. + const stale = Object.keys(UNPUBLISHED_EXEMPTIONS).filter((key) => { + const dot = key.indexOf('.'); + const type = key.slice(0, dot); + const specKey = key.slice(dot + 1); + return !undiscoverableSpecKeys(type).includes(specKey); + }); + expect(stale).toEqual([]); + }); + + it('the four keys objectui#3808 declared are discoverable, block by block', () => { + // Named, not just covered by the derived loop above. The derived assertion + // would also pass if these four were added to `UNPUBLISHED_EXEMPTIONS` + // instead of declared — which is precisely the move #3808 exists to rule + // out — so the keys it fixed are pinned by name, and pinned as DECLARED + // rather than merely "not failing". + const fixed: Array<[string, string]> = [ + ['record:details', 'hideFields'], + ['record:related_list', 'relationshipValueField'], + ['record:related_list', 'add'], + ['element:text_input', 'defaultValue'], + ]; + for (const [type, key] of fixed) { + expect(specTopLevelKeys(type), `${type} spec no longer declares ${key}`).toContain(key); + expect(declaredInputs(type) ?? [], `${type} does not publish ${key}`).toContain(key); + expect(Object.keys(UNPUBLISHED_EXEMPTIONS)).not.toContain(`${type}.${key}`); + } + }); }); diff --git a/packages/components/src/__tests__/text-input-inputs-spec-parity.test.ts b/packages/components/src/__tests__/text-input-inputs-spec-parity.test.ts new file mode 100644 index 0000000000..c2fb891a12 --- /dev/null +++ b/packages/components/src/__tests__/text-input-inputs-spec-parity.test.ts @@ -0,0 +1,131 @@ +/** + * 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. + * + * `element:text_input` — the published authoring surface stays in parity with + * `@objectstack/spec` `ElementTextInputProps` (objectui#3808). + * + * `text-input.test.tsx` next door already proves the RENDERER seeds a bound page + * variable from `defaultValue`. This file proves the complementary and, until + * #3808, false half: that an author can find out the key exists. + * + * WHY THIS BLOCK NEEDED IT MOST. `element:text_input` is deliberately NOT in + * `PUBLIC_BLOCKS` ("bare inputs belong to a form, not a page block", + * `packages/core/src/registry/public-blocks.ts:80`), so it never reaches + * `sdui.manifest.json` and the usual argument — "the manifest advertises it" — + * does not apply. Its `inputs` are a live contract anyway: + * `renderers/layout/page.tsx:462` builds the JSX-page compiler's prop whitelist + * from `getKnownTypes()` plus these same `inputs`, so while `defaultValue` was + * undeclared, `sdui-parser/src/validate.ts:74` reported `unknown-prop` for it on + * every JSX page — a warning against a key the renderer then went on to honour, + * with no way for the author to discover which of the two was right. + * + * Expectations are derived from the spec at runtime, not restated. + */ + +import { describe, it, expect } from 'vitest'; +import { ComponentRegistry } from '@object-ui/core'; +import { ElementTextInputPropsSchema } from '@objectstack/spec/ui'; +// Module scope, not a hook: the cold transform is billed to the import phase, +// which has no test/hook timeout (AGENTS.md §测试纪律, objectui#3010). +import '../renderers'; + +type ShapeCarrier = { shape?: unknown; _def?: { shape?: unknown } }; + +/** Resolve the props object's `.shape` through both spellings, lazy or plain. */ +function specTopLevelKeys(): string[] { + const carrier = ElementTextInputPropsSchema as unknown as ShapeCarrier; + const shape = carrier.shape ?? carrier._def?.shape; + const resolved = typeof shape === 'function' ? (shape as () => object)() : shape; + return resolved && typeof resolved === 'object' ? Object.keys(resolved) : []; +} + +const config = () => ComponentRegistry.getConfig('element:text_input'); +const inputs = () => config()?.inputs ?? []; +const inputNames = () => inputs().map((i) => i.name); +const input = (name: string) => inputs().find((i) => i.name === name); +const defaultValueDescription = () => input('defaultValue')?.description ?? ''; + +describe('element:text_input — registry inputs vs @objectstack/spec', () => { + it('is registered with a non-empty `inputs` surface', () => { + expect(config()).toBeDefined(); + expect(inputNames().length).toBeGreaterThan(0); + }); + + it('resolves a non-empty spec key set', () => { + // Guards the probe, not the subject: a Zod internals change would return `[]` + // here and make every assertion below vacuously agreeable. + expect(specTopLevelKeys().length).toBeGreaterThan(0); + }); + + it('declares no top-level input the spec does not accept', () => { + const allowed = new Set(specTopLevelKeys()); + expect(inputNames().filter((name) => !allowed.has(name))).toEqual([]); + }); + + it('publishes `defaultValue`, which the renderer has read all along', () => { + // A KEY-reachability claim, so the criterion is that the key SURVIVES the + // parse — not that the parse succeeds. This props schema is a strip-mode + // `z.object`, so an UNDECLARED key parses green too and is simply absent from + // `data` afterwards; asserting `success` alone would prove nothing at all. + expect(specTopLevelKeys()).toContain('defaultValue'); + const parsed = ElementTextInputPropsSchema.safeParse({ defaultValue: 'acme' }); + expect(parsed.success).toBe(true); + expect(parsed.data?.defaultValue).toBe('acme'); + + // The contrast that makes the criterion meaningful: same green parse, key + // gone, no diagnostic. That is what `defaultValue` looked like to every + // manifest consumer before it was declared here. + const undeclared = ElementTextInputPropsSchema.safeParse({ notASpecKey: 1 } as never); + expect(undeclared.success).toBe(true); + expect(Object.keys(undeclared.data ?? {})).not.toContain('notASpecKey'); + + expect(inputNames()).toContain('defaultValue'); + expect(defaultValueDescription()).not.toBe(''); + }); + + it('names the number arm the coarse `type` cannot express', () => { + // The spec's type is the union `string | number`; `ComponentInput.type` is one + // coarse control kind, so `'string'` is a real narrowing — + // `sdui-parser`'s `checkType` warns `type-mismatch` on `defaultValue={42}`, + // which the spec accepts. The narrowing is not the thing being asserted (it + // is a `ComponentInput` limit, tracked as objectui#3832); what is asserted is + // that the description does not hide it, so an author reaching for a numeric + // default knows the key takes one and knows why the warning appears. + expect(ElementTextInputPropsSchema.safeParse({ defaultValue: 42 }).success).toBe(true); + expect(ElementTextInputPropsSchema.safeParse({ defaultValue: true } as never).success).toBe(false); + + expect(input('defaultValue')?.type).toBe('string'); + expect(defaultValueDescription()).toMatch(/number/); + }); + + it('the `defaultValue` description says which of the two behaviours an author gets', () => { + // The seeding path and the uncontrolled-input path do different things, and + // which one applies depends on something the block does not own: whether a + // page variable's `source` points at this component's id. A description + // saying only "initial value" would be true and useless — the author of a + // form that submits `page.` needs to know the seed happens once, only + // while the variable is empty, and that the variable's own default wins. + const description = defaultValueDescription(); + expect(description).toMatch(/source/); + expect(description).toMatch(/once/i); + expect(description).toMatch(/empty/i); + }); + + it('carries no `defaultValue` OF ITS OWN on the defaultValue entry', () => { + // A `ComponentInput.defaultValue` on this input would publish a default for + // the default — the designer would pre-fill a seed value the renderer has no + // opinion about, and every text input in the gallery would come up carrying + // it. The spec declares no default here either. + // + // Existence asserted first: `input('defaultValue')?.defaultValue` is also + // `undefined` when the input is GONE, so without this line the check would + // pass most loudly in the one case it is supposed to notice. + expect(input('defaultValue')).toBeDefined(); + expect(input('defaultValue')?.defaultValue).toBeUndefined(); + expect(ElementTextInputPropsSchema.safeParse({}).data).not.toHaveProperty('defaultValue'); + }); +}); diff --git a/packages/components/src/renderers/basic/text-input.tsx b/packages/components/src/renderers/basic/text-input.tsx index 5d184f9e15..71fc73f85f 100644 --- a/packages/components/src/renderers/basic/text-input.tsx +++ b/packages/components/src/renderers/basic/text-input.tsx @@ -131,6 +131,17 @@ ComponentRegistry.register('text_input', ElementTextInputRenderer, { skipFallback: true, label: 'Text Input', category: 'input', + // `defaultValue` is DECLARED, not merely honoured (objectui#3808). The + // renderer has read it since the seeding effect landed, and the spec declares + // it (`ElementTextInputProps.defaultValue`, `string | number`) — but while it + // was missing from this list an author could not discover it, and every layer + // that reads a manifest said the opposite: `page.tsx`'s JSX-page compiler + // builds its prop whitelist from `getKnownTypes()` + these `inputs`, so + // `` came back as an `unknown-prop` + // warning on a key the renderer then went on to honour. That is objectui#3407 + // in the same shape as `readonly` — enforced, undiscoverable — and the + // reverse half of the parity gate in + // `apps/console/src/__tests__/registry-inputs-spec-parity.test.ts`. inputs: [ { name: 'label', type: 'string', label: 'Label' }, { name: 'placeholder', type: 'string', label: 'Placeholder' }, @@ -141,6 +152,30 @@ ComponentRegistry.register('text_input', ElementTextInputRenderer, { enum: ['text', 'email', 'number', 'tel', 'url', 'password'], defaultValue: 'text', }, + { + name: 'defaultValue', + // The spec's type is the union `string | number`, which `ComponentInput` + // has no way to spell — its `type` is one coarse control kind. `'string'` + // is the arm chosen here (a text input's ordinary case, and the DOM value + // is `String(...)`-coerced anyway) and the number arm is named in the + // description, following the same call made for the inline-translation + // shapes on `page:header.title` / `record:alert.title`. It is a real + // narrowing, not a free choice: `sdui-parser`'s `checkType` warns + // `type-mismatch` on `defaultValue={42}`, a value the spec accepts. The + // limit is `ComponentInput`'s, tracked separately — it is the union twin + // of the member-shape limit PR #3795 left open. + type: 'string', + label: 'Default Value', + // Description taken from what the renderer DOES with the key (the seeding + // effect above, and the native `defaultValue` pass-through at the + // ``), not from restating the spec's one-liner — the two + // behaviours differ depending on whether a page variable targets this + // input, and an author who only knew "initial value" would not know which + // one they get. No `defaultValue` on this entry: the value IS the default, + // so a default-for-the-default would be meaningless. + description: + 'Initial value (string or number). With a page variable bound to this input — a variable whose `source` is this component id — it is pushed into that variable ONCE on mount, and only while the variable is still empty, so `page.` and the submit body carry it before the user types; a variable that declares its own defaultValue wins. With no bound variable it becomes the native input\'s uncontrolled initial value and nothing else reads it.', + }, { name: 'required', type: 'boolean', label: 'Required' }, { name: 'disabled', type: 'boolean', label: 'Disabled' }, { name: 'description', type: 'string', label: 'Description' }, diff --git a/packages/plugin-detail/src/__tests__/recordDetailsInputs.spec-parity.test.ts b/packages/plugin-detail/src/__tests__/recordDetailsInputs.spec-parity.test.ts index f92ac0ab43..153f881810 100644 --- a/packages/plugin-detail/src/__tests__/recordDetailsInputs.spec-parity.test.ts +++ b/packages/plugin-detail/src/__tests__/recordDetailsInputs.spec-parity.test.ts @@ -180,6 +180,57 @@ describe('record:details — registry inputs vs @objectstack/spec', () => { expect(offSpec).toEqual([]); }); + it('publishes `hideFields`, which the renderer has read all along', () => { + // The reverse direction on this block (objectui#3808). `hideFields` was + // declared by the spec (objectstack#5611) and read by + // `renderers/record-details.tsx:147` while `inputs` omitted it, so the + // manifest, the generated `.d.ts` and the designer panel all said the key + // did not exist and `sdui-parser` reported `unknown-prop` on an author who + // wrote it anyway — while the renderer honoured it. Same shape as `readonly` + // in objectui#3407. + // + // A KEY-reachability claim, so the criterion is that the key SURVIVES the + // parse. These props schemas are strip-mode `z.object`s: an undeclared key + // is dropped from `data` with no error at all, which is exactly why the gap + // was silent — so "it is still there afterwards" is the proof, not + // `success === true` (which an undeclared key also gets). + expect(specTopLevelKeys()).toContain('hideFields'); + const parsed = RecordDetailsProps.safeParse({ hideFields: ['phone'] }); + expect(parsed.success).toBe(true); + expect(parsed.data?.hideFields).toEqual(['phone']); + + expect(inputs().map((i) => i.name)).toContain('hideFields'); + expect(input('hideFields')?.description ?? '').not.toBe(''); + }); + + it('`hideFields` documents bare names only, because the spec rejects entry objects', () => { + // The same fence `fields` is held to below, on the sibling key — and here it + // is a VALUE verdict, so the criterion is a full parse either way: the object + // spelling has to be rejected on its value, not merely stripped. + // + // The renderer is more tolerant than the contract at this read site + // (`typeof n === 'string' ? n : fieldName(n)`), which is not a second + // contract to advertise. Every in-repo producer passes strings + // (`synth/buildDefaultPageSchema.ts:557-562` types it `string[]`), so the + // tolerant arm is unexercised drift rather than a live dialect. + const element = arrayElement(shapeMember(RecordDetailsProps, 'hideFields')); + expect(shapeKeys(element)).toEqual([]); + expect(RecordDetailsProps.safeParse({ hideFields: ['phone'] }).success).toBe(true); + + const objectForm = RecordDetailsProps.safeParse({ hideFields: [{ name: 'phone' }] }); + expect(objectForm.success).toBe(false); + expect(objectForm.error?.issues.map((i) => i.code)).toContain('invalid_type'); + + // Non-empty FIRST. A `not.toContain('{')` on a description that does not + // exist passes for the wrong reason — the reverse-verification run for + // objectui#3808 deleted the `hideFields` declaration and watched this + // assertion stay green on `''` while the three assertions that matter went + // red. An empty description is a failure here, not a vacuous pass. + const description = input('hideFields')?.description ?? ''; + expect(description).not.toBe(''); + expect(description).not.toContain('{'); + }); + it('`fields` documents no entry shape, because the spec accepts bare names only', () => { // objectui#3807's fence check on the sibling input at the same call site. // Top-level `fields` is `z.array(z.string())`: there is no member shape to diff --git a/packages/plugin-detail/src/__tests__/recordRelatedListInputs.spec-parity.test.ts b/packages/plugin-detail/src/__tests__/recordRelatedListInputs.spec-parity.test.ts new file mode 100644 index 0000000000..35bf773ecf --- /dev/null +++ b/packages/plugin-detail/src/__tests__/recordRelatedListInputs.spec-parity.test.ts @@ -0,0 +1,176 @@ +/** + * 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. + * + * `record:related_list` — the published authoring surface stays in parity with + * `@objectstack/spec` `RecordRelatedListProps` (objectui#3808). + * + * Third sibling of `recordHighlightsInputs.spec-parity.test.ts` (objectui#3407 / + * PR #3795) and `recordDetailsInputs.spec-parity.test.ts` (objectui#3807), added + * for the direction those two carry and the repo-wide gate had not: a top-level + * key the SPEC declares must be discoverable from `inputs`. + * + * This block had the worst instance of it. `relationshipValueField` and `add` + * were both spec keys the renderer had honoured all along — + * `renderers/record-related-list.tsx:95` and `:186` — while `inputs` published + * neither, and nothing anywhere reported the mismatch: + * `gen-manifest.ts` left them out of `sdui.manifest.json` and + * `sdui-intrinsics.d.ts`, `sdui-parser/src/validate.ts:74` returned + * `unknown-prop` for an author who wrote one, and the renderer went on honouring + * it. `add` in particular is the ONLY way to build a junction-assignment list, + * so the single published route to that feature was an undiscoverable key. + * + * WHY A DESCRIPTION IS WORTH A TEST, and why `add`'s is checked member by + * member: `ComponentInput` is flat by design, so an `object` input can document + * its member shape nowhere but its own prose. The assertions below derive the + * member list from the spec at runtime, so a spec change fails here rather than + * leaving the description quietly incomplete — the failure mode objectui#3807 + * was filed for. + */ + +import { describe, it, expect } from 'vitest'; +import { ComponentRegistry } from '@object-ui/core'; +import { RecordRelatedListProps } from '@objectstack/spec/ui'; +import '../index'; + +type ShapeCarrier = { shape?: unknown; _def?: { shape?: unknown } }; + +/** Resolve a Zod object's `.shape` through both spellings, lazy or plain. */ +function shapeKeys(schema: unknown): string[] { + const carrier = schema as ShapeCarrier | undefined; + const shape = carrier?.shape ?? carrier?._def?.shape; + const resolved = typeof shape === 'function' ? (shape as () => object)() : shape; + return resolved && typeof resolved === 'object' ? Object.keys(resolved) : []; +} + +/** + * One entry of `.shape`, unwrapped past the optional/default wrappers until an + * object shape is reachable. `add` is `.optional()` and `add.picker` carries + * defaults on its own members, so a single `.unwrap()` is not enough. + */ +function innerObject(schema: unknown, key: string): unknown { + const carrier = schema as ShapeCarrier | undefined; + const shape = carrier?.shape ?? carrier?._def?.shape; + const resolved = (typeof shape === 'function' ? (shape as () => object)() : shape) as + | Record + | undefined; + let member = resolved?.[key] as + | { shape?: unknown; _def?: { shape?: unknown; innerType?: unknown; type?: unknown } } + | undefined; + for (let hop = 0; hop < 8; hop += 1) { + if (member?.shape ?? member?._def?.shape) return member; + const next = member?._def?.innerType ?? member?._def?.type; + if (!next || typeof next !== 'object') return member; + member = next as typeof member; + } + return member; +} + +const specTopLevelKeys = (): string[] => shapeKeys(RecordRelatedListProps); +const specAddKeys = (): string[] => shapeKeys(innerObject(RecordRelatedListProps, 'add')); +const specPickerKeys = (): string[] => + shapeKeys(innerObject(innerObject(RecordRelatedListProps, 'add'), 'picker')); + +const config = () => ComponentRegistry.getConfig('record:related_list'); +const inputs = () => config()?.inputs ?? []; +const inputNames = () => inputs().map((i) => i.name); +const input = (name: string) => inputs().find((i) => i.name === name); +const addDescription = () => input('add')?.description ?? ''; + +/** A spec-valid related list, so a fixture only ever carries the key under test. */ +const baseline = { objectName: 'task', relationshipField: 'account', columns: ['name'] }; + +describe('record:related_list — registry inputs vs @objectstack/spec', () => { + it('is registered with a non-empty `inputs` surface', () => { + expect(config()).toBeDefined(); + expect(inputNames().length).toBeGreaterThan(0); + }); + + it('declares no top-level input the spec does not accept', () => { + const allowed = new Set(specTopLevelKeys()); + expect(inputNames().filter((name) => !allowed.has(name))).toEqual([]); + }); + + it('publishes `relationshipValueField`, which the renderer has read all along', () => { + // A KEY-reachability claim, so the criterion is that the key SURVIVES the + // parse rather than merely that the parse succeeds. `RecordRelatedListProps` + // is a strip-mode `z.object`: an UNDECLARED key also parses green, it is just + // silently gone from `data` afterwards — which is exactly how this gap stayed + // invisible for as long as it did. + expect(specTopLevelKeys()).toContain('relationshipValueField'); + const parsed = RecordRelatedListProps.safeParse({ ...baseline, relationshipValueField: 'name' }); + expect(parsed.success).toBe(true); + expect(parsed.data?.relationshipValueField).toBe('name'); + + expect(inputNames()).toContain('relationshipValueField'); + }); + + it("`relationshipValueField` publishes the renderer's default, and it matches the read site", () => { + // `record-related-list.tsx:95` is `schema.relationshipValueField || 'id'`, and + // the input carries `defaultValue: 'id'` to match. Pinned because a default + // published on the authoring surface that disagrees with the renderer is + // worse than no default: the designer would pre-fill one value and the page + // would behave as another, with nothing comparing them. + expect(input('relationshipValueField')?.defaultValue).toBe('id'); + + // And the spec agrees, so all three say `id`. + expect(RecordRelatedListProps.safeParse(baseline).data?.relationshipValueField ?? 'id').toBe('id'); + }); + + it('publishes `add`, the only route to a junction-assignment list', () => { + expect(specTopLevelKeys()).toContain('add'); + const parsed = RecordRelatedListProps.safeParse({ + ...baseline, + add: { picker: { object: 'sys_position' }, linkField: 'position', label: 'Assign' }, + }); + expect(parsed.success).toBe(true); + expect(parsed.data?.add?.picker?.object).toBe('sys_position'); + + expect(inputNames()).toContain('add'); + expect(input('add')?.type).toBe('object'); + expect(addDescription()).not.toBe(''); + }); + + it('every spec member key of `add` is discoverable from its description', () => { + // `ComponentInput` has no member-shape slot (the LIMIT the repo-wide gate in + // `apps/console/src/__tests__/registry-inputs-spec-parity.test.ts` documents), + // so this prose is the only published description of the shape. Derived from + // the spec at runtime: a member key added upstream fails here instead of + // going unmentioned. + expect(specAddKeys().length).toBeGreaterThan(0); + expect(specPickerKeys().length).toBeGreaterThan(0); + + const description = addDescription(); + expect(specAddKeys().filter((key) => !description.includes(key))).toEqual([]); + expect(specPickerKeys().filter((key) => !description.includes(key))).toEqual([]); + }); + + it("`add`'s published defaults are the RENDERER's, not the spec's prose", () => { + // The one place the two disagree. `RelatedList.tsx:724` defaults + // `picker.valueField` to `id` — same as the spec — but `:390` defaults + // `picker.labelField` to `name`, where the spec's `.describe()` says it + // "defaults to the object title field". The description publishes what the + // platform does; publishing the spec's wording would document a behaviour no + // code implements. + const description = addDescription(); + expect(description).toMatch(/labelField[^.]*default "name"/); + expect(description).not.toMatch(/labelField[^.]*title field/); + }); + + it('names `picker.filter` as a gap instead of documenting it as a restriction', () => { + // The `record:activity.showSubscriptionToggle` precedent applied at member + // level. The spec declares `add.picker.filter` ("Restrict which records the + // picker offers") and nothing in this repo reads it — `RelatedList` fills + // `RecordPickerDialog`'s `objectName` / `displayField` / `columns` and never + // its `baseFilter` slot. A description that merely listed `filter` among the + // members would tell an author their picker is scoped when it offers every + // record; objectui#3831 owns the wiring, and this assertion fails the moment + // someone deletes the warning without doing it. + expect(specPickerKeys()).toContain('filter'); + expect(addDescription()).toMatch(/KNOWN GAP/); + expect(addDescription()).toMatch(/filter[\s\S]*not applied/); + }); +}); diff --git a/packages/plugin-detail/src/index.tsx b/packages/plugin-detail/src/index.tsx index 8c7095ebf6..5b20ac13da 100644 --- a/packages/plugin-detail/src/index.tsx +++ b/packages/plugin-detail/src/index.tsx @@ -274,6 +274,25 @@ ComponentRegistry.register('details', RecordDetailsRenderer, { { name: 'layout', type: 'enum', label: 'Layout', enum: ['auto', 'custom'], defaultValue: 'auto', description: 'auto uses the object highlightFields; custom uses explicit sections' }, { name: 'sections', type: 'array', label: 'Sections', description: 'Field groups rendered as the detail body, in order. Every entry is an OBJECT — `{ name?, label?, columns?, fields }` — a bare section-id string is NOT accepted (the spec retired that spelling in objectstack#5611, and the renderer reads name/label/fields off each entry, so a string entry renders no fields at all). `fields` (required) are the field names shown in this section, in order. `label` is the section heading; omit it for an untitled, borderless section. `name` is a stable snake_case identifier and the i18n anchor — the heading resolves through objects.._sections..label, so a section without a name shows its authored label in every locale. `columns` (1-4) is THIS section\'s field-grid width; omit it and the renderer derives the width. Required when layout is "custom", where sections are the only source of the detail body.' }, { name: 'fields', type: 'array', label: 'Fields', description: 'Explicit field list (overrides highlightFields)' }, + // `hideFields` is DECLARED, not merely honoured (objectui#3808). The spec + // declares it (objectstack#5611) and `RecordDetailsRenderer` has read it + // since the highlight-dedup phase (`renderers/record-details.tsx:147`), but + // it was missing here — so an author reading the manifest could not + // discover it, and every layer that reads the manifest said the opposite: + // `sdui.manifest.json` / `sdui-intrinsics.d.ts` omitted it and + // `sdui-parser`'s prop walk reported `unknown-prop` on a key the renderer + // then honoured. Same failure as `readonly` in objectui#3407. + // + // Bare field NAMES only. The renderer also tolerates `{name}` / `{field}` + // entries (`typeof n === 'string' ? n : fieldName(n)` at the read site), but + // `hideFields` is `z.array(z.string())` in the spec and rejects those values + // on parse — teaching them here would publish a second dialect the contract + // refuses, the same fence `fields` above is held to. + // + // The "hiding every field drops the section" sentence is read off + // `DetailSection.tsx:439` (`visibleFields.length === 0 && + // emptyCount === section.fields.length` returns null), not assumed. + { name: 'hideFields', type: 'array', label: 'Hide Fields', description: 'Field names to omit from the body — applied to the top-level `fields` list AND to every section\'s `fields`. Bare field names only. Authors rarely need it: the synth pipeline fills it with the fields already shown in `record:highlights`, and hand-authored pages get the same dedup live from HighlightFieldsContext, so its purpose is suppressing a field you do not want repeated (the page H1 title field is dropped for you too). Hiding every field of a section leaves that section out entirely.' }, ], }); @@ -284,9 +303,20 @@ ComponentRegistry.register('related_list', RecordRelatedListRenderer, { label: 'Related List', icon: 'List', // Mirrors @objectstack/spec RecordRelatedListProps. + // + // `relationshipValueField` and `add` are DECLARED, not merely honoured + // (objectui#3808). Both are spec keys this renderer has read all along — + // `renderers/record-related-list.tsx:95` and `:186` — while `inputs` omitted + // them, so the published surface and the runtime disagreed in the direction + // nothing reports: `sdui.manifest.json` / `sdui-intrinsics.d.ts` never + // mentioned them, `sdui-parser`'s prop walk raised `unknown-prop` on an + // author who wrote one anyway, and the renderer honoured it regardless. `add` + // in particular is not cosmetic — without it declared, the ONLY published way + // to build a junction-assignment list was to write an undiscoverable key. inputs: [ { name: 'objectName', type: 'string', label: 'Related Object', required: true, description: 'Related object name (e.g. "task")' }, { name: 'relationshipField', type: 'string', label: 'Relationship Field', required: true, description: 'Field on the related object pointing back to this record' }, + { name: 'relationshipValueField', type: 'string', label: 'Relationship Value Field', defaultValue: 'id', description: 'Which field OF THIS PARENT record `relationshipField` stores. Defaults to "id"; set it to the field a name-keyed junction points at (e.g. "name" when sys_user_position.position holds sys_position.name). The resolved value drives three things at once — the list filter, the Add-picker link value, and the pre-filled create form — so they cannot drift apart. While the parent record is still loading, a non-"id" field resolves to null and the list holds its fetch rather than querying on an empty value.' }, { name: 'columns', type: 'array', label: 'Columns', required: true, description: 'Fields to display in the related list' }, { name: 'sort', type: 'array', label: 'Sort' }, { name: 'limit', type: 'number', label: 'Limit', defaultValue: 5, description: 'Records to display initially' }, @@ -294,6 +324,29 @@ ComponentRegistry.register('related_list', RecordRelatedListRenderer, { { name: 'title', type: 'string', label: 'Title' }, { name: 'showViewAll', type: 'boolean', label: 'Show "View All"', defaultValue: true }, { name: 'actions', type: 'array', label: 'Actions', description: 'Action IDs available for related records' }, + // `add` publishes its MEMBER shape in prose for the reason the sibling + // array-of-objects inputs do (`record:details.sections`, + // `record:highlights.fields`, `record:path.stages`): `ComponentInput` is flat + // by design and has no slot for a member shape, so an `object` input can + // only document its members here. + // + // Documented members are exactly the spec's — `picker.object`, + // `picker.valueField`, `picker.labelField`, `linkField`, `label` — with each + // default taken from the RENDERER, which is where an author's expectation + // gets settled: `RelatedList.tsx:724` defaults `picker.valueField` to `id` + // (matching the spec's own default) but `:390` defaults `picker.labelField` + // to `name`, NOT to the object's title field as the spec's `.describe()` + // says. Publishing the spec's wording there would have been a description + // the platform does not honour. + // + // `picker.filter` is deliberately NOT documented as working: the spec + // declares it, and nothing in this repo reads it — `RelatedList` passes + // `picker.object` / `labelField` to `RecordPickerDialog` and never fills its + // `baseFilter` slot. Naming it here as a gap follows the + // `record:activity.showSubscriptionToggle` precedent above; silently + // documenting it as a restriction would tell an author their picker is + // scoped when it offers every record. + { name: 'add', type: 'object', label: 'Add Existing', description: 'Adds an "Add" button that assigns EXISTING records instead of creating one — the m2m/junction case. Shape: `{ picker: { object, valueField?, labelField?, filter? }, linkField?, label? }`. `picker.object` (required) is the object whose records the dialog offers. `picker.valueField` is the field of the picked record used as the link value (default "id"); `picker.labelField` is the column shown in the picker rows (default "name", and the other columns are derived from that object\'s schema). With `linkField` set, selecting records CREATES rows in this list\'s own object as `{ [relationshipField]: parentValue, [linkField]: pickedId }` — the junction case; omit `linkField` and the picked child is RE-PARENTED instead, by setting its own `relationshipField` to this parent. `label` is the button text (default "Add", localizable inline). Setting `add` also enables generic link removal on rows when no host delete handler is wired. KNOWN GAP: `picker.filter` is accepted by the spec but not applied — the dialog offers every record of `picker.object` whatever you put there.' }, ], });