fix(components): render a visible placeholder when a ui:icon glyph does not resolve

.changeset/5631-ui-icon-visible-placeholder.md

@@ -0,0 +1,25 @@
+---
+'@object-ui/components': minor
+---
+
+`ui:icon`: an unresolvable glyph now renders a visible placeholder instead of nothing
+
+An icon whose name does not resolve to a lucide glyph used to `return null`.
+That failed silently in two independent ways at once: invisible to a human (no
+gap, no error boundary — just an absent glyph), and clean-looking to a gate (a
+renderer that returns `null` spreads no attributes, so a DOM scan of it reports
+no findings).
+
+It now renders a dashed-square placeholder on the same SVG host, keeping the
+authored `className`, `size` and colour so the gap sits exactly where the icon
+would have been, with `role="img"`, an accessible name identifying the icon
+that failed, and a `data-objectui-icon-unresolved` marker. The `console.warn`
+stays and now names the cause.
+
+Also fixed: a node with no `name` at all reached `toPascalCase(undefined)` and
+threw, which the error boundary then swallowed — a third silent failure. It
+renders the placeholder too.
+
+Not included: `ui:icon` still reads the SDUI identity key `name` as its glyph
+name. Moving it to `schema.icon` is ruled but blocked on an authored-metadata
+migration — see objectui#5631.

packages/app-shell/src/__tests__/widget-dom-leak-sweep.test.tsx

@@ -149,7 +149,19 @@
  *
  * - 12 rendered no element at all: the overlays are closed until
  * `defaultOpen`, `action:*` return `null` with no actions, and `ui:icon`
- * returns `null` because the canary node's `name` is not a lucide icon.
+ * returned `null` because the canary node's `name` is not a lucide icon.
+ *
+ * ⚠️ `ui:icon` is the one of those twelve that has since been FIXED at the
+ * renderer rather than worked around here (objectui#5631). It used to need
+ * 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.
  * - 4 threw `useSidebar must be used within a SidebarProvider` and were
  * caught by `SchemaErrorBoundary`, whose markup is attribute-clean.
  *
@@ -649,7 +661,8 @@ const COMPONENTS_PLAIN_TYPES: readonly string[] = [
  'ui:date-picker', 'ui:dd', 'ui:del', 'ui:div', 'ui:dl', 'ui:dt', 'ui:em', 'ui:email',
  'ui:empty', 'ui:figcaption', 'ui:figure', 'ui:file-upload', 'ui:filter-builder', 'ui:flex',
  'ui:footer', 'ui:form', 'ui:grid', 'ui:h1', 'ui:h2', 'ui:h3', 'ui:h4', 'ui:h5', 'ui:h6',
- 'ui:header', 'ui:home', 'ui:hr', 'ui:html', 'ui:i', 'ui:image', 'ui:img', 'ui:input',
+ 'ui:header', 'ui:home', 'ui:hr', 'ui:html', 'ui:i', 'ui:icon', 'ui:image', 'ui:img',
+ 'ui:input',
  'ui:input-otp', 'ui:ins', 'ui:kbd', 'ui:label', 'ui:li', 'ui:list', 'ui:loading',
  'ui:main', 'ui:mark', 'ui:menubar', 'ui:nav', 'ui:navigation-menu', 'ui:ol', 'ui:p',
  'ui:page', 'ui:pagination', 'ui:password', 'ui:pre', 'ui:progress', 'ui:q',
@@ -673,11 +686,6 @@ const COMPONENTS_SPECIAL_TARGETS: readonly Target[] = [
  componentsTarget('ui:dropdown-menu', { ...OPEN_OVERLAY, items: [{ label: 'a', value: 'a' }] }),
  componentsTarget('ui:hover-card', OPEN_OVERLAY),
  componentsTarget('ui:tooltip', { trigger: CLEAN_SLOT, content: 'tip' }, '[data-state="closed"]'),
- // `IconRenderer` returns `null` when `schema.name` is not a lucide icon, and
- // the canary node's `name` is `canary_node` — so the default node rendered
- // NOTHING and read clean. (That collision is itself worth knowing: this
- // renderer reads the SDUI identity key `name` as an icon name.)
- componentsTarget('ui:icon', { name: 'check' }),
  // `action:*` return `null` with no actions (see CANARY_ACTIONS).
  componentsTarget('action:bar', { actions: CANARY_ACTIONS }),
  componentsTarget('action:group', { actions: CANARY_ACTIONS }),
@@ -977,6 +985,13 @@ const BARE_SPREAD_MINUS_NAME: readonly string[] = [
  * 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',

packages/components/src/renderers/basic/__tests__/icon-unresolvable-placeholder.test.tsx

@@ -0,0 +1,176 @@
+/**
+ * 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.
+ *
+ * `ui:icon` — an unresolvable glyph renders a VISIBLE placeholder, never
+ * nothing (objectui#5631, maintainer ruling 2026-08-22 comment 5380754137:
+ * "an unresolvable icon renders a visible placeholder instead of `null`,
+ * **regardless** of the key question").
+ *
+ * ## What this file pins, and what it deliberately does not
+ *
+ * It pins the END OF THE SILENCE — item 3 of the ruling, which is unconditional
+ * and independent of which schema key names the glyph. It does NOT pin
+ * `schema.icon` as the glyph key (item 1): that migration is blocked on a
+ * measured corpus, and the PR body carries the reading. So every case below
+ * still authors `name`, exactly as the renderer still reads it.
+ *
+ * ## Why the warning is asserted through an explicit spy
+ *
+ * Vitest 4 runs with `silent: 'passed-only'`, so `console.warn` output from a
+ * PASSING test is discarded — it never reaches stderr for a human to notice.
+ * Observing the warning by eye is therefore not available here, and a test
+ * that "checked" it by reading output would check nothing. `vi.spyOn` captures
+ * the call regardless of what the reporter prints. That property is not
+ * incidental: an unread `console.warn` is half of why objectui#5631 stayed
+ * invisible, so the warning is pinned as behaviour rather than trusted.
+ *
+ * ## Why the assertions are on the RENDERED RESULT, not on `!== null`
+ *
+ * The pre-fix branch returned `null`. A test asserting `container.firstChild`
+ * is non-null would pass against a placeholder that rendered an empty
+ * `<span/>` — invisible to a human and attribute-clean to the DOM-leak sweep,
+ * i.e. the same defect wearing a different shape. Each case below resolves
+ * something a person or a gate could actually see: a real `svg` host, the
+ * authored box (`className`/size) still applied to it, an accessible name, and
+ * the marker attribute.
+ */
+
+import { describe, it, expect, afterEach, vi } from 'vitest';
+import { render, screen, cleanup } from '@testing-library/react';
+import { SchemaRenderer } from '@object-ui/react';
+// Registers `ui:icon` at module scope, not in a hook
+// (object-ui/no-dynamic-import-in-test-hook, objectui#3010).
+import '../../../renderers';
+
+afterEach(() => {
+ cleanup();
+ vi.restoreAllMocks();
+});
+
+function renderIcon(schema: Record<string, unknown>) {
+ const warn = vi.spyOn(console, 'warn').mockImplementation(() => {});
+ const { container } = render(<SchemaRenderer schema={{ type: 'icon', ...schema } as never} />);
+ return { container, warn };
+}
+
+/** The marker the placeholder branch puts on its host, for gates and for this file. */
+const MARKER = '[data-objectui-icon-unresolved]';
+
+describe('ui:icon — unresolvable glyph', () => {
+ it('renders a visible SVG placeholder instead of nothing', () => {
+ const { container } = renderIcon({ name: 'definitely-not-a-lucide-icon' });
+
+ const placeholder = container.querySelector(MARKER);
+ expect(placeholder).not.toBeNull();
+ // The host is a real SVG, the same element kind a resolved glyph renders,
+ // so the gap occupies the layout slot the icon would have occupied.
+ expect(placeholder?.tagName.toLowerCase()).toBe('svg');
+ expect(placeholder?.getAttribute('data-objectui-icon-unresolved')).toBe(
+ 'definitely-not-a-lucide-icon',
+ );
+ });
+
+ it('names the unresolved icon in its accessible name', () => {
+ renderIcon({ name: 'definitely-not-a-lucide-icon' });
+
+ // `role="img"` + `aria-label`: the placeholder is perceivable, and it says
+ // WHICH icon failed rather than being an anonymous box.
+ expect(
+ screen.getByRole('img', { name: 'Unresolved icon: definitely-not-a-lucide-icon' }),
+ ).toBeTruthy();
+ });
+
+ it('warns, naming the identity-key collision that is objectui#5631', () => {
+ const { warn } = renderIcon({ name: 'save_icon' });
+
+ // Not a pinned call COUNT: React invokes the render function more than
+ // once here (measured: two calls for one `render`), and pinning the number
+ // would make this file fail on a StrictMode change rather than on the
+ // behaviour it is about. What matters is that the warning happens and what
+ // it says.
+ expect(warn).toHaveBeenCalled();
+ const message = String(warn.mock.calls[0]?.[0]);
+ expect(message).toContain('save_icon');
+ expect(message).toContain('objectui#5631');
+ // The warning must say why an ordinary authored identity lands here — the
+ // reader of this warning is an author who wrote `name: 'save_icon'` and is
+ // looking at a placeholder.
+ expect(message).toContain('identity key');
+ });
+
+ it('keeps the authored box: className and size still reach the placeholder', () => {
+ const { container } = renderIcon({
+ name: 'definitely-not-a-lucide-icon',
+ className: 'text-red-500',
+ size: 48,
+ });
+
+ const placeholder = container.querySelector(MARKER) as SVGElement | null;
+ expect(placeholder).not.toBeNull();
+ // Without this the placeholder would collapse to lucide's default box and
+ // the gap would not sit where the author put the icon.
+ expect(placeholder?.getAttribute('class')).toContain('text-red-500');
+ expect(placeholder?.getAttribute('style')).toContain('48px');
+ });
+
+ it('renders the placeholder — not a thrown error — when `name` is absent entirely', () => {
+ // Pre-fix this reached `toPascalCase(undefined)` and threw on
+ // `undefined.split`, which the SchemaErrorBoundary then swallowed: a THIRD
+ // way for this renderer to fail without saying so. `name` is typed
+ // `string` on `IconSchema`, but it arrives from authored JSON.
+ const { container, warn } = renderIcon({ id: 'no_name_node' });
+
+ const placeholder = container.querySelector(MARKER);
+ expect(placeholder).not.toBeNull();
+ expect(placeholder?.getAttribute('data-objectui-icon-unresolved')).toBe('(none)');
+ expect(String(warn.mock.calls[0]?.[0])).toContain('an absent icon name');
+ });
+});
+
+describe('ui:icon — resolvable glyph is untouched by objectui#5631', () => {
+ it('renders the real lucide glyph with no placeholder and no warning', () => {
+ const { container, warn } = renderIcon({ name: 'check', className: 'text-green-500' });
+
+ expect(container.querySelector(MARKER)).toBeNull();
+ const svg = container.querySelector('svg');
+ expect(svg).not.toBeNull();
+ expect(svg?.getAttribute('class')).toContain('text-green-500');
+ expect(warn).not.toHaveBeenCalled();
+ });
+
+ it('still resolves the kebab-case and renamed-icon paths', () => {
+ // `home` -> `Home` -> mapped to `House`: the `iconNameMap` hop, which the
+ // placeholder branch must not have short-circuited.
+ const { container, warn } = renderIcon({ name: 'home' });
+
+ expect(container.querySelector(MARKER)).toBeNull();
+ expect(container.querySelector('svg')).not.toBeNull();
+ expect(warn).not.toHaveBeenCalled();
+ });
+});
+
+describe('the placeholder glyph itself resolves (objectui#5622 mechanism)', () => {
+ it('is imported by name, so it cannot silently become another `null`', async () => {
+ // The failure this guards: lucide retires a spelling by dropping it from
+ // the runtime `icons` record while KEEPING the deprecated named export.
+ // A placeholder looked up in that record could therefore resolve to
+ // `undefined` and render nothing — objectui#5631 again, one level up.
+ // Measured on lucide-react 1.31.0: `CircleHelp` and `HelpCircle` are both
+ // ABSENT from the record while both resolve as named exports, so this is
+ // not a hypothetical hazard.
+ const lucide = await import('lucide-react');
+
+ expect(typeof (lucide as Record<string, unknown>).SquareDashed).not.toBe('undefined');
+ // The record-based lookup this file's renderer uses for AUTHORED names is
+ // exactly what the placeholder must not depend on. Pinned so that a future
+ // edit swapping the named import for `icons[...]` has to face this case.
+ expect(
+ Object.prototype.hasOwnProperty.call(lucide.icons, 'CircleHelp')
+ || Object.prototype.hasOwnProperty.call(lucide.icons, 'HelpCircle'),
+ ).toBe(false);
+ });
+});