diff --git a/.changeset/5234-onnavigate-documented-exception.md b/.changeset/5234-onnavigate-documented-exception.md new file mode 100644 index 0000000000..a0aabb6e02 --- /dev/null +++ b/.changeset/5234-onnavigate-documented-exception.md @@ -0,0 +1,34 @@ +--- +--- + +Comment- and docs-only: `ObjectGridSchema.onNavigate` is now stated as the explicit, +documented exception the maintainer ruled it on 2026-08-19 (objectui#5234, option C). +Nothing is added to or removed from any accept set, and no published behaviour changes. + +`@object-ui/plugin-grid`'s `ObjectGrid` reads `schema.onNavigate` at the +`useNavigationOverlay` call, while `GRID_QUERY_INPUTS` does not publish the key — so the +manifest, the designer panel and the generated `sdui-intrinsics.d.ts` deny a key the +renderer honours. That gap is now deliberate and said out loud rather than left to be +re-discovered by the next census: + +- An exemption comment at the read site, in the shape objectui#5091 / PR #5241 + established for `columnState`, `hideRowHeightToggle`, `maxInlineRowActions` and + `rowActionDefs` — with the one difference those four do not share: this key IS + declared in `@object-ui/types`, so the read is plain rather than a cast. +- A programmatic-only note on the declaration in `@object-ui/types` + (`ObjectGridSchema.onNavigate`): it is a function value and a schema is a serialisable + document, so `(recordId, action) => void` cannot survive a metadata round-trip whatever + declares it, and programmatic callers should prefer `ObjectGridComponentProps` where its + nine sibling callbacks live. +- `packages/plugin-grid/README.md` carried the un-narrowed universal claim that the grid + "never reads a callback off the schema", which `onNavigate` falsifies. Narrowed to match + `content/docs/plugins/plugin-grid.mdx`, which had already been narrowed to "any of these + nine"; both pages now also name the one callback that is read. +- `gridNonAuthorKeys.test.tsx` extended, not rewritten: the four objectui#5091 keys keep + every assertion they had, and ten cases are added for this key — the ledger premises + plus two source-reading cases that pin the exemption prose itself, which is the only + part of a zero-behaviour-change ruling an ordinary assertion cannot see. + +The key is deliberately NOT removed (option A: a breaking public type change plus a +deprecation cycle for zero measured harm) and deliberately NOT added to +`GRID_QUERY_INPUTS` (option B: publishing to the designer a key no author can express). diff --git a/content/docs/plugins/plugin-grid.mdx b/content/docs/plugins/plugin-grid.mdx index 72fa9d6402..6f126f566e 100644 --- a/content/docs/plugins/plugin-grid.mdx +++ b/content/docs/plugins/plugin-grid.mdx @@ -416,6 +416,12 @@ them, and writing one of them into a schema does nothing at all: the grid builds the inner table's handlers itself and never reads any of these nine off the schema. +The one callback the grid does read off the schema is `onNavigate`, declared on +`ObjectGridSchema` for programmatic callers only. It is a function value too, so +it is no more authorable than the nine — it is deliberately absent from the +manifest and the designer panel, and prefer passing it as a prop +(objectui#5234, maintainer ruling of 2026-08-19). + ```tsx import { ObjectGrid } from '@object-ui/plugin-grid'; import type { ObjectGridComponentProps } from '@object-ui/plugin-grid'; diff --git a/packages/plugin-grid/README.md b/packages/plugin-grid/README.md index 0f2527edc2..3498960275 100644 --- a/packages/plugin-grid/README.md +++ b/packages/plugin-grid/README.md @@ -513,7 +513,13 @@ const schema: ObjectGridSchema = { `onDelete`, `onBulkDelete` and `onAddRecord` are React props on `ObjectGridComponentProps` — they are functions, so no metadata document can hold them, and writing one into a schema does nothing at all: the grid builds the inner -table's handlers itself and never reads a callback off the schema. +table's handlers itself and never reads any of these nine off the schema. + +The one callback the grid does read off the schema is `onNavigate`, declared on +`ObjectGridSchema` for programmatic callers only. It is a function value too, so +it is no more authorable than the nine — it is deliberately absent from the +manifest and the designer panel, and prefer passing it as a prop +(objectui#5234, maintainer ruling of 2026-08-19). ```tsx import { ObjectGrid } from '@object-ui/plugin-grid'; diff --git a/packages/plugin-grid/src/ObjectGrid.tsx b/packages/plugin-grid/src/ObjectGrid.tsx index e48c003c0e..76e04e1742 100644 --- a/packages/plugin-grid/src/ObjectGrid.tsx +++ b/packages/plugin-grid/src/ObjectGrid.tsx @@ -1331,6 +1331,36 @@ export const ObjectGrid: React.FC = ({ const navigation = useNavigationOverlay({ navigation: schema.navigation, objectName: schema.objectName, + // NON-AUTHOR SURFACE — `onNavigate` is deliberately absent from + // `GRID_QUERY_INPUTS` (`index.tsx`), by the maintainer ruling of + // 2026-08-19 on objectui#5234, so this read is deliberate, not missed. + // + // Unlike the four keys objectui#5091 ruled, this one IS declared — in + // `@object-ui/types`' `ObjectGridSchema` (`objectql.ts`), with its own doc + // comment — which is why the read below is plain and not a cast. It is NOT + // drift absorbed by `BaseSchema`'s index signature: the reading that said + // so was measured wrong and withdrawn on the card before it was ruled. + // + // It stays unpublished because it is a FUNCTION VALUE and a schema is a + // SERIALISABLE DOCUMENT. `(recordId, action) => void` cannot survive a + // metadata round-trip whatever declares it, so no author writing JSON or + // YAML — and no AI emitting a schema document — can ever express this key. + // Publishing it into the designer panel and the generated + // `sdui-intrinsics.d.ts` would advertise an offer nobody can take, which + // is the option the ruling rejected as the most AI-error-prone of the + // three. Programmatic callers already have the channel its nine siblings + // use: `onRowClick`, `onRowSelect`, `onCellChange`, `onRowSave`, + // `onBatchSave`, `onEdit`, `onDelete`, `onBulkDelete` and `onAddRecord` + // are props on `ObjectGridComponentProps`, and that is where a caller + // should prefer to pass this one too. + // + // The contract says the same thing independently: `ComponentPropsMap + // ['object-grid']` (`@objectstack/spec@17`) is a `strictObject` and + // rejects `onNavigate` BY NAME (`unrecognized_keys`), so an author who + // took the offer could not save the document. Both halves — unlisted here, + // rejected there — are pinned by `__tests__/gridNonAuthorKeys.test.tsx`, + // together with the read itself, so this exemption cannot decay into a + // silent drop. onNavigate: schema.onNavigate, onRowClick, }); diff --git a/packages/plugin-grid/src/__tests__/gridNonAuthorKeys.test.tsx b/packages/plugin-grid/src/__tests__/gridNonAuthorKeys.test.tsx index 5c13ba4b11..252d7d5811 100644 --- a/packages/plugin-grid/src/__tests__/gridNonAuthorKeys.test.tsx +++ b/packages/plugin-grid/src/__tests__/gridNonAuthorKeys.test.tsx @@ -117,6 +117,9 @@ */ import { describe, it, expect, vi, beforeEach } from 'vitest'; +import { readFileSync } from 'node:fs'; +import path from 'node:path'; +import { fileURLToPath } from 'node:url'; import { render, screen, waitFor } from '@testing-library/react'; import '@testing-library/jest-dom'; import React from 'react'; @@ -593,3 +596,264 @@ describe('a view-level toolbar block must not shadow the object CRUD predicates ).toContain('status'); }); }); + +// --------------------------------------------------------------------------- +// The SIXTH key the census reached — `onNavigate` — a NON-AUTHOR key that is +// nevertheless DECLARED, and the one callback the grid reads off the schema +// (objectui#5234, maintainer ruling 2026-08-19, Option C). +// +// ## Why it is not a sixth entry in NON_AUTHOR_KEYS above +// +// Those four are CAST reads: `@object-ui/types` does not declare them, which is +// what makes `(schema as any).key` the only way to read them. `onNavigate` is +// the opposite — it is an explicitly declared member of `ObjectGridSchema` with +// its own doc comment, so the read at the `useNavigationOverlay` call is plain. +// +// The card originally argued the reverse: that the key was ABSENT from the +// interface and type-checked only because `BaseSchema`'s `[key: string]: any` +// absorbed it. Its own author withdrew that on 2026-08-18 after re-measuring — +// the evidence had been a grep bounded to lines 532-760 of an interface that +// runs past 760. So `declaresOnNavigate` below is not decoration: it pins the +// corrected fact, and it is the assertion that would have stopped the original +// error. Do not re-describe this key as drift or as an oversight. +// +// ## What the ruling actually decided, and what pins it +// +// Option C: keep the declaration, keep the read, and SAY that it is an +// exception — an exemption comment at the read site in the shape PR #5241 +// established for the four above, and a programmatic-only note on the type. +// Option A (remove the key) was rejected as a breaking public type change for +// zero measured harm; option B (declare it in `GRID_QUERY_INPUTS`) as +// publishing to the designer a key no author can ever express. +// +// That makes this card's deliverable PROSE, and prose is exactly what a pin +// normally cannot see. Every assertion about the world — not published, spec +// rejects it, parser says unknown-prop, the renderer still reads it, the type +// still declares it — was already TRUE before this card and stays true if its +// change is reverted. They are the ledger's premises and are pinned so the +// exemption cannot decay, but on their own they would pin nothing. +// +// So the two `documents its exemption` cases below read the SOURCE, the way +// `ObjectGrid.exportOptionsKeys.test.ts` in this directory does. They are the +// only assertions here that can tell the two states of the world apart, and +// they are anchored on text that exists in BOTH — the read line itself and the +// declaration line itself — so a failure means the comment went missing, never +// that the anchor moved. +// --------------------------------------------------------------------------- + +const here = path.dirname(fileURLToPath(import.meta.url)); +// packages/plugin-grid/src/__tests__ -> repo root +const repoRoot = path.resolve(here, '../../../..'); +const GRID_SOURCE = path.join(repoRoot, 'packages/plugin-grid/src/ObjectGrid.tsx'); +const TYPES_SOURCE = path.join(repoRoot, 'packages/types/src/objectql.ts'); + +const gridLines = readFileSync(GRID_SOURCE, 'utf8').split('\n'); +const typesLines = readFileSync(TYPES_SOURCE, 'utf8').split('\n'); + +/** + * The contiguous comment block immediately above `index` — `//` lines or a + * `/** ... *\/` block, stopping at the first line that is neither. Adjacency is + * the point: a matching phrase anywhere else in a 3500-line file would satisfy + * a whole-file `toContain` while the read site itself carried nothing. + */ +const commentBlockAbove = (lines: string[], index: number): string => { + const out: string[] = []; + for (let i = index - 1; i >= 0; i -= 1) { + const t = lines[i].trim(); + if (t === '' && out.length === 0) continue; + if (t.startsWith('//') || t.startsWith('*') || t.startsWith('/*')) out.unshift(t); + else break; + } + return out.join('\n'); +}; + +/** Indices of every line whose trimmed form satisfies `match`. */ +const indicesWhere = (lines: string[], match: (t: string) => boolean): number[] => + lines.map((l, i) => (match(l.trim()) ? i : -1)).filter((i) => i >= 0); + +/** `export interface ObjectGridSchema` .. its closing brace, as line indices. */ +const objectGridSchemaSpan = (): [number, number] => { + const start = typesLines.findIndex((l) => l.startsWith('export interface ObjectGridSchema')); + const end = typesLines.findIndex((l, i) => i > start && l === '}'); + return [start, end]; +}; + +/** The nine sibling callbacks the type note and the README claim are props-only. */ +const NINE_SIBLINGS = [ + 'onRowClick', 'onRowSelect', 'onCellChange', 'onRowSave', 'onBatchSave', + 'onEdit', 'onDelete', 'onBulkDelete', 'onAddRecord', +] as const; + +describe('`onNavigate` — the ledger premises (objectui#5234)', () => { + it.each(GRID_TAGS)('$label does not publish it as authoring surface', ({ type, namespace }) => { + expect( + declaredInputNames(type, namespace), + '`onNavigate` is now published to the manifest, the designer panel and the generated' + + ' `sdui-intrinsics.d.ts`. That is option B, which the 2026-08-19 ruling on objectui#5234' + + ' rejected as the most AI-error-prone of the three: it advertises to an author a key that' + + ' is a FUNCTION VALUE, and therefore one no JSON or YAML document can ever carry.', + ).not.toContain('onNavigate'); + // The control from the four above, restated here so "not published" cannot + // pass against a registration that published nothing at all. + expect(declaredInputNames(type, namespace)).toContain(DECLARED_CONTROL); + }); + + it('the spec refuses it as an unrecognized key', () => { + const result = specVerdict('onNavigate', () => {}); + expect( + result.success, + '`@objectstack/spec` now ACCEPTS object-grid.onNavigate — re-open the 2026-08-19 ruling.', + ).toBe(false); + // A KEY verdict, not a document one: the rest of the fixture is legal. + const unrecognized = (result as { error: { issues: Array<{ code: string; keys?: string[] }> } }).error.issues + .filter((issue) => issue.code === 'unrecognized_keys') + .flatMap((issue) => issue.keys ?? []); + expect(unrecognized).toContain('onNavigate'); + }); + + it('the parser reports it as unknown-prop — the ruled outcome, not a defect', () => { + const codes = diagnose({ onNavigate: () => {} }).filter((d) => d.code === 'unknown-prop'); + expect( + codes.map((d) => d.message).join('\n'), + '`onNavigate` no longer draws `unknown-prop`. If that is deliberate it means the key was' + + ' declared to the manifest — which the 2026-08-19 ruling forbids for this key.', + ).toContain('onNavigate'); + }); + + it('`@object-ui/types` DOES declare it — the corrected premise, pinned', () => { + // The fact the card got wrong. `onNavigate` appears three times in + // `objectql.ts` (also on `ObjectViewSchema` and `ListViewRuntimeProps`), so + // the search is bounded to the interface rather than run over the file — + // the unbounded-versus-mis-bounded read is what produced the original error. + const [start, end] = objectGridSchemaSpan(); + expect(start, 'ObjectGridSchema is gone from objectql.ts').toBeGreaterThanOrEqual(0); + expect(end).toBeGreaterThan(start); + const declarations = indicesWhere(typesLines, (t) => t.startsWith('onNavigate?:')) + .filter((i) => i > start && i < end); + expect( + declarations.length, + 'ObjectGridSchema no longer declares `onNavigate` exactly once. If it was REMOVED, that is' + + ' option A — a breaking public type change the 2026-08-19 ruling declined to take in the' + + ' launch window, so it needs its own ruling, not a tidy-up.', + ).toBe(1); + }); + + it('…and does NOT declare the nine siblings, which is the contrast the note draws', () => { + // The type note and both docs pages say the nine live only on + // `ObjectGridComponentProps`. Measured here so the claim cannot go stale + // under them: a tenth callback arriving on the schema would make the note + // false while every other assertion in this file stayed green. + const [start, end] = objectGridSchemaSpan(); + const declaredInSpan = new Set( + typesLines + .slice(start, end) + .map((l) => l.trim().match(/^([A-Za-z_$][\w$]*)\??\s*:/)?.[1]) + .filter(Boolean) as string[], + ); + // Non-vacuity: the scan found the interface's members at all. + expect(declaredInSpan.size).toBeGreaterThan(20); + expect(declaredInSpan.has('onNavigate')).toBe(true); + for (const sibling of NINE_SIBLINGS) { + expect( + declaredInSpan.has(sibling), + `\`${sibling}\` is now declared on ObjectGridSchema. The programmatic-only note on` + + ' `onNavigate` and the README both say the nine live ONLY on' + + ' `ObjectGridComponentProps` — one of them is now wrong.', + ).toBe(false); + } + }); + + it('the renderer still reads it — the half a reader of "non-author surface" would delete', async () => { + // Behavioural, not textual: the schema-supplied callback must still reach + // `useNavigationOverlay` and fire on a row click in the default `page` + // mode. Deleting the read is the one move that makes a programmatic + // caller's grid quietly stop navigating. + const onNavigate = vi.fn(); + const { container } = renderGrid({ id: 'nav-read', navigation: { mode: 'page' }, onNavigate }); + await settled(); + const cell = Array.from(container.querySelectorAll('tbody td')).find( + (td) => td.textContent?.trim() === 'Alice', + ); + expect(cell, 'the fixture row never rendered, so a silent no-call would look like a pass').toBeTruthy(); + (cell as HTMLElement).click(); + await waitFor(() => + expect( + onNavigate, + 'the schema-supplied `onNavigate` no longer fires on a row click — the read at the' + + ' `useNavigationOverlay` call was dropped.', + ).toHaveBeenCalled(), + ); + expect(onNavigate.mock.calls[0][0]).toBe('1'); + }); +}); + +describe('`onNavigate` — the exemption is DOCUMENTED, which is what this card delivered (objectui#5234)', () => { + it('the read site in ObjectGrid.tsx carries the exemption comment', () => { + // Anchored on the read line, which exists in both states of the world, so a + // red here means the COMMENT went missing — never that the anchor moved. + const reads = indicesWhere(gridLines, (t) => t === 'onNavigate: schema.onNavigate,'); + expect( + reads.length, + 'the `onNavigate: schema.onNavigate` read is gone or duplicated; re-aim this pin before' + + ' trusting anything else in this describe.', + ).toBe(1); + + const block = commentBlockAbove(gridLines, reads[0]); + expect(block.length, 'the read site carries no comment at all').toBeGreaterThan(0); + for (const phrase of [ + 'NON-AUTHOR SURFACE', + '`onNavigate`', + '`GRID_QUERY_INPUTS`', + 'FUNCTION VALUE', + 'SERIALISABLE DOCUMENT', + '`ObjectGridComponentProps`', + 'objectui#5234', + 'gridNonAuthorKeys.test.tsx', + ]) { + expect( + block, + `the exemption comment at the \`onNavigate\` read site no longer states ${phrase}.` + + ' The 2026-08-19 ruling on objectui#5234 chose option C precisely because the key stays' + + ' and the EXCEPTION is stated; without this comment the ruling left nothing behind.', + ).toContain(phrase); + } + }); + + it('the declaration in objectql.ts carries the programmatic-only note', () => { + const [start, end] = objectGridSchemaSpan(); + const declarations = indicesWhere(typesLines, (t) => t.startsWith('onNavigate?:')) + .filter((i) => i > start && i < end); + expect(declarations.length).toBe(1); + + const block = commentBlockAbove(typesLines, declarations[0]); + expect(block.length, 'the declaration carries no doc comment at all').toBeGreaterThan(0); + for (const phrase of [ + 'PROGRAMMATIC ONLY', + 'GRID_QUERY_INPUTS', + 'FUNCTION VALUE', + 'SERIALISABLE DOCUMENT', + 'ObjectGridComponentProps', + 'objectui#5234', + ]) { + expect( + block, + `the programmatic-only note on \`ObjectGridSchema.onNavigate\` no longer states ${phrase}.` + + ' The 2026-08-19 ruling required it on the declaration itself, because an agent that' + + ' reads the type and finds a documented callback concludes callbacks are authorable.', + ).toContain(phrase); + } + }); + + it('and the four keys ruled in objectui#5091 still carry theirs — this extension weakened nothing', () => { + // The control for the two cases above: they assert "a comment is present", + // which would also pass if this file had quietly stopped being able to see + // comments at all. The four older exemptions are the independent witness. + const gridSource = gridLines.join('\n'); + for (const { key } of NON_AUTHOR_KEYS) { + expect( + gridSource, + `the objectui#5091 exemption comment for \`${key}\` is gone from ObjectGrid.tsx.`, + ).toContain(`NON-AUTHOR SURFACE — \`${key}\``); + } + }); +}); diff --git a/packages/types/src/objectql.ts b/packages/types/src/objectql.ts index 39d95a26b4..ef49bd9a8c 100644 --- a/packages/types/src/objectql.ts +++ b/packages/types/src/objectql.ts @@ -843,6 +843,29 @@ export interface ObjectGridSchema extends BaseSchema { /** * Callback for page-level navigation (used by 'page' mode). * Called with recordId and action ('view' | 'edit'). + * + * PROGRAMMATIC ONLY — not authoring surface. Deliberately absent from + * `GRID_QUERY_INPUTS` (`@object-ui/plugin-grid`'s `index.tsx`), so the + * manifest, the designer panel and the generated `sdui-intrinsics.d.ts` do + * not offer it; maintainer ruling of 2026-08-19 on objectui#5234. + * + * The reason is the value, not the declaration: this is a FUNCTION VALUE and + * a schema is a SERIALISABLE DOCUMENT. `(recordId, action) => void` cannot + * survive a metadata round-trip whatever declares it, so no author writing + * JSON or YAML — and no AI emitting a schema document — can express this + * key. Writing it into a stored document does nothing at all. + * + * Programmatic callers should prefer `ObjectGridComponentProps` + * (`@object-ui/plugin-grid`), where the nine sibling callbacks — + * `onRowClick`, `onRowSelect`, `onCellChange`, `onRowSave`, `onBatchSave`, + * `onEdit`, `onDelete`, `onBulkDelete`, `onAddRecord` — live and only live, + * for exactly this reason. + * + * The declaration is kept rather than removed: with the key explicitly + * published, removing it is a breaking public type change plus a deprecation + * cycle, for zero measured harm. The exemption comment at the read site + * (`plugin-grid/src/ObjectGrid.tsx`) carries the same statement, and both are + * pinned by `plugin-grid/src/__tests__/gridNonAuthorKeys.test.tsx`. */ onNavigate?: (recordId: string | number, action?: string) => void;