Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 29 additions & 0 deletions .changeset/properties-wins-both-channels-5123.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
---
'@object-ui/react': patch
---

A node writing both `properties` and `props` now gets ONE answer per key, and it is the canonical `properties` one — on both read channels.

`properties` is the spec spelling of a node's config bag and `props` is the
annotated legacy alias, but which one actually reached the screen depended on
how the receiving renderer happened to read it — and the two channels disagreed
in opposite directions:

- **config bag** (`schema.properties.x`) — the `element:*` family's
`readProps()` merges `{ ...schema.props, ...schema.properties }`, so
`properties` won.
- **React prop** (`x` arriving as a prop) — `SchemaRenderer`'s `createElement`
spread the hoisted `properties.*` values first and then
`...(evaluatedSchema.props || {})` last, which overwrote them, so `props` won.

Measured on one render of one such node: the bag read `FROM_PROPERTIES` while
the same key read as a React prop gave `FROM_PROPS`. Which value rendered was
decided by nothing an author can see — only by whether their chosen component
belonged to the `readProps()` family.

The React-prop channel now declines to let the legacy alias override a key the
canonical bag also declares; the config-bag order was already correct and is
unchanged. Scope is co-occurrence only: a key that only `props` declares still
works exactly as before, and a node that writes one spelling is untouched. The
`props` alias is not retired here — only its precedence against a co-present
canonical spelling is settled.
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,144 @@
/**
* 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#5123 — one key, one answer, through REAL production renderers.
*
* The companion pin in
* `packages/react/src/__tests__/SchemaRenderer.aliasPrecedenceCrossChannel.test.tsx`
* states the invariant against a probe, because `@object-ui/react` by design
* does not depend on `@object-ui/components`. This file is the other half: the
* same node shape driven through the two renderer FAMILIES the defect actually
* split, with no stand-in for either read path.
*
* - `element:text` is the `readProps()` family — it reads its config out of
* the CONFIG BAG (`{ ...schema.props, ...schema.properties }`). The five
* members on `main` are `elements.tsx`, `text-input.tsx`,
* `record-picker.tsx`, `data-list.tsx`, `metadata-viewer.tsx`; a sixth,
* `plugin-detail/src/renderers/record-alert.tsx`, merges
* `{ ...schema, ...schema.properties }` — a different expression of the
* same `properties`-wins order.
* - `badge` is the SPREAD path — it takes what `createElement` hands it as
* React props (`{ schema, ...props }`) and puts it on the DOM. Everything
* outside the `readProps()` family reads this way.
*
* Before the fix these two families disagreed about the SAME key on the SAME
* node, in opposite directions: the bag read `properties`, the prop read
* `props`. Which one reached the screen depended only on which family the
* author's chosen component happened to belong to. Ruled 2026-08-18:
* `properties` wins on BOTH — the bag order was already right and is untouched.
*
* Requirement (2) of that ruling is why this file exists at all: pinning only
* the channel the fix edited would let the two drift apart again, which is the
* defect CLASS rather than the one spread order. So the assertions below are
* cross-family equalities, not per-file readings.
*/

import { describe, it, expect } from 'vitest';
import { render } from '@testing-library/react';
import { SchemaRenderer } from '@object-ui/react';
// Registers the renderers at module scope, NOT inside a `beforeAll` — there the
// cold transform is billed to `hookTimeout`. See
// object-ui/no-dynamic-import-in-test-hook (objectui#3010/#3021).
import '../renderers';

describe('alias precedence across the two renderer families (objectui#5123)', () => {
it('`element:text` (readProps family) renders the canonical `properties` value', () => {
// Unchanged by the fix — this channel was already correct and the ruling
// explicitly keeps it. Pinned so a future "unify the alias" change cannot
// satisfy the invariant by flipping THIS side instead, which would be the
// opposite of what was ruled.
const { container } = render(
<SchemaRenderer
schema={{
type: 'element:text',
className: 'xchan-text',
props: { content: 'FROM_PROPS' },
properties: { content: 'FROM_PROPERTIES' },
}}
/>
);

expect(container.querySelector('.xchan-text')?.textContent).toBe('FROM_PROPERTIES');
});

it('`badge` (spread path) receives the canonical `properties` value as its React prop', () => {
// The half the fix moved. `title` is an ordinary pass-through prop: the
// hoist puts `properties.title` on the node, `badge` spreads whatever it is
// handed onto the DOM. Before the fix the alias spread last and this read
// FROM_PROPS.
const { container } = render(
<SchemaRenderer
schema={{
type: 'badge',
label: 'b',
props: { title: 'FROM_PROPS' },
properties: { title: 'FROM_PROPERTIES' },
}}
/>
);

const badge = container.querySelector('[data-obj-type="badge"]');
expect(badge).toBeTruthy();
expect(badge?.getAttribute('title')).toBe('FROM_PROPERTIES');
});

it('the two families agree on one node — the invariant, stated as an equality', () => {
// The actual regression guard. Same key, same authored pair of bags, one
// node shape, read through both families; the assertion is that the two
// readings are EQUAL, so re-inverting either side fails here even if the
// per-family expectations above were edited to match a new reading.
const bags = {
props: { content: 'FROM_PROPS', title: 'FROM_PROPS' },
properties: { content: 'FROM_PROPERTIES', title: 'FROM_PROPERTIES' },
};

const bagChannel = render(
<SchemaRenderer
schema={{ type: 'element:text', className: 'xchan-both', ...bags }}
/>
);
const bagRead = bagChannel.container.querySelector('.xchan-both')?.textContent;

const propChannel = render(
<SchemaRenderer schema={{ type: 'badge', label: 'b', ...bags }} />
);
const reactPropRead = propChannel.container
.querySelector('[data-obj-type="badge"]')
?.getAttribute('title');

expect(bagRead).toBe(reactPropRead);
expect(bagRead).toBe('FROM_PROPERTIES');
});

it('a node carrying only the legacy `props` bag still works on both families', () => {
// The blast radius is co-occurrence only. `props` remains a working alias
// wherever `properties` does not also claim the key — the ruling narrowed
// precedence, it did not retire the alias (that question is objectui#4795's
// pending ②, and is NOT decided here).
const text = render(
<SchemaRenderer
schema={{
type: 'element:text',
className: 'xchan-legacy',
props: { content: 'ONLY_PROPS' },
}}
/>
);
expect(text.container.querySelector('.xchan-legacy')?.textContent).toBe('ONLY_PROPS');

const badge = render(
<SchemaRenderer
schema={{ type: 'badge', label: 'b', props: { title: 'ONLY_PROPS' } }}
/>
);
expect(
badge.container.querySelector('[data-obj-type="badge"]')?.getAttribute('title')
).toBe('ONLY_PROPS');
});
});
94 changes: 93 additions & 1 deletion packages/react/src/SchemaRenderer.tsx
Original file line numberDiff line numberDiff line change
Expand Up@@ -118,6 +118,93 @@ function resolveAriaProps(schema: Record<string, any>): Record<string, string |
return aria;
}

/**
* Keys the `properties` hoist deliberately refuses to copy onto the node — they
* identify WHICH renderer to dispatch to, so an inner `properties.type` (e.g. a
* tab's visual style `line` | `card` | `pill`) must never shadow the outer
* component descriptor. See the hoist in the evaluation memo below.
*/
const HOIST_PROTECTED_KEYS = new Set(['type', 'id']);

/**
* The legacy `props` bag, minus every key the canonical `properties` bag also
* declares (objectui#5123).
*
* A node may spell its config bag `properties` (the spec spelling) or `props`
* (the annotated legacy alias). A node that writes BOTH used to get a DIFFERENT
* answer for the same key depending on which channel read it, and the two
* channels' precedence was exactly OPPOSITE:
*
* - config bag: `readProps()` in the `element:*` family merges
* `{ ...schema.props, ...schema.properties }` — `properties` wins.
* - React prop: `createElement` below spread `...componentProps` (which
* carries the hoisted `properties.*` values) and then
* `...(evaluatedSchema.props || {})` LAST, which overwrote them —
* `props` won.
*
* Measured on one render of one node carrying both:
* `bagRead(properties.content)="FROM_PROPERTIES"` while
* `reactPropRead(content)="FROM_PROPS"`. Which one reached the screen depended
* only on whether the renderer happened to be in the `readProps()` family.
*
* Maintainer ruling 2026-08-18: **`properties` wins on BOTH channels — one
* answer per key.** The config-bag order was already correct and is untouched;
* this narrowing is the React-prop half. It restores the declared posture
* (`props` is an annotated legacy alias) rather than changing it.
*
* Implemented by REMOVING the overlapping keys from the alias spread rather
* than by re-spreading `properties` after it. That matters: `properties.*` is
* already on the node via the hoist, so the values are in `componentProps`
* having passed the metadata strip. Re-spreading the raw bag would push
* stripped schema metadata back out as React props — exactly the regression
* that made a spec-documented `dataSource` binding shadow the injected adapter
* and break the component (objectstack#5576). Subtracting adds no key to the
* outgoing bag; it only decides which of two co-present values a key carries.
*
* Scope, deliberately narrow — this only moves a reading where BOTH bags
* declare the same key:
* - a key only `props` declares is untouched (the alias keeps working);
* - a key only `properties` declares is untouched (it already won);
* - `type`/`id` are skipped, because the hoist never copied a canonical value
* up for them, so dropping the alias would DELETE the prop rather than
* replace it;
* - a degenerate (non-object) `properties` is left alone — the hoist and
* `readProps()` both merely object-spread it, and there is no canonical bag
* to prefer.
*
* Adjacent but NOT decided here: objectui#4795's pending question ② (whether
* the `properties` envelope is an official `ui:*` authoring channel at all).
* This is a precedence rule between two co-present spellings and takes no
* position on whether either should exist.
*/
function propsWithoutCanonicalKeys(
propsBag: Record<string, any> | undefined,
propertiesBag: unknown
): Record<string, any> {
if (!propsBag) return {};
// Only a real object bag can win a key. `typeof null === 'object'` is covered
// by the truthiness check; arrays are excluded for the same reason the
// evaluation guard excludes them — a degenerate `properties` must not have
// its shape reinterpreted here.
if (
!propertiesBag ||
typeof propertiesBag !== 'object' ||
Array.isArray(propertiesBag)
) {
return propsBag;
}
let narrowed: Record<string, any> | null = null;
for (const key of Object.keys(propertiesBag)) {
if (HOIST_PROTECTED_KEYS.has(key)) continue;
if (!Object.prototype.hasOwnProperty.call(propsBag, key)) continue;
// Copy lazily: the overwhelmingly common node declares one bag or neither,
// and this runs on every render of every node.
if (!narrowed) narrowed = { ...propsBag };
delete narrowed[key];
}
return narrowed ?? propsBag;
}

/**
* Per-component Error Boundary for SchemaRenderer.
* Catches render errors in individual components, preventing one broken
Expand DownExpand Up@@ -689,7 +776,12 @@ export const SchemaRenderer: ForwardRefExoticComponent<
{React.createElement(Component, {
schema: schemaForComponent,
...componentProps, // Spread non-metadata schema properties as props
...(evaluatedSchema.props || {}), // Override with explicit props if provided
// 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),
...ariaProps, // Inject ARIA attributes from AriaPropsSchema
...debugAttrs, // Debug-mode data attributes
disabled: __disabled || undefined,
Expand Down
Loading
Loading