diff --git a/.changeset/5632-svg-host-dom-passthrough.md b/.changeset/5632-svg-host-dom-passthrough.md new file mode 100644 index 0000000000..ec15b9639b --- /dev/null +++ b/.changeset/5632-svg-host-dom-passthrough.md @@ -0,0 +1,32 @@ +--- +'@object-ui/components': patch +--- + +`ui:icon` and `ui:spinner` route their host spread through `toDomProps` +(objectui#5632, the `BARE_SPREAD_ON_SVG` slice of objectui#5574). + +Both renderers forwarded their whole prop bag to the SVG they render, so every +authored SDUI key on the node became an attribute — 14 per target, and +`icon="check"` on all 71 icon nodes in the schema catalog. `IconSchema` and +`SpinnerSchema` declare only `icon` / `size` / `color`, and both renderers +already consume all three by name, so the SDUI pass-through list withholds +nothing they need. + +Two user-visible behaviours change, both of which were invisible to the DOM-leak +gate because the judge counts `stroke` / `width` / `height` as legitimate on an +SVG host: + +- **`ui:spinner` now spins.** Its computed `class` (`animate-spin` plus the size + class) was being overwritten by the `className` carried in the spread, so a + spinner rendered through `SchemaRenderer` had neither. It is merged now. +- **A sized `ui:spinner` no longer emits invalid dimensions.** `size` is an enum + (`sm`/`md`/`lg`/`xl`) and the spread handed the string to lucide's numeric + `size` prop, putting `width="lg" height="lg"` on the element. + +Also: an `icon` node's `color` is a Tailwind class (as `IconSchema.color` +declares, and as every authored value in the catalog uses). It reached lucide's +`color` prop through the spread as well, emitting an invalid +`stroke="text-red-500"` beside the class that does the real work; only the class +path remains. An authored raw CSS colour (e.g. `color: "red"`) no longer tints +the glyph through that accident — declare the colour as a class, which is the +declared contract. diff --git a/examples/schema-catalog/test/svg-host-dom-leak-5632.test.tsx b/examples/schema-catalog/test/svg-host-dom-leak-5632.test.tsx new file mode 100644 index 0000000000..eb4b4db607 --- /dev/null +++ b/examples/schema-catalog/test/svg-host-dom-leak-5632.test.tsx @@ -0,0 +1,336 @@ +/** + * 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. + */ + +/** + * Neither SVG-hosted renderer puts an authored schema prop on the DOM, and + * neither stopped putting a REAL one there (objectui#5632, the + * `BARE_SPREAD_ON_SVG` slice of objectui#5574). + * + * ## What this measures that the sweep gate cannot + * + * The same split the two probes next door describe + * (`layout-dom-leak-5574.test.tsx`, `form-control-dom-leak-5632.test.tsx`). + * `packages/app-shell/src/__tests__/widget-dom-leak-sweep.test.tsx` is the gate + * for this class and the stronger instrument for the OPEN TAIL — it plants + * canary keys no schema declares. What it does not plant are the renderers' OWN + * DECLARED PROPS: its canary node authors no `icon`, no `size`, no `color`. + * Adding them there would rewrite the measured attribute set of the renderers + * still ledgered, i.e. destroy the arrival reading that file preserves. + * + * For this group the declared half is the whole catalog-scale reading: + * `icon[icon]` is 71 of the 71 attributes measured before the fix — the glyph + * key the renderer CONSUMES to pick the component and then forwarded to the + * element as well. + * + * ## The half a leak gate cannot see, which on an SVG host is most of it + * + * A leak gate reports attributes that ARRIVE illegitimately. It has no case for + * one that STOPS arriving — and this is the group where that blind spot is + * widest, because `@object-ui/test-support`'s judge counts `stroke`, `width`, + * `height`, `fill` and `color` as legitimate on an SVG host. Everything lucide + * emits therefore sits in the judge's legitimate half, invisible in BOTH + * directions. + * + * That is not a hypothetical blind spot here. Two of this slice's three + * measured behaviour changes live inside it, and one of them was a live defect: + * + * - `spinner` declares `size` as an ENUM (`sm`/`md`/`lg`/`xl`) and consumes it + * through `sizeClasses`. The bare spread ALSO handed the string to lucide's + * numeric `size` prop, so a `size: 'lg'` node carried `width="lg" + * height="lg"` — invalid SVG dimensions, on every spinner in the catalog + * that sets a size, moving no number the gate watches. + * - `icon` declares `color` as a Tailwind CLASS ("Color Class") and applies it + * through `cn()`. The spread also reached lucide's `color` prop, emitting + * `stroke="text-red-500"` — an invalid paint value — beside the class doing + * the real work. All three `color` values authored in this catalog are + * classes, so nothing depended on the raw-CSS-colour accident. + * + * So the assertions below pin the LEGITIMATE attributes too, not just a zero. + * A zero leak reading plus an unpinned legitimate set is exactly how the + * sibling slice could have un-named and re-enabled every form control while its + * gate went green. + * + * ## Designing against the failure mode this test could have + * + * A zero means nothing on its own: a renderer that renders NOTHING spreads + * nothing and reads clean, and so does a walk that found no nodes. Three + * guards, the same three the sibling probe carries: + * + * 1. NODE COUNTS are asserted, per type, against the census below — read + * IDENTICAL in the before and after runs, so the after-zero is a reading + * and not a walk that stopped finding nodes. + * 2. `ui:grid` is measured in the SAME run as a CONTROL. Converged by + * objectui#4787 / PR #5573, it reads 0 in every configuration of this test, + * so on its own it discriminates nothing; what it does is read 0 in the + * same run in which `icon` read 71. + * 3. The JUDGE is self-checked for ELEMENT-AWARENESS, in the direction THIS + * file depends on: an SVG host and an HTML host must disagree about the + * same bag. A judge that allowed `color`/`width` everywhere — or one that + * lowercased SVG attribute names — would pass every assertion below while + * seeing none of this group. + * + * ## Why each node is rendered without its children + * + * `findLeaks` walks the whole subtree, so a node's nested SDUI children get + * their leaks attributed to the parent (measured next door: subtree scanning + * put 61 attributes on `grid`, the CONTROL, all from child nodes of other + * mechanism groups). Rendering each node with `children`/`body` removed scopes + * the reading to the renderer's own markup; nothing is lost, because the walk + * collects nested nodes of the measured types separately. + * + * Module-scope import of `@object-ui/components`, not `beforeAll` (AGENTS.md + * §测试纪律): registering the renderers is an unbounded module load and must not + * be billed to a bounded hook timeout. + */ +import { describe, it, expect } from 'vitest'; +import { render } from '@testing-library/react'; +import '@object-ui/components'; +import { SidebarProvider } from '@object-ui/components'; +import { SchemaRenderer } from '@object-ui/react'; +import { findLeaks, leakReport } from '@object-ui/test-support'; +import { allExamples } from '../src/index.js'; + +type Node = Record; + +/** Both members of the group, plus `grid` as the control (guard 2 above). */ +const MEASURED_TYPES = ['icon', 'spinner', 'grid'] as const; + +/** + * Nodes of each type in the catalog today, and how many render no element at + * all. Asserted, so a zero leak reading always comes with proof that something + * was rendered to read. + * + * These move when the CATALOG is authored, not when a renderer changes. A diff + * that changes them and nothing else is an example being added; update them. A + * diff that changes them while touching a renderer is what this guard is for — + * `noElement` in particular, because `ui:icon` has a branch that used to + * `return null` (objectui#5631) and a renderer made to bail early reads CLEAN + * below having earned nothing. + */ +const NODE_CENSUS: Readonly> = { + icon: { rendered: 71, noElement: 0 }, + spinner: { rendered: 6, noElement: 0 }, + grid: { rendered: 26, noElement: 0 }, +}; + +function collect(node: unknown, out: Node[] = []): Node[] { + if (Array.isArray(node)) { + for (const item of node) collect(item, out); + return out; + } + if (node && typeof node === 'object') { + const record = node as Node; + if ( + typeof record.type === 'string' && + (MEASURED_TYPES as readonly string[]).includes(record.type) + ) { + out.push(record); + } + for (const value of Object.values(record)) collect(value, out); + } + return out; +} + +/** Every attribute on the host element of a standalone node, `name="value"`, sorted. */ +function attributesOf(schema: Record): string[] { + const { container, unmount } = render( +
+ +
, + ); + const host = container.querySelector('[data-probe-root]')?.firstElementChild; + expect(host, `no element rendered for ${JSON.stringify(schema)}`).toBeTruthy(); + const attributes = Array.from(host!.attributes) + .map((attribute) => `${attribute.name}="${attribute.value}"`) + .sort(); + unmount(); + return attributes; +} + +describe('schema-catalog — no SVG-hosted renderer leaks an authored prop to the DOM (#5632)', () => { + it('the judge is element-aware — a zero below is a reading, not a blind spot', () => { + // Rendered directly rather than through the registry: this checks the + // JUDGE, and it must keep working even if every renderer in the repo is + // fixed. Without it, `findLeaks` could return `[]` unconditionally and + // every assertion in this file would still pass. + // + // The bag is spread rather than written as JSX attributes, which is not a + // typing dodge but the defect's own shape: `` does not + // type-check, and a bare spread of an untyped record is exactly how these + // attributes reached real elements without anyone hearing about it. + const bag: Record = { + className: 'c', + id: 'i', + 'data-obj-id': 'd', + color: 'red', + width: '24', + label: 'L', + zzcanary: 'S', + }; + + // On an SVG host `color` and `width` are real attributes, so only the two + // undeclared keys are leaks. This is the half objectui#5632 depends on: it + // is why dropping them from the spread is invisible to a leak gate, and + // therefore why this file pins the legitimate set below as well. + const svg = render(); + expect( + findLeaks(svg.container.firstElementChild!) + .map((leak) => leak.attribute) + .sort(), + ).toEqual(['label', 'zzcanary']); + + // On an HTML container the SAME bag leaks `color` and `width` too — the + // judge answers per element rather than from one flat list. A judge that + // did not would report the same set for both and this file would be + // vacuous exactly where the group lives. + const div = render(
); + expect( + findLeaks(div.container.firstElementChild!) + .map((leak) => leak.attribute) + .sort(), + ).toEqual(['color', 'label', 'width', 'zzcanary']); + }); + + it('every catalog node of these three types renders, and none leaks', () => { + const leaks: string[] = []; + const rendered = new Map(); + const noElement = new Map(); + const bump = (map: Map, key: string) => + map.set(key, (map.get(key) ?? 0) + 1); + + for (const example of allExamples()) { + for (const node of collect(example.schema)) { + const type = String(node.type); + bump(rendered, type); + // Children removed — see "Why each node is rendered without its + // children" above. `SidebarProvider` keeps the harness identical to the + // sibling probe's, so the before/after readings are comparable. + const { children: _children, body: _body, ...own } = node; + const { container, unmount } = render( + +
+ +
+ , + ); + const host = container.querySelector('[data-probe-root]')?.firstElementChild; + if (!host) { + bump(noElement, type); + unmount(); + continue; + } + const found = findLeaks(host); + if (found.length > 0) leaks.push(`${example.id} :: ${leakReport(type, found)}`); + unmount(); + } + } + + // Asserted BEFORE the leak reading, so a walk that rendered nothing fails + // as a broken instrument rather than passing as a clean tree. + const census = Object.fromEntries( + MEASURED_TYPES.map((type) => [ + type, + { rendered: rendered.get(type) ?? 0, noElement: noElement.get(type) ?? 0 }, + ]), + ); + expect( + census, + 'the catalog node census moved. If this diff only adds/removes examples, ' + + 'update NODE_CENSUS. If it touches a renderer, a node stopped rendering ' + + 'an element — and a renderer that renders nothing reads CLEAN below.', + ).toEqual(NODE_CENSUS); + + expect( + leaks, + 'an authored schema prop reached the DOM as an attribute. Route the spread ' + + 'through `toDomProps` (`@object-ui/core`) — never widen that list to ' + + 'reach one host and never add an exemption here. A key that genuinely ' + + 'belongs on this element is DECLARED and forwarded BY NAME ' + + '(objectui#4435), the way `style` is.', + ).toEqual([]); + }, 120000); + + /** + * The other direction, which no leak gate can assert: the attributes that + * SHOULD be on these elements still are. Full sets, not spot checks — a + * subset assertion would not have caught `stroke="text-red-500"` arriving, + * and will not catch `stroke` disappearing. + */ + it('the legitimate SVG attributes still arrive, and the declared paths still work', () => { + // `color` is a Tailwind CLASS, which is what `IconSchema.color` declares. + // It belongs in `class` and NOWHERE else: before this slice it also reached + // lucide's `color` prop and came out as `stroke="text-blue-500"`, an invalid + // paint value the judge counts as legitimate. + expect(attributesOf({ type: 'icon', icon: 'check', color: 'text-blue-500' })).toEqual([ + 'aria-hidden="true"', + 'class="lucide lucide-check text-blue-500"', + 'data-obj-type="icon"', + 'fill="none"', + 'height="24"', + 'stroke-linecap="round"', + 'stroke-linejoin="round"', + 'stroke-width="2"', + 'stroke="currentColor"', + 'viewBox="0 0 24 24"', + 'width="24"', + 'xmlns="http://www.w3.org/2000/svg"', + ]); + + // `IconSchema.size` is declared in PIXELS and this renderer consumes it into + // an inline `style`. That style is the channel the authored size renders + // through — CSS `width`/`height` win over the SVG presentation attributes — + // and it is untouched here; only the redundant second channel through + // lucide's `size` prop goes. + expect(attributesOf({ type: 'icon', icon: 'check', size: 48 })).toEqual([ + 'aria-hidden="true"', + 'class="lucide lucide-check"', + 'data-obj-type="icon"', + 'fill="none"', + 'height="24"', + 'stroke-linecap="round"', + 'stroke-linejoin="round"', + 'stroke-width="2"', + 'stroke="currentColor"', + 'style="width: 48px; height: 48px;"', + 'viewBox="0 0 24 24"', + 'width="24"', + 'xmlns="http://www.w3.org/2000/svg"', + ]); + + // The unresolved-glyph branch of objectui#5631 spreads too, and its + // `role`/`aria-label`/`data-*` must survive the filter — `role` is on the + // SDUI pass-through list and the other two are open families. + const unresolved = attributesOf({ type: 'icon', icon: 'no-such-glyph-xyz' }); + expect(unresolved).toContain('role="img"'); + expect(unresolved).toContain('aria-label="Unresolved icon: no-such-glyph-xyz"'); + expect(unresolved).toContain('data-objectui-icon-unresolved="no-such-glyph-xyz"'); + expect(unresolved.filter((attribute) => attribute.startsWith('icon='))).toEqual([]); + + // `SpinnerSchema.size` is an ENUM consumed through `sizeClasses`. Spreading + // it handed the string to lucide's numeric `size` prop, which is how + // `width="lg" height="lg"` reached every sized spinner in the catalog. + // `class` carries lucide's own two classes AND both of this renderer's — + // `animate-spin` and the size class. Before this slice it carried ONLY + // lucide's: the spread's `className` overrode the computed one, so a + // `ui:spinner` rendered through `SchemaRenderer` did not spin. + expect(attributesOf({ type: 'spinner', size: 'lg' })).toEqual([ + 'aria-hidden="true"', + 'class="lucide lucide-loader-circle animate-spin h-8 w-8"', + 'data-obj-type="spinner"', + 'fill="none"', + 'height="24"', + 'stroke-linecap="round"', + 'stroke-linejoin="round"', + 'stroke-width="2"', + 'stroke="currentColor"', + 'viewBox="0 0 24 24"', + 'width="24"', + 'xmlns="http://www.w3.org/2000/svg"', + ]); + }); +}); diff --git a/packages/app-shell/src/__tests__/widget-dom-leak-sweep.test.tsx b/packages/app-shell/src/__tests__/widget-dom-leak-sweep.test.tsx index b66c0138bb..8615fb403a 100644 --- a/packages/app-shell/src/__tests__/widget-dom-leak-sweep.test.tsx +++ b/packages/app-shell/src/__tests__/widget-dom-leak-sweep.test.tsx @@ -37,9 +37,9 @@ * | plugin-calendar | 3 | 0 | 0 | * | plugin-chatbot | 3 | 0 | 0 | * | plugin-dashboard | 8 | 2 | 7 / 9 | - * | components | 158 | 97 | 12 .. 15 | + * | components | 158 | 95 | 12 .. 15 | * - * **99 of 181 targets leak.** The `components` row is objectui#5574 and is + * **97 of 181 targets leak.** The `components` row is objectui#5574 and is * covered in its own section below; the two `plugin-dashboard` rows are the * older tail. Both are in {@link LEAK_LEDGER}: * `plugin-dashboard:metric` and `plugin-dashboard:metric-card`, the open tail @@ -156,12 +156,14 @@ * a forced `schemaExtras: { name: 'check' }` to render at all; it now * renders a visible placeholder for an unresolvable glyph, so it is swept * as an ordinary plain target on the node this file actually authors — - * identity `name: 'canary_node'` and nothing else. Its - * {@link BARE_SPREAD_ON_SVG} row was re-measured on that node as the - * ruling required and is UNCHANGED: the placeholder is the same bare - * spread onto the same SVG host, so it leaks the same fourteen. That the - * row did not move is the point — the reading no longer depends on a - * workaround that hid whether the renderer rendered. + * identity `name: 'canary_node'` and nothing else. Its `BARE_SPREAD_ON_SVG` + * row was re-measured on that node as the ruling required and was + * UNCHANGED: the placeholder is the same bare spread onto the same SVG + * host, so it leaked the same fourteen. That the row did not move is the + * point — the reading no longer depended on a workaround that hid whether + * the renderer rendered. That row is now GONE, burned by objectui#5632 + * (see the burn-down record below); this note is kept because the phantom + * it describes is a property of the SWEEP, not of the row. * - 4 threw `useSidebar must be used within a SidebarProvider` and were * caught by `SchemaErrorBoundary`, whose markup is attribute-clean. * @@ -250,8 +252,10 @@ * silences the warning while keeping the leak, Vitest discards console output * from passing tests, and the warning latches per prop name so a shared canary * consumes it. This family makes the point concrete: of the 14 attributes in - * the commonest shape, the ones React would warn about are a minority, and - * {@link BARE_SPREAD_ON_SVG} is a group React reports differently again. The + * the commonest shape, the ones React would warn about are a minority, and the + * SVG-hosted group objectui#5632 burned was one React reported differently + * again — camelCase survives on an SVG host, so the spelling React sees and the + * spelling that lands are the same and no warning distinguishes them. The * technique that closes the class reads the DOM, which is what this file does. * * ## Why this file lives in `packages/app-shell` @@ -909,8 +913,8 @@ interface LedgerEntry { /* ── objectui#5574: the `packages/components` reading, as a LEDGER ─────────── */ /** - * 97 of the 158 `packages/components` targets leak, and they do it in exactly - * SIX shapes (119 targets in seven shapes did on arrival; see the burn-down + * 95 of the 158 `packages/components` targets leak, and they do it in exactly + * FIVE shapes (119 targets in seven shapes did on arrival; see the burn-down * note below — and note the arrival count of shapes read `eight` here until * objectui#5632 counted them: the groups were seven, and the card's own table * listed seven). Writing that @@ -959,6 +963,28 @@ interface LedgerEntry { * reads 0 after, with `grid`'s 26 nodes at 0 both times as the control and * an unchanged per-type node census across the two runs. * + * - objectui#5632 (the slice after that one) — the entire + * `BARE_SPREAD_ON_SVG` shape, both members: `ui:icon` and `ui:spinner`. + * Converged on the bare `toDomProps`, which is the answer the form-control + * group above could NOT take: `IconSchema` and `SpinnerSchema` declare only + * `icon` / `size` / `color`, and both renderers already consume all three by + * name, so nothing legitimate was withheld and no third declaration was + * warranted. The group is DELETED, so the grouping is five mechanisms now. + * + * ⚠️ What this file could not have graded, recorded here because the next + * slice needs the warning: on an SVG host the judge counts `stroke`, + * `width`, `height`, `fill` and `color` as LEGITIMATE, so everything lucide + * emits sits in the half this gate never reports, in either direction. Two + * real behaviours lived in it. `ui:spinner` declares `size` as an ENUM and + * the spread handed the string to lucide's numeric `size` prop, so every + * sized spinner carried `width="lg" height="lg"`; and `ui:icon` declares + * `color` as a Tailwind CLASS, which the spread also fed to lucide's + * `color` prop, emitting `stroke="text-red-500"`. Both are gone, and + * neither moved a number here. They are measured and pinned in + * `examples/schema-catalog/test/svg-host-dom-leak-5632.test.tsx`, which + * asserts the LEGITIMATE attribute sets too — the guard this gate + * structurally cannot provide. + * * ## This is a ledger, not an allowlist — the difference, stated once * * An allowlist says "do not look here". Every row below says "we looked, this @@ -1012,26 +1038,6 @@ const BARE_SPREAD_MINUS_NAME: readonly string[] = [ 'reference_to', 'zzcanary', 'zzcanarycamel', 'zzcanarynum', 'zzcanaryobj', 'zzcanaryprop', ]; -/** - * The same bare spread onto an SVG host — and the one group whose attribute - * names are NOT lowercased. SVG attribute names are case-sensitive, so the - * camelCase canaries survive exactly as authored (`ariaLabel`, not - * `arialabel`). A ledger keyed on the lowercased spelling would have silently - * failed to match these two. - * - * `ui:icon`'s membership here was re-measured under objectui#5631, on the - * ordinary canary node rather than the forced-resolvable one the old entry - * needed, and came back identical — see the `ui:icon` note in this file's - * "four phantom cleans" section. `name` stays in this list: the renderer still - * spreads the authored identity onto the SVG, and closing that is the - * objectui#5632 burn-down, deliberately NOT folded in here. - */ -const BARE_SPREAD_ON_SVG: readonly string[] = [ - 'ariaDescribedBy', 'ariaLabel', 'bind', 'colorVariant', 'dataSource', 'events', 'name', - 'props', 'reference_to', 'zzcanary', 'zzcanaryCamel', 'zzcanarynum', 'zzcanaryobj', - 'zzcanaryprop', -]; - interface LedgerGroup { readonly attributes: readonly string[]; readonly reason: string; @@ -1067,16 +1073,6 @@ const COMPONENTS_LEAK_GROUPS: readonly LedgerGroup[] = [ 'ui:time', 'ui:toggle-group', 'ui:tree-view', 'ui:u', 'ui:ul', 'ui:utility', ], }, - { - attributes: BARE_SPREAD_ON_SVG, - reason: - 'the same bare spread onto an SVG host, where attribute names are ' + - 'case-sensitive — so the camelCase canaries survive as authored.', - issue: 'objectui#5574', - targets: [ - 'ui:icon', 'ui:spinner', - ], - }, { attributes: [...BARE_SPREAD, 'actions'].sort(), reason: @@ -1149,7 +1145,7 @@ const LEAK_LEDGER: Readonly> = { issue: 'objectui#4425', }, - /* ── packages/components: 115 of 158 targets, in eight measured shapes ──── */ + /* ── packages/components: 95 of 158 targets, in five measured shapes ───── */ ...Object.fromEntries( COMPONENTS_LEAK_GROUPS.flatMap((group) => group.targets.map((type) => [ diff --git a/packages/components/src/renderers/basic/icon.tsx b/packages/components/src/renderers/basic/icon.tsx index 978acac41f..d9b8ad55f4 100644 --- a/packages/components/src/renderers/basic/icon.tsx +++ b/packages/components/src/renderers/basic/icon.tsx @@ -6,7 +6,7 @@ * LICENSE file in the root directory of this source tree. */ -import { ComponentRegistry } from '@object-ui/core'; +import { ComponentRegistry, toDomProps } from '@object-ui/core'; import type { IconSchema } from '@object-ui/types'; import { SquareDashed } from 'lucide-react'; import React, { forwardRef } from 'react'; @@ -56,6 +56,48 @@ import { describeIconLookup, resolveIcon } from '../action/resolve-icon'; * as a second component declaration in a file that exports none. */ +/** + * The DOM pass-through for this file's SVG host (objectui#5632, the + * `BARE_SPREAD_ON_SVG` slice of objectui#5574). + * + * ## Why the BARE `toDomProps`, and not a declaration of its own + * + * The sibling slice for form controls needed one + * (`../../lib/form-control-dom-props.ts`): `name` and `disabled` are legal + * HTML on a control, so the element-agnostic SDUI list would have stripped + * real attributes. The question has to be asked again per host, and for an + * SVG icon the answer is the opposite one — MEASURED, not assumed: + * `IconSchema` declares exactly `icon`, `size` and `color`, and this renderer + * already CONSUMES all three by name (the glyph lookup, `sizeStyle`, and the + * `cn()` class list). Not one of them needs to reach the element through a + * spread, so nothing legitimate is withheld and no third declaration is + * warranted. + * + * ## What stops arriving, which is the half a leak gate cannot see + * + * The sweep gate reports attributes that arrive ILLEGITIMATELY. It has no case + * for one that stops arriving, and on an SVG host the judge counts + * `stroke` / `width` / `height` as legitimate — so all three changes below were + * invisible to it in both directions and had to be read off the DOM directly: + * + * - `color` reached lucide's own `color` prop and became `stroke`. Every + * authored `color` in the catalog is a Tailwind CLASS, which is what + * `IconSchema.color` declares ("Color Class") and what the `cn()` list + * already applies — so the spread was emitting `stroke="text-red-500"`, an + * invalid SVG paint value, next to the class that does the real work. + * Dropping it removes the garbage and leaves the declared path untouched. + * - `size` reached lucide's `size` prop and set the `width`/`height` + * ATTRIBUTES, duplicating the `sizeStyle` this renderer already writes. CSS + * `width`/`height` win over the SVG presentation attributes, so the authored + * size still renders from the style; only the redundant second channel goes. + * - `icon` itself landed on the element as `icon="check"` — 71 times in + * `examples/schema-catalog`, and the one leak this file's catalog probe + * measures. + * + * A key that genuinely belongs on this element is DECLARED and forwarded BY + * NAME (objectui#4435), the way `style` is below. ⛔ Never reopen the spread + * and never widen the shared list to reach one host (AGENTS.md #0.1). + */ // Index signature on the parameter annotation, not on the `forwardRef` type // argument — mechanism note on `action:bar` (objectui#4422), pinned by // `__tests__/forwardref-props-annotation.guard.test.ts`. @@ -156,7 +198,7 @@ const IconRenderer = forwardRef diff --git a/packages/components/src/renderers/feedback/spinner.tsx b/packages/components/src/renderers/feedback/spinner.tsx index 2ffbfbf8b9..3daa4877b0 100644 --- a/packages/components/src/renderers/feedback/spinner.tsx +++ b/packages/components/src/renderers/feedback/spinner.tsx @@ -6,13 +6,43 @@ * LICENSE file in the root directory of this source tree. */ -import { ComponentRegistry } from '@object-ui/core'; +import { ComponentRegistry, toDomProps } from '@object-ui/core'; import type { SpinnerSchema } from '@object-ui/types'; import { Loader2 } from 'lucide-react'; import { cn } from '../../lib/utils'; +/** + * `ui:spinner`'s host is an SVG (lucide's `Loader2`), so its DOM pass-through is + * the bare `toDomProps` — objectui#5632, the `BARE_SPREAD_ON_SVG` slice of + * objectui#5574. `SpinnerSchema` declares exactly one key beyond the SDUI base, + * `size`, and this renderer already CONSUMES it by name through `sizeClasses`; + * nothing it declares needs to reach the element through a spread. + * + * ⚠️ The bare spread was not merely noisy here, and the defect it carried is one + * the sweep gate could never report: `size` is an ENUM (`sm`/`md`/`lg`/`xl`), + * and spreading it handed the string to lucide's numeric `size` prop, which put + * `width="lg" height="lg"` — invalid SVG dimensions — on the element. The judge + * counts `width`/`height` as legitimate on an SVG host, so that reading sat + * inside the LEGITIMATE half of the measurement and moved no number the gate + * watches, in either direction. It is measured and pinned in + * `examples/schema-catalog/test/svg-host-dom-leak-5632.test.tsx`. + * + * ## The `className` this renderer was dropping + * + * `className` is on the SDUI pass-through list, so filtering the spread does + * NOT stop it clobbering — it stayed inside `props` here and overrode the + * computed class that follows it, which is why a `ui:spinner` rendered through + * `SchemaRenderer` carried `class="lucide lucide-loader-circle"` and neither + * `animate-spin` nor its size class: measured on this file's pre-slice tree, + * i.e. the spinner did not spin. It is destructured and MERGED here, the way + * the sibling `basic/icon.tsx` in this same group already did it and the way + * the migration's worked example `layout/grid.tsx` (objectui#4787 / PR #5573) + * does by ordering. Same bare spread, same line, the other half of its harm: + * routing the spread without this leaves the filter forwarding a key that + * destroys a legitimate computed value. + */ ComponentRegistry.register('spinner', - ({ schema, ...props }: { schema: SpinnerSchema; [key: string]: any }) => { + ({ schema, className, ...props }: { schema: SpinnerSchema; className?: string; [key: string]: any }) => { const { 'data-obj-id': dataObjId, 'data-obj-type': dataObjType, @@ -29,8 +59,8 @@ ComponentRegistry.register('spinner', return ( );