From aac3e65bc320e5b0fe76fa980a918f0769649d40 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 29 Aug 2026 06:00:52 +0000 Subject: [PATCH] feat(react): name a `props` config bag that no `schema`-reading renderer sees MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `SchemaRenderer` hoists `properties.*` onto the node; `props` — the annotated legacy alias of the same bag — is evaluated and then spread as React props instead. A renderer declared as `({ schema })`, the normal component-renderer shape, therefore never sees it, and every gate accepts the spelling because `BaseSchema` is `.passthrough()`. The `element:*` family is the exception: its `readProps()` merges both bags. Emit a `console.warn` at the SchemaRenderer tier naming the node and the dropped keys and pointing at `properties`. Silent for the `element:*` family and for `view:simple`, the one non-element type measured to read the raw bag. Zero behaviour change, pinned against a reading captured on the tree before the diagnostic existed. Level and dedupe follow the ruling's census precondition. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01CRJge11jso9TpXRWFt1Z49 --- ...props-bag-component-renderer-diagnostic.md | 59 ++ packages/react/src/SchemaRenderer.tsx | 48 +- ...SchemaRenderer.propsBagDiagnostic.test.tsx | 550 ++++++++++++++++++ .../react/src/utils/propsBagDiagnostic.ts | 260 +++++++++ 4 files changed, 912 insertions(+), 5 deletions(-) create mode 100644 .changeset/6708-props-bag-component-renderer-diagnostic.md create mode 100644 packages/react/src/__tests__/SchemaRenderer.propsBagDiagnostic.test.tsx create mode 100644 packages/react/src/utils/propsBagDiagnostic.ts diff --git a/.changeset/6708-props-bag-component-renderer-diagnostic.md b/.changeset/6708-props-bag-component-renderer-diagnostic.md new file mode 100644 index 000000000..e8b332c35 --- /dev/null +++ b/.changeset/6708-props-bag-component-renderer-diagnostic.md @@ -0,0 +1,59 @@ +--- +'@object-ui/react': patch +--- + +A `props` config bag on a component-renderer node is now named at render +instead of dropped in silence (objectui#6708). + +`SchemaRenderer` HOISTS every `properties.*` value onto the node, so a key +written under `properties` is a real value on `schema.` by the time a +renderer destructures it. `props` — the annotated legacy alias of the same bag +— is NOT hoisted: it is evaluated and then spread as React props on the created +element. A renderer declared as `({ schema })`, which is the normal shape for +the component renderers, therefore never sees it. The `element:*` family is the +exception: its `readProps()` merges `{ ...schema.props, ...schema.properties }`, +so the same spelling is honoured there. + +Every gate accepts the `props` spelling — `BaseSchema` is `.passthrough()` with +`[key: string]: any` — and the docs call it a supported alias, so nothing +between the author and the screen said a word. Re-measured on `faac0d935` +through the real `SchemaRenderer` with a probe that records both channels: + +| node | React prop `data` | `schema.data` | +|---|---|---| +| `props: { data: "${data.customers}" }` | the evaluated array | absent | +| `properties: { data: "${data.customers}" }` | the evaluated array | the array | + +Same key, same value, one envelope apart. The expression is evaluated on both +legs, so this is a dropped value rather than an unevaluated one. Read through a +real `data-table` (objectui#6665's four-leg pin) the same pair renders +`No results found` against the two rows. + +The diagnostic's level and dedupe were chosen from a census, which the ruling +made a precondition. Every JSON document, every `json` fence in every +`.md`/`.mdx`, and every TypeScript object literal in the repo was walked for +nodes carrying both `type` and `props`: 39 such nodes, 22 of them on +component-renderer types, and 19 of those 22 are test fixtures exercising this +shape on purpose. The authored, non-test corpus holds 5 — three of which are +deliberate counter-examples in the skills guides. Nothing floods, so the level +is not softened for volume; the dedupe is keyed on the MESSAGE rather than on +the schema object, so a metadata generator emitting one wrong envelope across +many nodes still gets one line while two genuinely different nodes get two. + +`console.warn`, matching objectui#6575 and objectui#6665 — the two prior +instances of this exact "you declared something and the renderer dropped it" +shape — rather than the `console.error` its neighbour at this tier uses for a +raw `${...}` placed verbatim in front of a user. Nothing is placed here; a +value is dropped. + +No behaviour change, which is the entire reason this arm was chosen. Hoisting +`props` to parity with `properties` was refused at ruling: it would weld the +legacy alias in as a permanent second spelling, against this repo's +alias-retirement direction. Refusing the key at parse stays blocked on the +`.passthrough()` ceiling (objectui#5155 / objectui#6269). What every renderer +receives is pinned byte-for-byte against a reading captured on the tree before +the diagnostic existed. Nothing is added to the published surface either — the +predicate, message builder, prefix constant and test-only reset are +module-internal and are not re-exported from the package entry, matching +objectui#6575's own symbols. The trap stops being silent; it does not stop +being a trap. diff --git a/packages/react/src/SchemaRenderer.tsx b/packages/react/src/SchemaRenderer.tsx index 57c0beb3f..1e48de087 100644 --- a/packages/react/src/SchemaRenderer.tsx +++ b/packages/react/src/SchemaRenderer.tsx @@ -30,6 +30,7 @@ import { usePredicateScope } from './hooks/useExpression.js'; import { usePageVariables } from './hooks/usePageVariables.js'; import { resolveKeyedI18nLabel } from './utils/i18n.js'; import { reportUnevaluatedExpressions } from './utils/unevaluatedExpression.js'; +import { reportDroppedPropsBag } from './utils/propsBagDiagnostic.js'; import { reportUnresolvableVisibilityPredicate, reportAdapterOnlyDataPredicate, @@ -1330,6 +1331,44 @@ export const SchemaRenderer: ForwardRefExoticComponent< ); } + // The legacy `props` alias, narrowed exactly as objectui#5123 ruled: for a + // key BOTH bags declare, `properties` wins here as it already wins in + // `readProps()`, so one key has one answer on both channels. + // + // HOISTED out of the `createElement` call below (objectui#6708) so the + // diagnostic and the spread read the SAME bag. It is the same pure call with + // the same arguments producing the same object in the same spread position — + // nothing about what any renderer receives moves — but it removes the one way + // this diagnostic could go wrong: reporting a set of keys that is not the set + // actually handed to the component. + const outgoingPropsBag = propsWithoutCanonicalKeys( + evaluatedSchema.props, + evaluatedSchema.properties + ); + + // Dev-build diagnostic (objectui#6708, maintainer ruling 2026-08-29, option + // 2): those keys are spread as React props and never hoisted onto the node, + // so a renderer that reads its config from `schema` — every family except + // `element:*`'s `readProps()` — drops them without a word. + // + // Sited HERE, beside its objectui#4795 neighbour and after the metadata + // destructure, for the same reason: this is the point where what leaves this + // component for the renderer is finally known. Read-only — it reports what + // the line above already computed and changes nothing that is rendered. + // + // The AUTHORED bag comes from `schema`, not `evaluatedSchema`: the evaluation + // memo rebuilds `props` with an object spread, which turns a degenerate + // `props: 'text'` into `{ '0': 't', … }` long before this line. See + // `collectDroppedPropsKeys`. + if (__DEV__) { + reportDroppedPropsBag( + evaluatedSchema.type, + evaluatedSchema.id, + (schema as { props?: unknown } | null | undefined)?.props, + outgoingPropsBag + ); + } + // SDUI scoped styling (ADR-0065) — computed in the memo hoisted above the // early returns; see the doc comment there for why it cannot live here. const { scopeClass, scopedCss, mergedClassName, schemaForComponent } = scopedStyling; @@ -1361,11 +1400,10 @@ export const SchemaRenderer: ForwardRefExoticComponent< schema: schemaForComponent, ...componentProps, // Spread non-metadata schema properties as props // The legacy `props` alias still overrides plain top-level keys, but no - // longer overrides the canonical `properties` bag: for a key BOTH bags - // declare, `properties` wins here exactly as it already wins in - // `readProps()`, so one key has one answer on both channels - // (objectui#5123, maintainer ruling 2026-08-18). - ...propsWithoutCanonicalKeys(evaluatedSchema.props, evaluatedSchema.properties), + // longer overrides the canonical `properties` bag (objectui#5123, + // maintainer ruling 2026-08-18). Computed above rather than inline, so + // the objectui#6708 diagnostic names this exact bag — see there. + ...outgoingPropsBag, ...ariaProps, // Inject ARIA attributes from AriaPropsSchema ...debugAttrs, // Debug-mode data attributes disabled: __disabled || undefined, diff --git a/packages/react/src/__tests__/SchemaRenderer.propsBagDiagnostic.test.tsx b/packages/react/src/__tests__/SchemaRenderer.propsBagDiagnostic.test.tsx new file mode 100644 index 000000000..13b042893 --- /dev/null +++ b/packages/react/src/__tests__/SchemaRenderer.propsBagDiagnostic.test.tsx @@ -0,0 +1,550 @@ +/** + * ObjectUI + * Copyright (c) 2024-present ObjectStack Inc. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +/** + * objectui#6708 — a key authored under the `props` envelope never reaches a + * renderer that reads `schema`, and until now nothing said so. + * + * The maintainer ruling (2026-08-29, verbatim 「同意」) took option 2: diagnose + * it at the `SchemaRenderer` tier — one seam, every renderer family covered, + * ZERO behaviour change. Option 1 (hoisting `props` to parity with + * `properties`) was refused; option 3 (refusing the key at parse) stays blocked + * on the `.passthrough()` ceiling (objectui#5155 / objectui#6269). + * + * ## The three describes, and why the first two exist at all + * + * 1. **The asymmetry, re-measured on this base.** The card was filed off a + * reading taken on `5967be095`. A defect that is only quoted ages silently, + * so legs 2 and 3 — the same key, the same value, one envelope apart — are + * re-run here through the real `SchemaRenderer` before anything is claimed + * about them. + * 2. **What renderers receive, pinned against the PRE-diagnostic tree.** The + * ruling's whole reason for choosing this arm is that nothing renderers see + * moves. `BASE_READING` below is not a snapshot this file wrote for itself: + * it was captured by rendering these seven nodes on `faac0d935` with + * `SchemaRenderer.tsx` reverted to its committed state — the tree with no + * diagnostic in it — and pasted here verbatim. So this is a before/after + * comparison, not a self-fulfilling snapshot, and it fails the moment the + * diagnostic starts changing what it reports on. + * 3. **The diagnostic itself**, in both directions the ruling pins. + * + * ## Why probes rather than the real `data-table` + * + * `@object-ui/components` depends on THIS package, so importing a real renderer + * here would be a dependency cycle. The probes stand in for the two families + * exactly where they differ: `SchemaReadingProbe` reads `schema.` (what + * `statistic`, `card`, `data-table` and every other component renderer do), + * `BothBagsProbe` merges both bags the way the `element:*` family's + * `readProps()` does. The end-to-end four-leg reading through the real + * `data-table` lives in + * `packages/components/src/__tests__/data-table-node-data-diagnostic.test.tsx` + * (objectui#6665) and stays green unchanged. + */ + +import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; +import { render } from '@testing-library/react'; +import React from 'react'; +import { ComponentRegistry } from '@object-ui/core'; +import { SchemaRenderer } from '../SchemaRenderer'; +import { SchemaRendererContext } from '../context/SchemaRendererContext'; +import { + DROPPED_PROPS_BAG_PREFIX, + collectDroppedPropsKeys, + formatDroppedPropsBagMessage, + readsPropsBag, + __resetDroppedPropsBagWarnings, +} from '../utils/propsBagDiagnostic'; + +/** The provider really does hold the path every expression below spells. */ +const DATA = { customers: ['ada', 'grace'] }; + +const renderWithData = (schema: unknown) => + render( + + + , + ); + +/** + * Only this diagnostic's own lines. Filtered by its prefix rather than by call + * count: these renders go through the REAL `SchemaRenderer`, and an unrelated + * warning must not be able to satisfy — or break — an assertion about this one. + */ +const warnings = (): string[] => + ((console.warn as unknown as { mock?: { calls: unknown[][] } }).mock?.calls ?? []) + .map(c => String(c[0])) + .filter(m => m.startsWith(DROPPED_PROPS_BAG_PREFIX)); + +/* -------------------------------------------------------------------------- * + * 1 + 2 — the asymmetry, and what renderers receive + * -------------------------------------------------------------------------- */ + +/** + * Captured on `faac0d935` with `packages/react/src/SchemaRenderer.tsx` reverted + * to its committed blob (`57c0beb3f`), i.e. with no diagnostic in the tree, by + * running exactly the cases below. Pasted verbatim — see this file's header for + * why that provenance is the point. + * + * Read it as the mechanism written out: on `componentProps` the React prop + * `data` holds the evaluated array while `schemaDotData` is `null`, and on + * `propertiesOnly` the SAME key is on the node. That pair is the card. + */ +const BASE_READING: Record = JSON.parse(`{ + "componentProps": { + "propKeys": ["className","data","data-obj-id","data-obj-type","disabled","id","props","schema","title"], + "restSnapshot": { + "id": "n1", + "props": {"data":["ada","grace"],"title":"T"}, + "data": ["ada","grace"], + "title": "T", + "data-obj-id": "n1", + "data-obj-type": "test:cap" + }, + "schemaSnapshot": {"type":"test:cap","id":"n1","props":{"data":["ada","grace"],"title":"T"}}, + "schemaDotData": null, + "schemaDotTitle": null + }, + "elementProps": { + "propKeys": ["className","content","data-obj-id","data-obj-type","disabled","id","props","schema"], + "restSnapshot": { + "id": "n2", + "props": {"content":"X"}, + "content": "X", + "data-obj-id": "n2", + "data-obj-type": "element:cap" + }, + "schemaSnapshot": {"type":"element:cap","id":"n2","props":{"content":"X"}}, + "schemaDotData": null, + "schemaDotTitle": null + }, + "propertiesOnly": { + "propKeys": ["className","data","data-obj-id","data-obj-type","disabled","id","properties","schema"], + "restSnapshot": { + "id": "n3", + "properties": {"data":["ada","grace"]}, + "data": ["ada","grace"], + "data-obj-id": "n3", + "data-obj-type": "test:cap" + }, + "schemaSnapshot": {"type":"test:cap","id":"n3","properties":{"data":["ada","grace"]},"data":["ada","grace"]}, + "schemaDotData": ["ada","grace"], + "schemaDotTitle": null + }, + "bothBags": { + "propKeys": [ + "a", + "b", + "className", + "data-obj-id", + "data-obj-type", + "disabled", + "id", + "properties", + "props", + "schema" + ], + "restSnapshot": { + "id": "n4", + "props": {"a":"fromProps","b":"onlyProps"}, + "properties": {"a":"fromProperties"}, + "a": "fromProperties", + "b": "onlyProps", + "data-obj-id": "n4", + "data-obj-type": "test:cap" + }, + "schemaSnapshot": { + "type": "test:cap", + "id": "n4", + "props": {"a":"fromProps","b":"onlyProps"}, + "properties": {"a":"fromProperties"}, + "a": "fromProperties" + }, + "schemaDotData": null, + "schemaDotTitle": null + }, + "emptyProps": { + "propKeys": ["className","data-obj-id","data-obj-type","disabled","id","props","schema"], + "restSnapshot": {"id":"n5","props":{},"data-obj-id":"n5","data-obj-type":"test:cap"}, + "schemaSnapshot": {"type":"test:cap","id":"n5","props":{}}, + "schemaDotData": null, + "schemaDotTitle": null + }, + "noBags": { + "propKeys": ["className","data-obj-id","data-obj-type","disabled","id","schema","title"], + "restSnapshot": {"id":"n6","title":"plain","data-obj-id":"n6","data-obj-type":"test:cap"}, + "schemaSnapshot": {"type":"test:cap","id":"n6","title":"plain"}, + "schemaDotData": null, + "schemaDotTitle": "plain" + }, + "viewSimple": { + "propKeys": ["className","columns","data-obj-id","data-obj-type","disabled","id","props","schema"], + "restSnapshot": {"id":"n7","props":{"columns":3},"columns":3,"data-obj-id":"n7","data-obj-type":"view:simple"}, + "schemaSnapshot": {"type":"view:simple","id":"n7","props":{"columns":3}}, + "schemaDotData": null, + "schemaDotTitle": null + } +}`); + +/** Everything the component was handed, minus what cannot be compared. */ +function snap(value: unknown): unknown { + return JSON.parse( + JSON.stringify(value, (_k, v) => { + if (typeof v === 'function') return '[fn]'; + if (v && typeof v === 'object' && (v as { $$typeof?: unknown }).$$typeof) return '[react]'; + return v; + }) ?? 'null', + ); +} + +const captured: Record = {}; + +/** Reads its config from `schema` — the normal component-renderer shape. */ +const makeCapturingProbe = (label: string) => (props: Record) => { + const { schema, children: _children, ...rest } = props as { + schema?: Record; + children?: unknown; + } & Record; + captured[label] = { + propKeys: Object.keys(props).sort(), + restSnapshot: snap(rest), + schemaSnapshot: snap(schema), + schemaDotData: snap(schema?.data ?? null), + schemaDotTitle: snap(schema?.title ?? null), + }; + return
; +}; + +/** + * The seven nodes, one per envelope shape. `test:cap` and `element:cap` are the + * two families; `view:simple` is the one non-`element:` type measured to read + * the raw bag (`plugin-view`'s `SimpleViewRenderer` reads `schema.props.columns`). + */ +const CAPTURE_CASES: ReadonlyArray = [ + ['componentProps', { type: 'test:cap', id: 'n1', props: { data: '${data.customers}', title: 'T' } }], + ['elementProps', { type: 'element:cap', id: 'n2', props: { content: 'X' } }], + ['propertiesOnly', { type: 'test:cap', id: 'n3', properties: { data: '${data.customers}' } }], + [ + 'bothBags', + { + type: 'test:cap', + id: 'n4', + props: { a: 'fromProps', b: 'onlyProps' }, + properties: { a: 'fromProperties' }, + }, + ], + ['emptyProps', { type: 'test:cap', id: 'n5', props: {} }], + ['noBags', { type: 'test:cap', id: 'n6', title: 'plain' }], + ['viewSimple', { type: 'view:simple', id: 'n7', props: { columns: 3 } }], +]; + +describe('objectui#6708 — the `props` / `properties` asymmetry, re-measured on this base', () => { + beforeEach(() => { + vi.spyOn(console, 'warn').mockImplementation(() => {}); + vi.spyOn(console, 'error').mockImplementation(() => {}); + __resetDroppedPropsBagWarnings(); + for (const key of Object.keys(captured)) delete captured[key]; + for (const [label] of CAPTURE_CASES) { + ComponentRegistry.register('cap', makeCapturingProbe(label), { + namespace: 'test', + skipFallback: true, + }); + ComponentRegistry.register('cap', makeCapturingProbe(label), { + namespace: 'element', + skipFallback: true, + }); + // Registered under a namespace so the bare `view:simple` fallback key is + // the one the node resolves through; registering with NO namespace would + // trip the registry's own deprecation warning. + ComponentRegistry.register('view:simple', makeCapturingProbe(label), { + namespace: 'test-6708', + }); + const { unmount } = renderWithData(CAPTURE_CASES.find(([l]) => l === label)![1]); + unmount(); + } + }); + + afterEach(() => { + vi.restoreAllMocks(); + ComponentRegistry.unregister?.('cap', 'test'); + ComponentRegistry.unregister?.('cap', 'element'); + ComponentRegistry.unregister?.('view:simple', 'test-6708'); + }); + + it('leg 2 — the key under `props` is EVALUATED and then not on the node', () => { + const leg = captured.componentProps as Record; + // Evaluation happened: the React prop holds the resolved array, not the + // `${...}` source. So this is not "the expression never ran". + expect((leg.restSnapshot as Record).data).toEqual(['ada', 'grace']); + // And the renderer, which reads `schema`, sees nothing. + expect(leg.schemaDotData).toBeNull(); + }); + + it('leg 3 — the SAME key under `properties` IS on the node', () => { + const leg = captured.propertiesOnly as Record; + expect(leg.schemaDotData).toEqual(['ada', 'grace']); + }); + + it('the two legs differ ONLY in the envelope — that pair is the card', () => { + const two = captured.componentProps as Record; + const three = captured.propertiesOnly as Record; + // Same value reaches the element on both legs... + expect((two.restSnapshot as Record).data).toEqual( + (three.restSnapshot as Record).data, + ); + // ...and only one of them reaches a `schema` reader. + expect(two.schemaDotData).toBeNull(); + expect(three.schemaDotData).not.toBeNull(); + }); + + it('changes NOTHING any renderer receives — every case, against the pre-diagnostic tree', () => { + // Acceptance pin 4, measured rather than asserted. `BASE_READING` was taken + // on the committed `SchemaRenderer.tsx`; this run has the diagnostic in it. + expect(captured).toEqual(BASE_READING); + }); + + it('objectui#5123 precedence is untouched: `properties` still wins a shared key', () => { + const both = (captured.bothBags as Record).restSnapshot as Record; + expect(both.a).toBe('fromProperties'); + expect(both.b).toBe('onlyProps'); + }); +}); + +/* -------------------------------------------------------------------------- * + * 3 — the diagnostic + * -------------------------------------------------------------------------- */ + +/** Reads `schema.`, like `statistic` / `card` / `data-table`. */ +const SchemaReadingProbe = ({ schema }: { schema: Record }) => ( +
{String(schema.title ?? '')}
+); + +/** Merges both bags, like the `element:*` family's `readProps()`. */ +const BothBagsProbe = ({ schema }: { schema: Record }) => { + const merged = { + ...((schema.props as Record) ?? {}), + ...((schema.properties as Record) ?? {}), + }; + return
{String(merged.content ?? '')}
; +}; + +describe('objectui#6708 — the SchemaRenderer-tier diagnostic', () => { + beforeEach(() => { + vi.spyOn(console, 'warn').mockImplementation(() => {}); + vi.spyOn(console, 'error').mockImplementation(() => {}); + __resetDroppedPropsBagWarnings(); + ComponentRegistry.register('probe', SchemaReadingProbe, { + namespace: 'test-6708', + skipFallback: true, + }); + ComponentRegistry.register('probe', BothBagsProbe, { + namespace: 'element', + skipFallback: true, + }); + ComponentRegistry.register('view:simple', SchemaReadingProbe, { namespace: 'test-6708' }); + }); + + afterEach(() => { + vi.restoreAllMocks(); + ComponentRegistry.unregister?.('probe', 'test-6708'); + ComponentRegistry.unregister?.('probe', 'element'); + ComponentRegistry.unregister?.('view:simple', 'test-6708'); + }); + + it('PIN 1 — fires on a component-renderer node, naming the node and pointing at `properties`', () => { + renderWithData({ + type: 'test-6708:probe', + id: 'customers-table', + props: { data: '${data.customers}' }, + }); + + const lines = warnings(); + expect(lines).toHaveLength(1); + // The ADDRESS: which node, spelled the way the author wrote it. + expect(lines[0]).toContain('`test-6708:probe`'); + expect(lines[0]).toContain("(id: 'customers-table')"); + // The KEY that was dropped. + expect(lines[0]).toContain('`data`'); + // The MECHANISM, and the way out the ruling asked it to point at. + expect(lines[0]).toContain('`props` is NOT hoisted onto the node'); + expect(lines[0]).toContain('Write it under `properties` instead'); + expect(lines[0]).toContain('objectui#6708'); + }); + + it('PIN 2 — stays SILENT on an `element:*` node, where `props` is legitimate', () => { + // That family's `readProps()` merges `{ ...schema.props, ...schema.properties }`, + // so the key is not dropped and a warning here would be false. The ruling + // pins this direction explicitly. + renderWithData({ type: 'element:probe', id: 'blurb', props: { content: 'hello' } }); + expect(warnings()).toEqual([]); + }); + + it('PIN 3 — stays SILENT on a plain `properties` node', () => { + renderWithData({ type: 'test-6708:probe', id: 'ok', properties: { title: 'hello' } }); + expect(warnings()).toEqual([]); + }); + + it('stays SILENT on a node that authored neither bag', () => { + renderWithData({ type: 'test-6708:probe', id: 'plain', title: 'hello' }); + expect(warnings()).toEqual([]); + }); + + it('is not fooled by an EMPTY `props` bag — nothing was authored, nothing was lost', () => { + renderWithData({ type: 'test-6708:probe', id: 'empty', props: {} }); + expect(warnings()).toEqual([]); + }); + + it('stays SILENT when `properties` already declares every `props` key', () => { + // objectui#5123 subtracted those keys from the outgoing bag, so the author + // is getting the canonical answer and nothing was silently dropped. + renderWithData({ + type: 'test-6708:probe', + id: 'shadowed', + props: { title: 'fromProps' }, + properties: { title: 'fromProperties' }, + }); + expect(warnings()).toEqual([]); + }); + + it('names ONLY the key `properties` does not also declare', () => { + renderWithData({ + type: 'test-6708:probe', + id: 'mixed', + props: { title: 'fromProps', subtitle: 'onlyProps' }, + properties: { title: 'fromProperties' }, + }); + const lines = warnings(); + expect(lines).toHaveLength(1); + expect(lines[0]).toContain('`subtitle`'); + expect(lines[0]).not.toContain('`title`'); + }); + + it('stays SILENT on `view:simple`, the one measured non-`element:` reader of the bag', () => { + renderWithData({ type: 'view:simple', id: 'grid', props: { columns: 3 } }); + expect(warnings()).toEqual([]); + }); + + it('stays SILENT on a degenerate, non-object `props`', () => { + // A different, shape-level defect with a different consequence, and there is + // no config bag to point the author at. + // + // This one is load-bearing rather than defensive. The evaluation memo + // rebuilds `props` with `{ ...newSchema.props }` under a bare truthiness + // guard, so by the time the diagnostic runs a string has already become + // `{ '0': 'n', '1': 'o', … }` — nine keys that ARE spread as React props and + // genuinely do not reach `schema`. Reading only that bag made this test fail + // with a message naming `schema.0`, which is why the AUTHORED bag is read + // too. Deleting that guard turns this red again. + renderWithData({ type: 'test-6708:probe', id: 'degenerate', props: 'not-a-bag' }); + expect(warnings()).toEqual([]); + }); + + it('stays SILENT on an array `props`, for the same reason', () => { + renderWithData({ type: 'test-6708:probe', id: 'arrayish', props: ['a', 'b'] }); + expect(warnings()).toEqual([]); + }); + + it('says it ONCE for one authoring bug repeated across nodes', () => { + // The census case the dedupe exists for: a generator emitting the same wrong + // envelope on many nodes. Those are distinct schema objects, so an + // object-keyed dedupe would print one line each. + renderWithData({ type: 'test-6708:probe', props: { data: 1 } }); + renderWithData({ type: 'test-6708:probe', props: { data: 1 } }); + expect(warnings()).toHaveLength(1); + }); + + it('still gives two genuinely different nodes two lines', () => { + renderWithData({ type: 'test-6708:probe', id: 'first', props: { data: 1 } }); + renderWithData({ type: 'test-6708:probe', id: 'second', props: { data: 1 } }); + expect(warnings()).toHaveLength(2); + }); + + it('does not repeat itself across a re-render of the same node', () => { + const node = { type: 'test-6708:probe', id: 'rerendered', props: { data: 1 } }; + const { rerender } = renderWithData(node); + rerender( + + + , + ); + expect(warnings()).toHaveLength(1); + }); +}); + +/* -------------------------------------------------------------------------- * + * The pure halves, asserted directly — a diagnostic whose only test is "a spy + * was called" goes green the moment someone no-ops it. + * -------------------------------------------------------------------------- */ + +describe('objectui#6708 — readsPropsBag', () => { + it.each([ + ['element:text', true], + ['element:definition-list', true], + ['element:', true], + ['view:simple', true], + ['data-table', false], + ['card', false], + ['statistic', false], + ['view:grid', false], + ['text', false], + ['', false], + ] as const)('%s -> %s', (type, expected) => { + expect(readsPropsBag(type)).toBe(expected); + }); + + it('answers false for a non-string type rather than throwing', () => { + expect(readsPropsBag(undefined)).toBe(false); + expect(readsPropsBag(42)).toBe(false); + expect(readsPropsBag(null)).toBe(false); + }); +}); + +describe('objectui#6708 — collectDroppedPropsKeys', () => { + it('returns the authored keys for a component-renderer node', () => { + const bag = { data: 1, title: 'x' }; + expect(collectDroppedPropsKeys('card', bag, bag)).toEqual(['data', 'title']); + }); + + it.each([ + ['the readProps family', 'element:text', { content: 'x' }, { content: 'x' }], + ['an empty outgoing bag', 'card', { title: 'x' }, {}], + ['no bag at all', 'card', undefined, undefined], + ['a string authored bag', 'card', 'nope', { 0: 'n', 1: 'o' }], + ['an array authored bag', 'card', ['nope'], { 0: 'nope' }], + ['a null authored bag', 'card', null, null], + ] as const)('returns null for %s', (_label, type, authored, outgoing) => { + expect(collectDroppedPropsKeys(type, authored, outgoing)).toBeNull(); + }); + + it('reads BOTH bags — an authored object with an emptied outgoing bag is silent', () => { + // The objectui#5123 case: `properties` declared the same key, so it was + // subtracted from the outgoing bag and the author has the canonical answer. + expect(collectDroppedPropsKeys('card', { title: 'fromProps' }, {})).toBeNull(); + }); +}); + +describe('objectui#6708 — formatDroppedPropsBagMessage', () => { + it('reads as one sentence for a single key', () => { + const message = formatDroppedPropsBagMessage('card', 'summary', ['title']); + expect(message.startsWith(DROPPED_PROPS_BAG_PREFIX)).toBe(true); + expect(message).toContain('Key under `props`: `title`'); + expect(message).toContain('`schema.title` is undefined'); + expect(message).toContain('Write it under `properties` instead'); + }); + + it('pluralises, and lists every key, for more than one', () => { + const message = formatDroppedPropsBagMessage('card', undefined, ['title', 'subtitle']); + expect(message).toContain('Keys under `props`: `title`, `subtitle`'); + expect(message).toContain('Write them under `properties` instead'); + // No id was authored, so none is claimed. + expect(message).not.toContain('(id:'); + }); + + it('does not pretend to know an untyped node', () => { + expect(formatDroppedPropsBagMessage(undefined, undefined, ['x'])).toContain('(untyped node)'); + }); +}); diff --git a/packages/react/src/utils/propsBagDiagnostic.ts b/packages/react/src/utils/propsBagDiagnostic.ts new file mode 100644 index 000000000..d54ef23d1 --- /dev/null +++ b/packages/react/src/utils/propsBagDiagnostic.ts @@ -0,0 +1,260 @@ +/** + * 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. + */ + +/** + * Dev-build diagnostic: a `props` CONFIG BAG on a node whose renderer reads its + * config from `schema` — so the keys inside it are evaluated, spread as React + * props, and then never read (objectui#6708). + * + * ## The defect this names + * + * `SchemaRenderer` HOISTS every `properties.*` value onto the node (minus + * `HOIST_PROTECTED_KEYS`), so a key written under `properties` is a real value + * on `schema.` by the time a renderer destructures it. `props` is NOT + * hoisted: it is spread as React props on the created element. A renderer + * declared as `({ schema })` — the normal shape for the component renderers — + * therefore never sees it. + * + * Measured through the real `SchemaRenderer` on `faac0d935`, one node per row, + * with a probe renderer that records both channels: + * + * { type: 'test:cap', props: { data: '${data.customers}' } } + * -> React prop `data` = the evaluated array, `schema.data` = undefined + * { type: 'test:cap', properties: { data: '${data.customers}' } } + * -> React prop `data` = the evaluated array, `schema.data` = the array + * + * Same key, same value, one envelope apart. The card's own four-leg reading of + * the same asymmetry through a real `data-table` is pinned in + * `packages/components/src/__tests__/data-table-node-data-diagnostic.test.tsx` + * (objectui#6665): the `props` leg renders `No results found`, the `properties` + * leg renders the rows. + * + * Every gate accepts the `props` spelling — `BaseSchema` is `.passthrough()` + * with `[key: string]: any` — and `props` is documented as the annotated legacy + * alias of the config bag, so nothing between the author and the screen says a + * word. That is the success-receipt shape objectui#6575 and objectui#6665 exist + * to remove; this is the third instance, and the first at a tier that covers + * every renderer instead of one component. + * + * ## What it deliberately does NOT do + * + * It changes NO behaviour. Hoisting `props` to parity with `properties` was + * option 1 and the maintainer REFUSED it (ruling 2026-08-29, verbatim + * 「同意」 on option 2): hoisting would weld the legacy alias in as a permanent + * second spelling, the opposite of this repo's alias-retirement direction. + * Refusing the key at parse was option 3 and stays blocked on the + * `.passthrough()` ceiling (objectui#5155 / objectui#6269). So the trap stops + * being silent; it does not stop being a trap, and what every renderer receives + * is byte-for-byte what it received before — pinned directly, not asserted, in + * `SchemaRenderer.propsBagDiagnostic.test.tsx`. + * + * ## Why `console.warn`, when its neighbour in this directory uses `error` + * + * Two conventions cross here and the choice follows the SHAPE, not the tier. + * `unevaluatedExpression.ts` sits at this same tier and shouts with + * `console.error`, but its subject is different: a raw `${…}` placed VERBATIM + * in front of a user. Nothing is placed here — a value is dropped. That is + * exactly objectui#6575's and objectui#6665's subject ("you declared something + * and the renderer dropped it"), and both of those emit `console.warn`. The + * card names this the third instance of that shape, so it joins that family. + * + * ## Level and dedupe were chosen from a census, not from taste + * + * The ruling fixed the order: measure component-level `props` usage across the + * in-repo corpus FIRST, and tune level/dedupe so the diagnostic informs rather + * than floods. Measured on `faac0d935` by walking every JSON document, every + * `json` fence in every `.md`/`.mdx`, and every TypeScript object literal in + * the repo (TS compiler API) for nodes carrying both `type` and `props`: + * 39 such nodes, of which 22 are on component-renderer types — and 19 of those + * 22 are test fixtures exercising this exact shape on purpose. The authored, + * non-test corpus holds 5, none of them in application runtime metadata. + * + * So there is nothing to flood, and the level is not softened for volume. The + * dedupe is still keyed on the MESSAGE rather than on the schema object, + * because the failure the census makes plausible is a metadata GENERATOR that + * emits the same wrong envelope on many nodes: those are distinct schema + * objects, so an object-keyed `WeakSet` (the shape + * `reportUnevaluatedExpressions` uses next door) would print one line per node + * for one authoring bug. Keying on the rendered message collapses that to one + * line while still giving two genuinely different nodes two lines. Full census + * table: the objectui#6708 PR body. + */ + +/** + * The namespace whose renderers merge BOTH bags. + * + * Every `readProps()` in this repo that merges `{ ...schema.props, + * ...schema.properties }` belongs to a component registered with + * `namespace: 'element'` — measured by reading all five of them on + * `faac0d935`: `elements.tsx` (`element:text` / `divider` / `image` / `button` + * / `number`), `data-list.tsx` (`definition-list` / `repeater`), + * `text-input.tsx`, `record-picker.tsx` and `metadata-viewer.tsx`. For that + * family `props` is a legitimate spelling, so silence there is correct rather + * than a gap — and it is the direction the ruling pins explicitly. + */ +const ELEMENT_NAMESPACE_PREFIX = 'element:'; + +/** + * Node types OUTSIDE the `element:` namespace that nevertheless read the raw + * `props` bag off the schema, so the diagnostic must not claim their keys were + * dropped. + * + * Derived, not guessed: a repo-wide grep for reads of `schema.props` / + * `schema?.props` on `faac0d935` returns the `element:` family above plus + * exactly these. `plugin-view`'s `SimpleViewRenderer` reads + * `schema.props?.columns` for its grid layout. + * + * ⚠️ This list is a MEASUREMENT of the current tree, and its cost is stated + * rather than hidden: `view:simple` reads exactly one key out of the bag, so a + * `props` key other than `columns` on a `view:simple` node IS dropped and is + * NOT diagnosed. Silence on a node where the envelope is partly legitimate was + * preferred to a message that asserts a drop it did not check — a diagnostic + * that overstates its own consequence teaches authors to distrust it + * (objectui#6665's `describeIgnoredBind` makes the same trade). The honest way + * to shrink this list is to stop reading the legacy bag in that renderer, which + * is a behaviour change on a published component and not this card's to make. + */ +const NON_ELEMENT_PROPS_BAG_READERS: ReadonlySet = new Set(['view:simple']); + +/** + * Does this node's renderer read the `props` bag as a config bag? + * + * Exported so the report and its pins cannot drift apart: one predicate, two + * readers. A non-string `type` answers `false` — an untyped node cannot be in + * a family — and the caller has already resolved a component for it, so the + * unknown-type box is not in play. + */ +export function readsPropsBag(type: unknown): boolean { + if (typeof type !== 'string' || type.length === 0) return false; + if (type.startsWith(ELEMENT_NAMESPACE_PREFIX)) return true; + return NON_ELEMENT_PROPS_BAG_READERS.has(type); +} + +/** Prefix every line starts with — the handle tests, greps and log filters hold. */ +export const DROPPED_PROPS_BAG_PREFIX = '[ObjectUI] A `props` config bag was not read'; + +/** Where the offending node lives, for the first line of the message. */ +function describeAddress(type: unknown, id: unknown): string { + const node = typeof type === 'string' && type ? `\`${type}\`` : '(untyped node)'; + const where = typeof id === 'string' && id ? ` (id: '${id}')` : ''; + return `${node}${where}`; +} + +/** + * Build the message. Separate from the emit so a test can assert the words a + * developer is going to read, not merely that something was logged. + */ +export function formatDroppedPropsBagMessage( + type: unknown, + id: unknown, + droppedKeys: readonly string[], +): string { + const keyList = droppedKeys.map(k => `\`${k}\``).join(', '); + const one = droppedKeys.length === 1; + return ( + `${DROPPED_PROPS_BAG_PREFIX} - node ${describeAddress(type, id)}\n` + + ` ${one ? 'Key' : 'Keys'} under \`props\`: ${keyList}\n` + + '`props` is NOT hoisted onto the node — only `properties.*` is. It is spread as\n' + + `React props on the created element, so \`schema.${droppedKeys[0]}\` is undefined and a\n` + + `renderer declared as \`({ schema })\` never sees ${one ? 'this key' : 'these keys'}. Nothing throws and\n` + + 'nothing is logged by the renderer: the node renders as if the bag were empty.\n' + + ` Write ${one ? 'it' : 'them'} under \`properties\` instead (or at node level, where the node's\n` + + 'schema declares the key). The `element:*` renderers merge both bags via\n' + + '`readProps()`; every other renderer reads `schema`. (objectui#6708)' + ); +} + +/** + * Reported messages, so a re-render — or a second node carrying the same + * authoring bug — does not repeat the line. Module state, exactly like + * `visibilityDiagnostic.ts`'s `Set`, and reset the same way for tests. + */ +const _warnedPropsBags = new Set(); + +/** + * Test-only reset for the dedupe above. Without it the second test to assert + * the same warning reads the first test's dedupe entry and sees silence — a + * green run that checked nothing. + */ +export function __resetDroppedPropsBagWarnings(): void { + _warnedPropsBags.clear(); +} + +/** A real config bag: an object, and not an array pretending to be one. */ +function isConfigBag(value: unknown): value is Record { + return !!value && typeof value === 'object' && !Array.isArray(value); +} + +/** + * Which keys of the OUTGOING props bag are dropped by a `schema`-reading + * renderer, or `null` when there is nothing to say. + * + * Two bags are read, and needing both is a MEASUREMENT rather than caution. + * + * `outgoingPropsBag` is the bag `SchemaRenderer` actually spreads — the value + * of `propsWithoutCanonicalKeys(schema.props, schema.properties)`, passed in + * rather than re-derived. That is what makes the message honest about the two + * cases it must not confuse: + * + * - a key BOTH bags declare has already been subtracted by that function + * (objectui#5123: `properties` wins on both channels), so the author is + * getting the canonical answer and nothing was silently dropped; + * - a key only `props` declares survives into that bag, and it is exactly the + * key that reaches no `schema` reader. + * + * `authoredPropsBag` is what the AUTHOR wrote, read off the original schema + * before the evaluation memo touched it. It is needed because that memo + * rebuilds the bag with `{ ...newSchema.props }` under a bare truthiness guard, + * so a degenerate `props: 'not-a-bag'` arrives at the spread site as + * `{ '0': 'n', '1': 'o', … }` — measured, not supposed. Reading only the + * outgoing bag would therefore report nine dropped keys named `0` … `8` and + * tell the author that `schema.0` is undefined, which is true and useless. A + * string is not a config bag; there is nothing to point at, and the shape-level + * defect it represents is a different question from this one. + * + * Deliberately narrow, and each exclusion is a reading rather than an omission: + * + * - an EMPTY bag says nothing was authored and nothing was lost; + * - a non-object authored `props` is the degenerate case above; + * - a node in the {@link readsPropsBag} family is silent by the ruling. + */ +export function collectDroppedPropsKeys( + type: unknown, + authoredPropsBag: unknown, + outgoingPropsBag: unknown, +): string[] | null { + if (readsPropsBag(type)) return null; + if (!isConfigBag(authoredPropsBag)) return null; + if (!isConfigBag(outgoingPropsBag)) return null; + const keys = Object.keys(outgoingPropsBag); + return keys.length > 0 ? keys : null; +} + +/** + * Dev-build only. Reports once per distinct message via `console.warn`, and + * returns the message it emitted (or `null`) so a caller or a test can read the + * decision rather than infer it from a spy. + * + * The caller applies the production gate, so this stays a single dev-only + * branch at the call site and the whole module is dead code in a production + * build. + */ +export function reportDroppedPropsBag( + type: unknown, + id: unknown, + authoredPropsBag: unknown, + outgoingPropsBag: unknown, +): string | null { + const droppedKeys = collectDroppedPropsKeys(type, authoredPropsBag, outgoingPropsBag); + if (!droppedKeys) return null; + const message = formatDroppedPropsBagMessage(type, id, droppedKeys); + if (_warnedPropsBags.has(message)) return null; + _warnedPropsBags.add(message); + console.warn(message); + return message; +}