diff --git a/.changeset/record-picker-flat-sort-limit.md b/.changeset/record-picker-flat-sort-limit.md new file mode 100644 index 0000000000..add78d19f4 --- /dev/null +++ b/.changeset/record-picker-flat-sort-limit.md @@ -0,0 +1,54 @@ +--- +"@objectstack/spec": minor +--- + +feat(spec): `element:record_picker` declares the flat `sort` / `limit` shorthands the renderer already honours (#6276) + +`element:record_picker` resolves its query from four keys through one identical +pattern — the component-level `dataSource` first, the flat `properties` +shorthand second: + +```ts +const object = ds.object ?? props.object; +const filter = ds.filter ?? props.filter; +const sort = ds.sort ?? props.sort; +const limit = ds.limit ?? props.limit ?? 50; +``` + +After #5775 the first two flat spellings were declared and the last two were +not, so one renderer read half a contract and half a trapdoor. An author who +inferred `properties.limit: 20` from the declared `object` / `filter` spelling +got the renderer's default 50 with **zero diagnostics**: the key was stripped +before anything could read it, the #5068 gate reported it as undeclared, and no +value check ever ran. That is the ADR-0078 shape, on the element that had just +been rewritten to remove it. + +**Additive — nothing that parsed before stops parsing.** Both keys are optional +and take the shape `ElementDataSourceSchema` already declares for its own +`sort` / `limit`, deliberately: they are the same contract read through a second +spelling, so a divergent shape here would be a third sort dialect rather than a +shorthand. + +```ts +// now declared, retained, and value-checked +{ + type: 'element:record_picker', + properties: { + object: 'showcase_project', + sort: [{ field: 'created_at', order: 'desc' }], // SortItem[] + limit: 20, // positive integer + }, +} +``` + +`dataSource.sort` / `dataSource.limit` still win when both are written — the +declaration documents the precedence, it does not change it. The renderer's +`?? 50` stays a **renderer** fallback and is deliberately not a schema default: +`.default(50)` would materialize a limit on every parsed picker and turn an +unset key into an authored one. + +The maintainer's ruling (2026-08-08, direction A) is the #5611 rule applied +again — the delivered, authorized shape is the contract. Direction B, retiring +the whole flat family in favour of a single `dataSource` door, was not dropped: +it is a cross-element decision (`element:form` / `element:filter` carry the same +flat `object`), tracked as #6590 for v18, and this change does not block it. diff --git a/content/docs/references/ui/component.mdx b/content/docs/references/ui/component.mdx index 04ca6e1ef5..c0a9d2fbea 100644 --- a/content/docs/references/ui/component.mdx +++ b/content/docs/references/ui/component.mdx @@ -147,6 +147,8 @@ const result = AIChatWindowProps.parse(data); | **valueField** | `string` | optional | Field whose value is written into the bound page variable (default `id`) | | **label** | `string \| Record` | optional | Control label rendered above the select | | **filter** | `any` | optional | Filter criteria for available records | +| **sort** | `{ field: string; order: Enum<'asc' \| 'desc'> }[]` | optional | Row order — synonym of the component-level `dataSource.sort`, which takes precedence when both are set | +| **limit** | `integer` | optional | Max records offered — synonym of the component-level `dataSource.limit`, which takes precedence when both are set (renderer default 50) | | **targetVariable** | `string` | optional | Page variable to bind selected record ID(s) | | **placeholder** | `string \| Record` | optional | Placeholder text | | **emptyText** | `string \| Record` | optional | Text shown when the query returns no records (default "No records") | diff --git a/packages/lint/src/validate-component-props.test.ts b/packages/lint/src/validate-component-props.test.ts index 4a3ad3b857..9711d60ff9 100644 --- a/packages/lint/src/validate-component-props.test.ts +++ b/packages/lint/src/validate-component-props.test.ts @@ -240,6 +240,48 @@ describe('validateComponentProps — value verdicts', () => { expect(findings).toEqual([]); }); + /** + * #6276 — the #5068 worklist entry #5775's key-by-key ruling left behind. + * The picker's renderer reads FOUR keys through one `ds. ?? props.` + * pattern; two of the flat spellings were declared and two were not, so this + * gate reported `sort`/`limit` as undeclared while the renderer honoured + * them. Both halves of the rule are asserted, because they fail differently: + * the key must stop being an unknown-key finding, and its VALUE must now be + * judged (before the declaration a wrong `limit` was stripped in silence). + */ + it('reports nothing on the flat `sort` / `limit` shorthands the picker honours (#6276)', () => { + const findings = validateComponentProps( + stackWith([ + { + type: 'element:record_picker', + id: 'project_picker', + properties: { + object: 'showcase_project', + labelField: 'name', + sort: [{ field: 'created_at', order: 'desc' }], + limit: 20, + }, + }, + ]), + ); + expect(findings).toEqual([]); + }); + + it('now judges the VALUE of a flat `limit` instead of stripping it (#6276)', () => { + const findings = validateComponentProps( + stackWith([ + { + type: 'element:record_picker', + properties: { object: 'showcase_project', limit: 'twenty' }, + }, + ]), + ); + expect(invalid(findings).map((f) => f.path)).toEqual([ + 'pages[0].regions[0].components[0].properties.limit', + ]); + expect(invalid(findings)[0].message).toContain('expected number, received string'); + }); + /** * The container half of #5775. `page:card` `children` and the three thin * containers' `children` were all reported as unknown keys while the diff --git a/packages/spec/authorable-surface/ui.json b/packages/spec/authorable-surface/ui.json index 99fab194ca..7c77b4cb6b 100644 --- a/packages/spec/authorable-surface/ui.json +++ b/packages/spec/authorable-surface/ui.json @@ -414,10 +414,12 @@ "ui/ElementRecordPickerProps:filter", "ui/ElementRecordPickerProps:label", "ui/ElementRecordPickerProps:labelField", + "ui/ElementRecordPickerProps:limit", "ui/ElementRecordPickerProps:multiple [RETIRED]", "ui/ElementRecordPickerProps:object", "ui/ElementRecordPickerProps:placeholder", "ui/ElementRecordPickerProps:searchFields [RETIRED]", + "ui/ElementRecordPickerProps:sort", "ui/ElementRecordPickerProps:targetVariable", "ui/ElementRecordPickerProps:valueField", "ui/ElementTextInputProps:aria", diff --git a/packages/spec/src/ui/component.test.ts b/packages/spec/src/ui/component.test.ts index 0f7fbe7bf6..d27a168ca1 100644 --- a/packages/spec/src/ui/component.test.ts +++ b/packages/spec/src/ui/component.test.ts @@ -20,7 +20,7 @@ import { ElementRecordPickerPropsSchema, ElementTextInputPropsSchema, } from './component.zod'; -import { PageComponentSchema, PageSchema } from './page.zod'; +import { PageComponentSchema, PageSchema, ElementDataSourceSchema } from './page.zod'; describe('PageHeaderProps', () => { it('should accept minimal header', () => { @@ -844,6 +844,77 @@ describe('Interactive Elements — element:record_picker', () => { expect(props).not.toHaveProperty('searchFields'); expect(props).not.toHaveProperty('multiple'); }); + + // ── #6276 — the flat `sort` / `limit` shorthands ───────────────────────── + // The renderer resolves four keys through one pattern + // (`ds. ?? props.`); after #5775 two of the four flat spellings were + // declared and two were not. These pin the other two, in BOTH halves of what + // a declaration buys: the key is retained (not stripped into silence) and the + // VALUE is judged (a wrong shape is rejected by name rather than dropped). + it('retains the flat `sort` shorthand — declared, not stripped (#6276)', () => { + const props = ElementRecordPickerPropsSchema.parse({ + object: 'showcase_project', + sort: [{ field: 'created_at', order: 'desc' }], + }); + expect(props.sort).toEqual([{ field: 'created_at', order: 'desc' }]); + }); + + it('retains the flat `limit` shorthand — declared, not stripped (#6276)', () => { + const props = ElementRecordPickerPropsSchema.parse({ object: 'showcase_project', limit: 20 }); + expect(props.limit).toBe(20); + }); + + // The exact ADR-0078 trap the issue reported: an author who infers + // `properties.limit: 20` from the declared `object`/`filter` spelling used to + // get the renderer's default 50 with zero diagnostics, because the key was + // stripped before anything could read it. + it('rejects a non-integer / non-positive `limit` by name (#6276)', () => { + expect(() => ElementRecordPickerPropsSchema.parse({ object: 'a', limit: 0 })).toThrow(/limit/); + expect(() => ElementRecordPickerPropsSchema.parse({ object: 'a', limit: -5 })).toThrow(/limit/); + expect(() => ElementRecordPickerPropsSchema.parse({ object: 'a', limit: 2.5 })).toThrow(/limit/); + expect(() => ElementRecordPickerPropsSchema.parse({ object: 'a', limit: 'ten' })).toThrow(/limit/); + }); + + it('rejects a malformed `sort` by name (#6276)', () => { + // A bare field name — the shape an author reaches for when the key is + // undeclared and nothing has ever told them otherwise. + expect(() => ElementRecordPickerPropsSchema.parse({ object: 'a', sort: 'created_at' })) + .toThrow(/sort/); + // Right container, wrong direction vocabulary. + expect(() => ElementRecordPickerPropsSchema.parse({ + object: 'a', + sort: [{ field: 'created_at', order: 'descending' }], + })).toThrow(/sort/); + // Right container, missing the required half of the pair. + expect(() => ElementRecordPickerPropsSchema.parse({ object: 'a', sort: [{ field: 'created_at' }] })) + .toThrow(/sort/); + }); + + // The shorthand IS the `dataSource` key, so one value must parse identically + // through both doors. This is what stops the flat spelling drifting into a + // third sort dialect (the ledger's `report.zod.ts` row records three already). + it('parses `sort` / `limit` identically to `dataSource` (one shape, two spellings) (#6276)', () => { + const sort = [{ field: 'name', order: 'asc' as const }]; + const viaProps = ElementRecordPickerPropsSchema.parse({ object: 'a', sort, limit: 25 }); + const viaDataSource = ElementDataSourceSchema.parse({ object: 'a', sort, limit: 25 }); + expect(viaProps.sort).toEqual(viaDataSource.sort); + expect(viaProps.limit).toEqual(viaDataSource.limit); + // …and the same rejections on the same values. + expect(ElementRecordPickerPropsSchema.safeParse({ object: 'a', limit: 0 }).success) + .toBe(ElementDataSourceSchema.safeParse({ object: 'a', limit: 0 }).success); + expect(ElementRecordPickerPropsSchema.safeParse({ object: 'a', sort: 'name' }).success) + .toBe(ElementDataSourceSchema.safeParse({ object: 'a', sort: 'name' }).success); + }); + + // The renderer's `?? 50` is a RENDERER fallback, deliberately not a schema + // default: `.default(50)` would materialize a limit on every parsed picker + // and turn an unset key into an authored one (and would then have to be kept + // in sync with objectui by hand). + it('does not default `limit` — the 50 is the renderer fallback (#6276)', () => { + const props = ElementRecordPickerPropsSchema.parse({ object: 'a' }); + expect(props.limit).toBeUndefined(); + expect(props.sort).toBeUndefined(); + }); }); // --------------------------------------------------------------------------- diff --git a/packages/spec/src/ui/component.zod.ts b/packages/spec/src/ui/component.zod.ts index e387580d54..9a07dfb385 100644 --- a/packages/spec/src/ui/component.zod.ts +++ b/packages/spec/src/ui/component.zod.ts @@ -110,6 +110,17 @@ import { FeedItemType, FeedFilterMode } from '../data/feed.zod'; // component-level `visibleWhen` (ADR-0089) — that one is a page to rewrite, not // a key to declare. // +// "The rest of that inventory" was one pair short, and how the shortfall +// happened is the reusable part: #5775's ruling named its keys individually, so +// the two `element:record_picker` shorthands the renderer reads through the +// SAME `ds.x ?? props.x` line as the keys that were named — `sort` and `limit` +// — fell outside it and stayed undeclared. #6276 declared them on the same +// #5611 rule (maintainer ruling 2026-08-08, direction A). The lesson for the +// next divergence sweep: enumerate by the RENDERER'S read pattern, not by the +// key list a previous ruling happened to quote. Retiring the flat family +// wholesale in favour of `dataSource` is the standing alternative, deferred to +// v18 as #6590 — not rejected. +// // ── #5068: THE GATE IS WIRED — read the flip precisely ───────────────────── // // `packages/lint/src/validate-component-props.ts` dispatches on the component's @@ -170,6 +181,10 @@ import { FeedItemType, FeedFilterMode } from '../data/feed.zod'; import { lazySchema } from '../shared/lazy-schema'; import { ExpressionInputSchema } from '../shared/expression.zod'; import { retiredKey } from '../shared/retired-key'; +// `element:record_picker`'s flat `sort` shorthand is the SAME contract as +// `ElementDataSourceSchema.sort` (page.zod.ts) — one shape, imported from the +// shared source rather than re-spelled here (#6276). +import { SortItemSchema } from '../shared/enums.zod'; const EmptyProps = z.object({}); /** @@ -664,6 +679,35 @@ export const ElementFormPropsSchema = lazySchema(() => z.object({ * enforce-or-remove: the control is a single-select `Select` with no search * box, so both were capability claims nothing kept (#5021 / #4988 precedent). * Either may return the day it is implemented; a declaration is not a roadmap. + * + * ⚠️ #6276 finished the same inventory one key-pair later, and the finding is + * worth stating as a rule rather than as two more keys. The renderer resolves + * its query from FOUR keys through one identical pattern — `dataSource` first, + * the flat `properties` shorthand second: + * + * ```ts + * const object = ds.object ?? props.object; + * const filter = ds.filter ?? props.filter; + * const sort = ds.sort ?? props.sort; + * const limit = ds.limit ?? props.limit ?? 50; + * ``` + * + * After #5775 two of those four shorthands were declared (`object`, `filter`) + * and two were not, so one renderer read half a contract and half a trapdoor: + * an author who inferred `properties.limit: 20` from the `object`/`filter` + * spelling got the renderer's default 50 with zero diagnostics — ADR-0078, on + * the same element that had just been rewritten to remove it. The maintainer's + * ruling (2026-08-08, direction A) declares the other two, so all four flat + * shorthands are contract. Direction B — retiring the whole flat family and + * making `dataSource` the single data-binding door — was NOT dropped: it is a + * cross-element decision (`element:form` / `element:filter` carry the same flat + * `object`), tracked as #6590 for v18, and A does not block it. When B lands + * these two retire alongside `object` / `filter` under ADR-0087, together. + * + * Both keys are declared in the shape `ElementDataSourceSchema` already uses + * for its own `sort` / `limit`, deliberately: they are the SAME contract read + * through a second spelling, so a divergent shape here would be a third + * dialect rather than a shorthand. */ export const ElementRecordPickerPropsSchema = lazySchema(() => z.object({ object: z.string().describe('Object to pick records from'), @@ -678,6 +722,24 @@ export const ElementRecordPickerPropsSchema = lazySchema(() => z.object({ /** Control label rendered above the select. */ label: I18nLabelSchema.optional().describe('Control label rendered above the select'), filter: FilterConditionSchema.optional().describe('Filter criteria for available records'), + /** + * Row order (#6276). The flat shorthand for `dataSource.sort`, and the same + * shape — `SortItemSchema[]`, the pairs the renderer forwards to the query as + * `$orderby`. `dataSource.sort` wins when both are written + * (`ds.sort ?? props.sort`). + */ + sort: z.array(SortItemSchema).optional() + .describe('Row order — synonym of the component-level `dataSource.sort`, which takes precedence when both are set'), + /** + * Row cap (#6276). The flat shorthand for `dataSource.limit`, same shape. + * `dataSource.limit` wins when both are written, and with neither the + * renderer queries `$top: 50` (`ds.limit ?? props.limit ?? 50`) — that 50 is + * the renderer's fallback, not a schema default, so it is documented here + * rather than declared: declaring it would materialize a `limit: 50` on every + * parsed picker and turn an unset key into an authored one. + */ + limit: z.number().int().positive().optional() + .describe('Max records offered — synonym of the component-level `dataSource.limit`, which takes precedence when both are set (renderer default 50)'), targetVariable: z.string().optional().describe('Page variable to bind selected record ID(s)'), placeholder: I18nLabelSchema.optional().describe('Placeholder text'), /** Shown in place of the row list when the query returns nothing. */