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
16 changes: 16 additions & 0 deletions .changeset/skill-provider-envelope-teaching-5372.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
---
---

Published-skill teaching only — this publishes nothing, declared explicitly with an
empty frontmatter rather than left undeclared. No package `src/` is touched: the
change is confined to `skills/objectui/**` (the published skill package) plus one new
test under `packages/components/src/__tests__/`, which pins the corrected teaching to
the real renderer.

The rules told authors that the `properties` / `props` envelope belonged to the
`element:*` namespace and that every other key "lives on the node". Measured on a real
`SchemaRenderer` inside a `SchemaRendererProvider`, `properties` is evaluated and then
hoisted onto the node in *every* namespace — so it was the only spelling that reached a
`data-table`'s rows from a provider `dataSource`, while the node-level and `props`
spellings rendered a header over the empty state with nothing thrown and nothing logged.
The guides now record that measurement instead of contradicting it.
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,237 @@
/**
* 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#5372 — the published rules told authors the `properties` / `props`
* envelope belonged to the `element:*` namespace and nowhere else, and that
* every other key "lives on the node". Half of that is right and the half that
* is wrong is the half an author hits when wiring a provider to a table.
*
* Two envelopes, two fates (`packages/react/src/SchemaRenderer.tsx`, the
* `evaluatedSchema` memo):
*
* - `props.*` — evaluated, then spread as React props. A `ui:*` /
* `page:*` renderer reads `schema.*` and never sees it.
* - `properties.*` — evaluated, then HOISTED onto the node by the COMPAT
* hoist (`type` / `id` excepted). It therefore lands
* exactly where every renderer reads, in every namespace.
* - a node key — read, but never expression-evaluated.
*
* So the one spelling the rules told an author not to reach for was the only
* one that reaches a provider's data, and the two the rules endorsed both fail
* in this repo's most expensive shape: a correct header over an empty state,
* nothing thrown, nothing logged.
*
* ⛔ This file pins the MEASUREMENT and the corrected teaching. It takes no
* position on the two directions the card ruled out of scope — widening
* node-level evaluation, and giving `data-table` a `bind` read (declined by the
* objectui#5125 ruling). Whether `properties` should be an official `ui:*`
* authoring channel is objectui#4795's open question ②; the guides record it
* rather than recommend it, and nothing here asserts it should be taught.
*
* Three halves, same discipline as the sibling `skill-guide-data-table-binding`:
* a counter-probe so a zero is a reading, doc-sameness against the real
* published bytes, and behaviour through the REAL renderer.
*/

import { describe, it, expect } from 'vitest';
import { render, screen } from '@testing-library/react';
import React from 'react';
import fs from 'node:fs';
import path from 'node:path';
import { fileURLToPath } from 'node:url';
import { SchemaRenderer, SchemaRendererProvider } from '@object-ui/react';

// The REAL renderers, at module scope so `data-table` / `card` are registered
// before the first render (AGENTS.md §测试纪律). Relative, not the bare
// specifier: this file lives inside `@object-ui/components`
// (`scripts/check-package-self-import.mjs`).
import '../renderers';

const here = path.dirname(fileURLToPath(import.meta.url));
const repoRoot = path.resolve(here, '../../../..');
const skillsRoot = path.join(repoRoot, 'skills/objectui');

const COLUMNS = [
{ name: 'name', label: 'Name' },
{ name: 'email', label: 'Email' },
];
const ROWS = [
{ name: 'Ada Lovelace', email: 'ada@example.com' },
{ name: 'Grace Hopper', email: 'grace@example.com' },
];
const PROVIDER = { customers: ROWS, label: 'Evaluated Title' };
const EMPTY_STATE = 'No results foundTry adjusting your filters or search query.';

function renderNode(schema: unknown) {
return render(
<SchemaRendererProvider dataSource={PROVIDER}>
<SchemaRenderer schema={schema as never} />
</SchemaRendererProvider>,
);
}

/** Every rendered body cell's text, row-major. */
function bodyCells(): string[] {
return Array.from(document.querySelectorAll('tbody td')).map((td) => (td.textContent ?? '').trim());
}

/** Every published file in the skill package, by repo-relative path. */
function publishedFiles(): string[] {
const out: string[] = [];
const walk = (dir: string) => {
for (const entry of fs.readdirSync(dir, { withFileTypes: true })) {
const full = path.join(dir, entry.name);
if (entry.isDirectory()) walk(full);
else out.push(path.relative(repoRoot, full));
}
};
walk(skillsRoot);
return out.sort();
}

function readSkillFile(rel: string): string {
return fs.readFileSync(path.join(repoRoot, rel), 'utf8');
}

describe('#5372 behaviour — a provider `dataSource` into a `data-table`', () => {
it('a node-level `data` expression renders the empty state, silently', () => {
renderNode({ type: 'data-table', data: '${data.customers}', columns: COLUMNS });

// The raw `${…}` string is not an array, so DataTableRenderer falls back to
// EMPTY_ROWS. No throw and no console line is the whole defect.
expect(bodyCells()).toEqual([EMPTY_STATE]);
expect(screen.queryByText('Ada Lovelace')).not.toBeInTheDocument();
});

it('a `props` envelope renders the empty state — evaluated, then not read', () => {
renderNode({ type: 'data-table', props: { data: '${data.customers}' }, columns: COLUMNS });

expect(bodyCells()).toEqual([EMPTY_STATE]);
expect(screen.queryByText('Ada Lovelace')).not.toBeInTheDocument();
});

it('a `properties` envelope puts the provider rows on screen — evaluated, then hoisted', () => {
renderNode({ type: 'data-table', properties: { data: '${data.customers}' }, columns: COLUMNS });

expect(screen.queryByText('No results found')).not.toBeInTheDocument();
expect(bodyCells()).toEqual([
'Ada Lovelace',
'ada@example.com',
'Grace Hopper',
'grace@example.com',
]);
});

it('the route the guides teach — host-resolved rows on the node — renders', () => {
renderNode({ type: 'data-table', data: ROWS, columns: COLUMNS });

expect(bodyCells()).toEqual([
'Ada Lovelace',
'ada@example.com',
'Grace Hopper',
'grace@example.com',
]);
});
});

describe('#5372 behaviour — the hoist is not `element:*`-only', () => {
// `card` is a `ui:*` renderer reading `schema.title` (renderers/layout/card.tsx).
// If the envelope really were an `element:*` exception, the `properties` leg
// here would render no header. It renders one.
const header = () => (document.querySelector('[data-obj-type="card"]')?.textContent ?? '').trim();
const hasHeaderEl = () => !!document.querySelector('[data-obj-type="card"]')?.firstElementChild;

it('`props.title` on a `ui:*` card renders no header at all', () => {
renderNode({ type: 'card', props: { title: 'Customer Summary' } });
expect(hasHeaderEl()).toBe(false);
expect(header()).toBe('');
});

it('`properties.title` on the same card renders the header', () => {
renderNode({ type: 'card', properties: { title: 'Customer Summary' } });
expect(header()).toBe('Customer Summary');
});

it('a node-level `title` is read but never evaluated', () => {
renderNode({ type: 'card', title: '${data.label}' });
expect(header()).toBe('${data.label}');
});

it('the same expression under `properties` is evaluated AND read', () => {
renderNode({ type: 'card', properties: { title: '${data.label}' } });
expect(header()).toBe('Evaluated Title');
});
});

describe('#5372 doc-sameness — the published rules record what was measured', () => {
const PROTOCOL = 'skills/objectui/rules/protocol.md';

it('counter-probe: the rules file is readable and still carries the node rule', () => {
const md = readSkillFile(PROTOCOL);
// A known-present term. The assertions below are only readings because
// this one passes: a moved or renamed file would fail here first.
expect(md.length).toBeGreaterThan(0);
expect(md).toContain('Rule: Keys Live on the Node');
});

it('the retired `element:*`-only claim is gone from the rules', () => {
const md = readSkillFile(PROTOCOL);
// The exact sentence the measurement rules false, and the instruction that
// followed from it.
expect(md).not.toMatch(/The one exception is the `element:\*` namespace/);
expect(md).not.toMatch(/do not apply either shape everywhere/);
});

it('the rules state the hoist, which is the fact that makes the rest true', () => {
// Newline-tolerant: the claim is wrapped prose, not a fixed line.
expect(readSkillFile(PROTOCOL)).toMatch(/hoists every key onto\s+the node/);
});
});

describe('#5372 class guard — no published skill prescribes a dead envelope', () => {
it('counter-probe: the published tree is enumerable and non-empty', () => {
const files = publishedFiles();
expect(files.length).toBeGreaterThan(0);
expect(files).toContain('skills/objectui/rules/protocol.md');
});

it('no guide tells an author to move an expression "instead of" onto `props`', () => {
// The shape the prose carried in two places with none of the searchable
// envelope tokens: a Common-Mistakes bullet prescribing the envelope that
// renders nothing.
for (const rel of publishedFiles().filter((f) => f.endsWith('.md'))) {
expect(readSkillFile(rel), `${rel} prescribes the retired \`props.*\` workaround`)
.not.toMatch(/instead of\s+`props\./);
}
});

it('no published eval REQUIRES a `props.*` spelling in a correct answer', () => {
// The graded form of the same false rule. `must_contain` is the assertion
// that decides whether an answer passes, so a `props.` entry there marks
// the silently-blank spelling as correct.
const evals = publishedFiles().filter((f) => f.includes('/evals/') && f.endsWith('.json'));
expect(evals.length).toBeGreaterThan(0);

let graded = 0;
for (const rel of evals) {
const doc = JSON.parse(readSkillFile(rel)) as {
evals?: { assertions?: { must_contain?: string[] } }[];
};
for (const item of doc.evals ?? []) {
const must = item.assertions?.must_contain ?? [];
graded += must.length;
expect(must.filter((t) => /^props\./.test(t)), `${rel} requires a dead \`props.*\` spelling`)
.toEqual([]);
}
}
// Counter-probe: a per-entry loop over empty assertion lists passes
// vacuously.
expect(graded).toBeGreaterThan(0);
});
});
2 changes: 1 addition & 1 deletion skills/objectui/SKILL.md
Original file line numberDiff line numberDiff line change
Expand Up@@ -135,7 +135,7 @@ See `rules/no-touch-zones.md` for the full list and rationale.
- Introducing package coupling (e.g. a UI package depending on business logic).
- Registering components without a namespace in plugin-heavy projects.
- Skipping docs updates for newly introduced schema patterns.
- Putting expression values in top-level `value` / `label` fields instead of `props.*`.
- Expecting a `${...}` on a top-level `value` / `label` to evaluate, or "fixing" it by moving it under `props` — the first renders the literal, the second renders nothing at all. Resolve the value in the host, or carry it on a `text` node's `content` ([`rules/protocol.md`](./rules/protocol.md)).
- Missing the published stylesheet imports — `@object-ui/components/style.css` then `@object-ui/fields/style.css`, in that order — components render but look completely unstyled. The components sheet carries the theme tokens and the `:root` / `.dark` defaults; the fields sheet is a subtracted supplement that resolves against them.
- Pointing Tailwind at the installed ObjectUI packages instead of importing those two sheets: the published tarballs carry `dist` only, so the theme block the themed utilities are built on is not there to scan. Inside the ObjectUI workspace the reverse holds — packages are linked to their sources, an app scans them and declares the theme itself. See [`rules/styling.md`](./rules/styling.md) for both cases; do not keep a second copy of the answer here.

Expand Down
16 changes: 9 additions & 7 deletions skills/objectui/evals/schema-expressions.json
Original file line numberDiff line numberDiff line change
Expand Up@@ -4,32 +4,34 @@
{
"id": 1,
"prompt": "I have a statistic card in my Object UI schema but the value shows literally as ${data.revenue} instead of the actual number. Here's my schema: { \"type\": \"statistic\", \"value\": \"${data.revenue}\", \"label\": \"Revenue\" }. What's wrong?",
"expected_output": "Identifies that top-level 'value' and 'label' are not expression-evaluated, and provides the corrected schema using props.value and props.label.",
"expected_output": "Identifies that top-level 'value' and 'label' are not expression-evaluated, and does NOT offer props.value / props.label as the fix — that envelope is evaluated and then discarded, rendering nothing. Corrects it by resolving the value in the host before rendering, or by carrying it on a text node's content.",
"files": [],
"assertions": {
"must_contain": [
"props.value",
"props.label",
"content",
"${data."
],
"must_not_contain": [
"top-level value works"
"top-level value works",
"props.value",
"props.label"
]
}
},
{
"id": 2,
"prompt": "I need to make a button visible only to admin users AND only when the record status is 'active'. Also, the button should show the record owner's name. Show me the correct schema.",
"expected_output": "Produces a schema with combined visible/hidden expression using && operator, and uses props.label with template expression for the owner name.",
"expected_output": "Produces a schema with combined visible/hidden expression using && operator, and does not put the owner name in props.label — that envelope is evaluated and then discarded. The name is resolved in the host, or carried on a text node's content.",
"files": [],
"assertions": {
"must_contain": [
"visible",
"&&",
"props.label",
"${"
],
"must_not_contain": []
"must_not_contain": [
"props.label"
]
}
},
{
Expand Down
11 changes: 9 additions & 2 deletions skills/objectui/guides/data-integration.md
Original file line numberDiff line numberDiff line change
Expand Up@@ -200,7 +200,12 @@ completely — no error, no warning, nothing in the console.
`data-table` is not among them, which is the trap worth knowing by name: it
takes its rows from an inline `data` array on the node, so a bound `data-table`
renders a correct-looking header over an empty body, with nothing thrown and
nothing logged.
nothing logged. Pointing the node's own `data` key at an expression
(`"data": "${data.customers}"`) fails the same silent way — node keys are not
expression-evaluated — so **the host resolves the array and puts it on the
node**, as below. The one spelling that does carry a provider expression
through is measured, with its open-question caveat, in
[`../rules/protocol.md`](../rules/protocol.md).

```json
{
Expand DownExpand Up@@ -247,7 +252,9 @@ A `statistic`'s `label` / `value` / `description` are read off the node but are

Do not reach for a `props` envelope to get an expression evaluated — values
inside it are evaluated and then handed over as React props, which these
renderers never read, so the component paints an empty frame.
renderers never read, so the component paints an empty frame. (`properties` is a
different envelope and behaves differently; see
[`../rules/protocol.md`](../rules/protocol.md).)

### Via DataSource methods (in plugin code)

Expand Down
16 changes: 12 additions & 4 deletions skills/objectui/guides/page-builder.md
Original file line numberDiff line numberDiff line change
Expand Up@@ -89,9 +89,16 @@ renderers read `schema.title` / `schema.content` / `schema.columns` directly;
`SchemaRenderer` spreads `schema.props` as React props instead of merging it
into the node, so a key parked under `props` is never read and the component
paints an empty frame (the envelope itself also lands in the DOM as
`props="[object Object]"`). Namespaced `element:*` components are the one
deliberate exception — they read their config out of `properties` / `props`
by design (`readProps` in `packages/components/src/renderers/basic/elements.tsx`).
`props="[object Object]"`). Namespaced `element:*` components are where `props`
is read by design (`readProps` in
`packages/components/src/renderers/basic/elements.tsx`).

`properties` is a different envelope with a different fate: `SchemaRenderer`
evaluates it and then **hoists its keys onto the node**, so unlike `props` it
does reach every renderer. That is why it is the only spelling that gets a
provider expression into a `data-table` today — measured, with the failing legs
and the reason it is recorded rather than recommended, in
[`rules/protocol.md`](../rules/protocol.md).

Prefer expression-based behavior (`hidden`, `disabled`) over imperative
branching in component code.
Expand DownExpand Up@@ -180,6 +187,7 @@ clear the first gate; only the short list below clears the second.
| `visible` / `visibleOn` | Boolean expression. `visible` takes priority over `hidden`. |
| `disabled` / `disabledOn` | Boolean expression. Passed as prop to component. |
| `props.*` | Template-evaluated, but handed to the component as React props — a `ui:*` / `page:*` renderer never reads the result back, so the evaluated value is discarded. Only `element:*` components consume it. Do not use it as an expression carrier. |
| `properties.*` | Template-evaluated **and hoisted onto the node**, so unlike `props` the result is read — by every namespace. Its status as an authoring channel is open (objectui#4795); see [`rules/protocol.md`](../rules/protocol.md) before reaching for it. |

**NOT evaluated (raw strings passed through):**

Expand DownExpand Up@@ -478,7 +486,7 @@ It exposes `ObjectRenderer`, `PageRenderer`, `DashboardRenderer` and matching pr
- Introducing package coupling (for example, UI package depending on business logic package).
- Registering components without namespace in plugin-heavy projects.
- Skipping docs updates for newly introduced schema patterns.
- Putting expression values in top-level `value`/`label` fields instead of `props.*`.
- Expecting a `${...}` on top-level `value` / `label` to evaluate — it does not, and moving it under `props` renders nothing at all. Resolve it in the host, or carry it on a `text` node's `content`.
- Missing Shadcn CSS variables — components render but look completely unstyled.
- Forgetting the `@object-ui/components/style.css` and `@object-ui/fields/style.css` imports, or importing them in the wrong order — ObjectUI's utilities never reach the page.

Expand Down
Loading
Loading