diff --git a/.changeset/6783-readprops-degenerate-config-bag.md b/.changeset/6783-readprops-degenerate-config-bag.md new file mode 100644 index 0000000000..8e1594ae2c --- /dev/null +++ b/.changeset/6783-readprops-degenerate-config-bag.md @@ -0,0 +1,12 @@ +--- +'@object-ui/components': patch +'@object-ui/react': minor +--- + +`element:*` renderers stop re-reading a degenerate config bag as its own character indices — the third and last channel of the objectui#6752 / objectui#6760 hazard (objectui#6783). + +Five modules under `packages/components/src/renderers/basic/` — `elements.tsx`, `data-list.tsx`, `text-input.tsx`, `record-picker.tsx`, `metadata-viewer.tsx` — each carried a copy of the same reader, `{ ...(schema?.props ?? {}), ...(schema?.properties ?? {}) }`. `??` only replaces `null`/`undefined`, so a non-object bag went into the object spread and came back out as indexed keys: for `properties: 'not-a-bag'`, the config bag a renderer received was `{ '0': 'n', '1': 'o', … '8': 'g' }` — nine keys nobody authored. The five copies are now one `readProps` (`renderers/basic/readProps.ts`) that asks `isConfigBag`, and a degenerate bag on either side contributes no keys. + +`@object-ui/react` exports `isConfigBag` from its package entry. That is the API addition here, and it is the reason the fix is not a sixth spelling of the predicate: objectui#6761 converged six occurrences of "is this a real config bag?" behind one definition in `packages/react/src/utils/configBag.ts` and pinned it, but the pin scans `packages/react/src` only — a copy written one package over would be invisible to it. `@object-ui/components` already depends on `@object-ui/react` (all five modules import from it today), so the reachable answer was to publish the definition rather than retell it. Same reason the node-gate predicate reporter is exported at that entry (objectui#6038): one definition, read by every package that asks. + +**What this does not change, measured rather than predicted.** No rendered output moves on today's tree. All five renderers read named keys off this bag, and the single onward spread — `metadata-viewer`'s `` — hands it to components that destructure named fields, so the indexed keys were computed and then dropped. The census behind objectui#6708 found zero authored nodes carrying a degenerate config bag, so this was a latent shape, not a live failure. What the guard buys is what objectui#6752 measured its own guard buys, one channel further down: the authored value's shape is not reinterpreted. objectui#5123's precedence is untouched — `properties` still wins a contested key, and a degenerate bag declares no key for either side to win. diff --git a/packages/components/src/renderers/basic/__tests__/degenerate-config-bag.test.tsx b/packages/components/src/renderers/basic/__tests__/degenerate-config-bag.test.tsx new file mode 100644 index 0000000000..aa386e9031 --- /dev/null +++ b/packages/components/src/renderers/basic/__tests__/degenerate-config-bag.test.tsx @@ -0,0 +1,255 @@ +/** + * 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#6783 — the THIRD channel of the degenerate-config-bag hazard: the + * bag an `element:*` renderer reads, `{ ...schema.props, ...schema.properties }`. + * + * `??` only replaces `null`/`undefined`, so a non-object bag went straight into + * the object spread and was re-read as its own character indices. Five modules + * under `renderers/basic/` carried a copy of that read — `elements.tsx`, + * `data-list.tsx`, `text-input.tsx`, `record-picker.tsx`, + * `metadata-viewer.tsx` — and none of them asked the question + * `packages/react` converged on one answer for (objectui#6761's `isConfigBag`). + * + * ## BASE_READING — measured on `107babef6`, this branch's base + * + * Captured by running the legs below against the five pre-fix copies (the fix + * reverted in the worktree, `readProps.ts` absent), and pasted verbatim: + * + * readProps({ type, properties: 'not-a-bag' }) + * -> keys ["0","1","2","3","4","5","6","7","8"] + * readProps({ type, props: 'not-a-bag' }) + * -> keys ["0","1","2","3","4","5","6","7","8"] + * readProps({ type, props: { content: 'X' }, properties: 'not-a-bag' }) + * -> keys ["0","1","2","3","4","5","6","7","8","content"] + * readProps({ type, properties: ['a','b'] }) + * -> keys ["0","1"] + * + * ⇒ nine keys nobody authored, on the same node shape objectui#6752 and + * objectui#6760 already cleaned upstream. + * + * ## What did NOT move, also measured on the same base + * + * Ablating this card back to the pre-fix tree — the shared reader's body + * returned to `?? {}` AND all five modules restored from `107babef6` — moves + * 7 of the 16 assertions below and leaves 9 standing. Every DOM assertion in + * "what the guard does not buy" is among the 9. That is the honest reading of this card and it is + * recorded rather than smoothed over: all five renderers read NAMED keys off + * this bag, and the single onward spread — `metadata-viewer`'s + * `` — hands the bag to components that + * destructure named `ViewerProps` fields. So the indexed keys were computed and + * then dropped, and no rendered output on this base changes either way. The + * guard buys what objectui#6752 measured ITS guard buys: the authored value's + * shape is not reinterpreted. Nothing more, today; the property starts paying + * the moment one of these five spreads its bag onto a DOM element, which + * `metadata-viewer` is one refactor away from. + * + * ## The three channels are in SERIES, and this leg proves it on this base + * + * The probe below drives a node through the REAL `SchemaRenderer`, so both + * upstream guards are in force, and reads what the renderer actually receives. + * Measured on `107babef6`: `schema.properties` still holds the authored + * `'not-a-bag'` when it reaches the component. Upstream does not sanitize this + * channel — by design (objectui#6752's guard exists to PRESERVE the authored + * shape) — so the renderer-side read is the only thing that can close it. + */ + +import { describe, it, expect, afterEach } from 'vitest'; +import * as React from 'react'; +import { render, cleanup } from '@testing-library/react'; +import { readFileSync, readdirSync } from 'node:fs'; +import path from 'node:path'; +import { fileURLToPath } from 'node:url'; +import { ComponentRegistry } from '@object-ui/core'; +import { SchemaRenderer } from '@object-ui/react'; +import { readProps } from '../readProps'; +// Registers every `element:*` renderer at module scope, not in a hook +// (object-ui/no-dynamic-import-in-test-hook, objectui#3010). +import '../../../renderers'; + +afterEach(cleanup); + +const keysOf = (schema: unknown) => Object.keys(readProps(schema)); + +describe('objectui#6783 — a degenerate config bag contributes no keys', () => { + it('a degenerate `properties` is not enumerated into indexed keys', () => { + // BASE_READING: ["0" … "8"]. + expect(keysOf({ type: 'element:text', properties: 'not-a-bag' })).toEqual([]); + }); + + it('a degenerate `props` is not enumerated either — the same question on the alias', () => { + // BASE_READING: ["0" … "8"]. + expect(keysOf({ type: 'element:text', props: 'not-a-bag' })).toEqual([]); + }); + + it('a degenerate bag on ONE side leaves the other side intact', () => { + // BASE_READING: ["0" … "8","content"], in that order — the ablation + // corrected the prediction here. The nine indices sort AHEAD of the + // authored key whatever the spread order, because integer-like keys come + // first in JS property order; `content` itself survived pre-fix, so what + // this pins is the ABSENCE of the nine, not the presence of `content`. + expect(keysOf({ props: { content: 'X' }, properties: 'not-a-bag' })).toEqual(['content']); + expect(keysOf({ props: 'not-a-bag', properties: { content: 'Y' } })).toEqual(['content']); + }); + + it('an ARRAY is degenerate too — `typeof [] === "object"` is why the predicate has two halves', () => { + // BASE_READING: ["0","1"]. + expect(keysOf({ properties: ['a', 'b'] })).toEqual([]); + }); + + it('the empty-ish values behave exactly as they did — `??` and the predicate agree here', () => { + expect(keysOf({})).toEqual([]); + expect(keysOf({ properties: null, props: undefined })).toEqual([]); + expect(keysOf(undefined)).toEqual([]); + // A number spreads to nothing even pre-fix; pinned so the fix is not read + // as having introduced this. + expect(keysOf({ properties: 42 })).toEqual([]); + }); + + it('objectui#5123 precedence is untouched: `properties` wins a contested key', () => { + const bag = readProps<{ content?: string; only?: string }>({ + props: { content: 'FROM_PROPS', only: 'FROM_PROPS' }, + properties: { content: 'FROM_PROPERTIES' }, + }); + expect(bag.content).toBe('FROM_PROPERTIES'); + expect(bag.only).toBe('FROM_PROPS'); + }); +}); + +describe('objectui#6783 — the three channels are in series, measured end to end', () => { + const PROBE = 'test:degenerate_bag_probe'; + + function registerProbe() { + ComponentRegistry.register( + 'degenerate_bag_probe', + ({ schema }: { schema: any }) => ( +
+ ), + { namespace: 'test', skipFallback: true } + ); + if (!ComponentRegistry.get(PROBE)) throw new Error(`${PROBE} is not registered`); + } + + it('the authored degenerate value still REACHES the renderer through SchemaRenderer', () => { + registerProbe(); + const { getByTestId } = render( + + ); + + // Neither objectui#6752's evaluation guard nor objectui#6760's hoist + // sanitizes the authored value — they PRESERVE its shape, which is what + // leaves this third channel the only place the question can be answered. + const probe = getByTestId('probe'); + expect(probe.getAttribute('data-authored-type')).toBe('string'); + expect(probe.getAttribute('data-authored-properties')).toBe('not-a-bag'); + }); + + it('and the bag that renderer computes from it now carries nothing', () => { + registerProbe(); + const { getByTestId } = render( + + ); + + // BASE_READING through the same path: "0,1,2,3,4,5,6,7,8". + expect(getByTestId('probe').getAttribute('data-bag-keys')).toBe(''); + }); +}); + +describe('objectui#6783 — what the guard does not buy (measured, not predicted)', () => { + // One renderer per module that carried a copy. Each renders identically with + // a degenerate bag and with no bag at all — GREEN on the pre-fix tree too. + const CASES: Array<[string, string]> = [ + ['elements.tsx', 'element:text'], + ['data-list.tsx', 'element:definition-list'], + ['text-input.tsx', 'element:text_input'], + ['record-picker.tsx', 'element:record_picker'], + ['metadata-viewer.tsx', 'element:metadata_viewer'], + ]; + + it.each(CASES)('%s — `%s` renders the same with a degenerate bag as with none', (_file, type) => { + const C = ComponentRegistry.get(type) as React.ComponentType; + if (!C) throw new Error(`${type} is not registered`); + + const degenerate = render(); + const degenerateHtml = degenerate.container.innerHTML; + cleanup(); + + const absent = render(); + expect(degenerateHtml).toBe(absent.container.innerHTML); + }); +}); + +/** + * The convergence, ratcheted. objectui#6761's lesson is that convergence alone + * fixes today and not tomorrow: every spelling of this read is an expression + * that produces no error when it drifts, so nothing would report a sixth copy + * appearing next week. This scan makes the CHEAP path — copying the four lines + * that used to be in each of these modules — fail. + */ +describe('objectui#6783 — one reader, and the pin that keeps it one', () => { + const here = path.dirname(fileURLToPath(import.meta.url)); + const BASIC_ROOT = path.resolve(here, '..'); + + const productionSources = () => + readdirSync(BASIC_ROOT, { withFileTypes: true }) + .filter((e) => e.isFile() && /\.tsx?$/.test(e.name) && !/\.test\.tsx?$/.test(e.name)) + .map((e) => e.name) + .sort(); + + /** `schema.props ?? {}` / `schema?.properties || {}` — either bag, either fallback. */ + const LOCAL_BAG_READ = + /\??\.\s*(?:props|properties)\s*(?:\?\?|\|\|)\s*\{\s*\}/g; + + /** + * Comments out first: this scans for a runtime READ, and the shared reader's + * own docblock quotes the four lines it replaced verbatim. Prose naming the + * removed spelling is the opposite of the drift being pinned — reporting it + * would train the next author to stop writing the explanation. + */ + const stripComments = (source: string) => + source.replace(/\/\*[\s\S]*?\*\//g, '').replace(/(^|[^:'"`])\/\/[^\n]*/g, '$1'); + + it('no module under renderers/basic reads a config bag with its own `?? {}` fallback', () => { + const found: string[] = []; + for (const name of productionSources()) { + const source = stripComments(readFileSync(path.join(BASIC_ROOT, name), 'utf8')); + LOCAL_BAG_READ.lastIndex = 0; + let match: RegExpExecArray | null; + while ((match = LOCAL_BAG_READ.exec(source)) !== null) { + found.push(`${name}: ${match[0].replace(/\s+/g, ' ')}`); + } + } + // Every one of these is the copy objectui#6783 removed. Adding an entry + // here is claiming a NEW question; importing `readProps` is saying it is + // this one. + expect(found).toEqual([]); + }); + + it('every module that reads a config bag imports the shared reader', () => { + for (const name of ['elements.tsx', 'data-list.tsx', 'text-input.tsx', 'record-picker.tsx', 'metadata-viewer.tsx']) { + const source = readFileSync(path.join(BASIC_ROOT, name), 'utf8'); + expect(source, `${name} no longer imports the shared reader`).toMatch( + /import \{ readProps \} from '\.\/readProps';/ + ); + } + }); + + it('the shared reader asks `@object-ui/react`’s one predicate, not a local retelling', () => { + const source = readFileSync(path.join(BASIC_ROOT, 'readProps.ts'), 'utf8'); + expect(source).toMatch(/import \{ isConfigBag \} from '@object-ui\/react';/); + // The conjunction objectui#6761's own pin scans for, spelled here would be + // the seventh copy — one package over, where that pin cannot see it. + expect(source).not.toMatch(/Array\.isArray/); + }); +}); diff --git a/packages/components/src/renderers/basic/data-list.tsx b/packages/components/src/renderers/basic/data-list.tsx index d6c56c49f8..d4e29868a8 100644 --- a/packages/components/src/renderers/basic/data-list.tsx +++ b/packages/components/src/renderers/basic/data-list.tsx @@ -21,12 +21,7 @@ import * as React from 'react'; import { ComponentRegistry } from '@object-ui/core'; import { useAdapter } from '@object-ui/react'; import { cn } from '../../lib/utils'; - -function readProps>(schema: any): T { - const fromProperties = (schema?.properties ?? {}) as T; - const fromProps = (schema?.props ?? {}) as T; - return { ...fromProps, ...fromProperties }; -} +import { readProps } from './readProps'; function toText(v: unknown): string { if (v == null || v === '') return '—'; diff --git a/packages/components/src/renderers/basic/elements.tsx b/packages/components/src/renderers/basic/elements.tsx index 63350e0eb3..b2e2cf7d86 100644 --- a/packages/components/src/renderers/basic/elements.tsx +++ b/packages/components/src/renderers/basic/elements.tsx @@ -39,19 +39,12 @@ import { import { cn } from '../../lib/utils'; import { LazyIcon } from '../../lib/lazy-icon'; import { Button, Separator } from '../../ui'; +import { readProps } from './readProps'; // --------------------------------------------------------------------------- // Shared helpers // --------------------------------------------------------------------------- -function readProps>(schema: any): T { - // Per spec, element components carry their config in `schema.properties`. - // Tolerate `schema.props` (legacy alias) so JSON written either way works. - const fromProperties = (schema?.properties ?? {}) as T; - const fromProps = (schema?.props ?? {}) as T; - return { ...fromProps, ...fromProperties }; -} - function ariaAttrs(aria?: Record): Record { if (!aria || typeof aria !== 'object') return {}; const out: Record = {}; diff --git a/packages/components/src/renderers/basic/metadata-viewer.tsx b/packages/components/src/renderers/basic/metadata-viewer.tsx index 1766dadc2b..82a1dcbcb5 100644 --- a/packages/components/src/renderers/basic/metadata-viewer.tsx +++ b/packages/components/src/renderers/basic/metadata-viewer.tsx @@ -33,6 +33,7 @@ import { AlertTriangle, } from 'lucide-react'; import { cn } from '../../lib/utils'; +import { readProps } from './readProps'; // --------------------------------------------------------------------------- // Shared helpers @@ -46,12 +47,6 @@ interface ViewerProps { detail?: 'business' | 'technical'; } -function readProps(schema: any): ViewerProps { - const fromProperties = (schema?.properties ?? {}) as ViewerProps; - const fromProps = (schema?.props ?? {}) as ViewerProps; - return { ...fromProps, ...fromProperties }; -} - /** Tolerate `fields` as either an object map or an array of `{name,...}`. */ function getField(obj: any, name?: string): any { if (!obj || !name) return undefined; @@ -360,7 +355,7 @@ function PermissionView({ name }: ViewerProps) { // --------------------------------------------------------------------------- export function ElementMetadataViewerRenderer({ schema }: { schema: any }) { - const props = readProps(schema); + const props = readProps(schema); switch (props.type) { case 'state_machine': return ; diff --git a/packages/components/src/renderers/basic/readProps.ts b/packages/components/src/renderers/basic/readProps.ts new file mode 100644 index 0000000000..fb1313c60b --- /dev/null +++ b/packages/components/src/renderers/basic/readProps.ts @@ -0,0 +1,75 @@ +/** + * 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. + */ + +import { isConfigBag } from '@object-ui/react'; + +/** + * The config bag an `element:*` renderer reads — THE one definition of it in + * `@object-ui/components` (objectui#6783). + * + * Per spec, element components carry their config in `schema.properties`; + * `schema.props` is tolerated as the legacy alias so JSON written either way + * works. `properties` wins a contested key — the 2026-08-18 cross-channel + * ruling (objectui#5123), whose spread order is preserved here verbatim and + * pinned by `packages/components/src/__tests__/alias-precedence-cross-channel.test.tsx`. + * + * ## The third channel (objectui#6783) + * + * This body used to be copied into five renderer modules — `elements.tsx`, + * `data-list.tsx`, `text-input.tsx`, `record-picker.tsx`, + * `metadata-viewer.tsx` — each spelling the two halves as + * `(schema?.properties ?? {}) as T`. `??` only replaces `null`/`undefined`, so + * a DEGENERATE bag (a string, a number, an array) went straight into the + * object spread and was re-read as its own indexed keys: for + * `properties: 'not-a-bag'` the bag a renderer received was + * `{ '0': 'n', '1': 'o', … '8': 'g' }` — nine keys nobody authored. + * + * That is the same hazard objectui#6752 and objectui#6760 closed in + * `packages/react/src/SchemaRenderer.tsx` on the two channels UPSTREAM of this + * one (the `props` evaluation memo plus `propsWithoutCanonicalKeys`, and the + * `properties` hoist). Those guards do not reach here, and are not meant to: + * what they buy is that the AUTHORED value's shape survives into the node, so + * a renderer downstream still receives `schema.properties === 'not-a-bag'` and + * gets to answer the question for itself. The three channels are in SERIES. + * + * ## Why it asks `isConfigBag` rather than carrying its own predicate + * + * Because a copy that DRIFTS produces no error — each spelling is a boolean + * expression, so two channels answering "is this a real config bag?" + * differently is silent by construction. objectui#6761 ended that inside + * `@object-ui/react` by converging six occurrences behind one exported + * definition and pinning it (`utils/configBag.pin.test.ts` fails on a seventh + * spelling); `@object-ui/components` depends on `@object-ui/react` — every one + * of the five modules above already imports from it — so the reachable answer + * here is to READ that definition, not to write the seventh spelling one + * package over where its pin cannot see it. + * + * ## What the guard buys, measured + * + * Not a rendered pixel, on today's tree: all five renderers read NAMED keys + * off this bag, and the one onward spread (`metadata-viewer`'s + * ``) hands them to components that destructure + * named fields, so the indexed keys were computed and then discarded. What it + * buys is the same thing objectui#6752 measured its own guard buys — the + * authored value's shape is not reinterpreted — one channel further down, + * which is exactly the property that stops mattering only until the first of + * these five spreads its bag onto a DOM element. + */ +export function readProps>(schema: any): T { + // A degenerate bag contributes NO keys, on either side. This does not move + // objectui#5123's precedence: that rule decides which of two co-present + // values a key carries, and a degenerate bag declares no key for either bag + // to win — the indices were never authored, they are the object spread's + // reading of a string. + const fromProps = isConfigBag(schema?.props) ? schema.props : {}; + const fromProperties = isConfigBag(schema?.properties) ? schema.properties : {}; + // Cast once, on the merged result. The five copies this replaces cast each + // HALF (`(schema?.properties ?? {}) as T`), which asserted `T` of a value + // that could be a string. + return { ...fromProps, ...fromProperties } as T; +} diff --git a/packages/components/src/renderers/basic/record-picker.tsx b/packages/components/src/renderers/basic/record-picker.tsx index d990059569..7782fd6daf 100644 --- a/packages/components/src/renderers/basic/record-picker.tsx +++ b/packages/components/src/renderers/basic/record-picker.tsx @@ -51,14 +51,7 @@ import { SelectItem, } from '../../ui'; import { cn } from '../../lib/utils'; - -function readProps>(schema: any): T { - // Per spec, element components carry their config in `schema.properties`. - // Tolerate `schema.props` (legacy alias) so JSON written either way works. - const fromProperties = (schema?.properties ?? {}) as T; - const fromProps = (schema?.props ?? {}) as T; - return { ...fromProps, ...fromProperties }; -} +import { readProps } from './readProps'; function toText(v: unknown): string { if (v == null) return ''; diff --git a/packages/components/src/renderers/basic/text-input.tsx b/packages/components/src/renderers/basic/text-input.tsx index 25a2ce075e..9dc08d2c1f 100644 --- a/packages/components/src/renderers/basic/text-input.tsx +++ b/packages/components/src/renderers/basic/text-input.tsx @@ -30,14 +30,7 @@ import { usePageVariableBinding } from '@object-ui/react'; import { useObjectTranslation, pickLocalized } from '@object-ui/i18n'; import { Input, Label } from '../../ui'; import { cn } from '../../lib/utils'; - -function readProps>(schema: any): T { - // Per spec, element components carry their config in `schema.properties`. - // Tolerate `schema.props` (legacy alias) so JSON written either way works. - const fromProperties = (schema?.properties ?? {}) as T; - const fromProps = (schema?.props ?? {}) as T; - return { ...fromProps, ...fromProperties }; -} +import { readProps } from './readProps'; type TextInputType = 'text' | 'email' | 'number' | 'tel' | 'url' | 'password'; const INPUT_TYPES: TextInputType[] = ['text', 'email', 'number', 'tel', 'url', 'password']; diff --git a/packages/react/src/index.ts b/packages/react/src/index.ts index 717f85b44f..7cbf9ffe3d 100644 --- a/packages/react/src/index.ts +++ b/packages/react/src/index.ts @@ -19,6 +19,17 @@ export * from './element-data-source/ElementDataSourceGate.js'; // i18n utilities export { resolveKeyedI18nLabel } from './utils/i18n.js'; +// "Is this a real config bag?" — the ONE definition of that question +// (objectui#6761), exported at the package entry for the same reason the node-gate +// reporter above is (objectui#6038): a surface in `@object-ui/components`, which +// depends on this package, asks it too. `element:*` renderers there read their +// config out of `{ ...schema.props, ...schema.properties }`, the third channel of +// the degenerate-bag hazard objectui#6752 / objectui#6760 closed here +// (objectui#6783). A second copy one package over would be a spelling that +// `utils/configBag.pin.test.ts` — which scans `packages/react/src` — cannot see, +// which is the drift that pin exists to stop. +export { isConfigBag } from './utils/configBag.js'; + // Node-gate predicate diagnostics. Exported at the package entry (objectui#6038) // so every surface that evaluates a node `visibleWhen` reports a fault through // ONE reporter and ONE dedupe `Set` — `page:tabs` item predicates live in