Uh oh!
There was an error while loading. Please reload this page.
feat(types,components)!: ui:icon names its glyph with icon, not the identity key name - #6012
Merged
Merged
Conversation
…he identity key `name`
`name` is the SDUI identity key every authored node carries alongside `id`, not
`ui:icon`'s private prop. An ordinary `{ type:'icon', id:'save_icon',
name:'save_icon' }` therefore asked lucide for `SaveIcon`, missed, and rendered
nothing — silent to a human and clean to a DOM gate, because a renderer that
renders nothing spreads no attributes to find.
This is the chartered contract migration, not a renderer patch. `IconSchema`
and its zod mirror declare `icon: string` required, exactly as `name` was
required before it, and `name` reverts to the optional identity inherited from
`BaseSchema`. The mirror's previous `name: z.string()` requirement is what
blocked the renderer-only fix: the published contract refused the ruled shape
while the renderer read a key the contract never declared.
There is deliberately no `icon ?? name` fallback. The break is loud in three
places instead: the mirror refuses a legacy node with a message naming the
rename, the renderer draws the existing placeholder and warns with the exact
rename plus a `data-objectui-icon-legacy-name-key` marker, and
`migrateIconNodeKeys` converts stored metadata in bulk, reporting rather than
guessing on nodes carrying both keys or no glyph key at all.
All 98 authored icon nodes in the repo are converted, along with the registry
`inputs` entry and the component docs.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CSoz9uGhaaSgiq3hshtN7L…default pin `icon-renderer-declared-default.test.ts` selects the registry input that drives a dropped component's default by key. That key moved from `name` to `icon`, so the selector came back `undefined` and the pin lost its subject. Only the selector and the prose naming it moved; the file's subject is still objectui#5622's retired-spelling question, and its controls are untouched. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CSoz9uGhaaSgiq3hshtN7L
…lt types `check-doc-snippet-types.mjs` type-checks documentation code blocks, and the `migrateIconNodeKeys` example referenced two undeclared names. Declared them so the snippet is judged rather than excluded. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CSoz9uGhaaSgiq3hshtN7L
…icon-contract-migration
Contributor
✅ Console Performance Budget
The eager closure is every chunk the entry reaches through static imports — what the browser fetches and parses before the app renders. The entry chunk on its own is a small fraction of it. 📦 Bundle Size Report
Size Limits
|
yinlianghui
marked this pull request as ready for review
August 24, 2026 12:57
Uh oh!
There was an error while loading. Please reload this page.
This was referenced Aug 24, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes#5631
The chartered contract migration, per the maintainer ruling of 2026-08-24 — 「5631 A′,按一次正经的契约迁移立项。」 — which confirms the 2026-08-22 direction (A:
iconis the icon key,nameis identity always) at the price the escalation measured. All four ruled parts move in one stroke: contract, corpus, renderer, docs + registry.Final verification ran on
cc4d700a(this branch's head, after mergingorigin/main).The defect
nameis the SDUI identity key every authored node carries alongsideid— notui:icon's private prop. So an ordinary node like{ type: 'icon', id: 'save_icon', name: 'save_icon' }asked lucide forSaveIcon, missed, and rendered nothing: silent to a human, and clean to a DOM gate, because a renderer that renders nothing spreads no attributes to find.Zone 2 re-verified before building on it
Both prior measurements were re-run on this branch's base (post-#5959), with control probes.
The contract, measured against the built zod mirror — probe output, before the change:
Reproduced exactly. The last control is the sharpest:
icon: 42was refused at[name], i.e.iconwas not a declared key at all — the published contract required the broken shape and did not know the ruled one. That is why the renderer could not be migrated alone.After the change, same probe:
The last control matters twice over: it shows the migration message is scoped to the absent case only, so a genuine type error still gets zod's own precise diagnosis rather than a wrong diagnosis about a rename.
The corpus sweep re-run with a brace-matching scanner over all tracked
.ts/.tsx/.json/.mdx/.md:nameiconnameresolves to a live lucide glyph (mechanical rename)namedoes not resolve98 / 0 confirmed exactly, matching #5959's reading. (My scanner initially reported 101; the extra three are the
IconSchemadeclaration inlayout.ts, theinterfaceblock inicon.mdx, and one doc example incontent/docs/blocks/block-schema.mdx— declarations and prose, not authored nodes. The doc example is converted too.)The 3 non-resolving ones are all
icon: 'check-circle'inexamples/schema-catalog/src/schemas/marketing/call-to-action.json.check-circleis a retired lucide spelling (#5622 / #5586 family — the wrong name, not the wrong key). They rendered nothing before this PR and render the visible placeholder after it: behaviour unchanged, no regression, and deliberately not "fixed" here. Filed as #6009 — see below.So the ruling's stop-and-report clause did not fire: every node was mechanically convertible as a key rename, which is what this card is about.
What changed
1. Contract —
packages/types/src/layout.ts+zod/layout.zod.tsIconSchemadeclaresicon: stringrequired, exactly asnamewas required before it — a key rename at constant strictness, not a loosening.namereverts to the optional identity inherited fromBaseSchema.Keeping requiredness constant is also what keeps
__tests__/zod-mirror-parity.test.ts's ledger silent: an optional mirror key against a required declaration is drift that guard measures and would demand aKnownDriftentry for. The parity suite is green with no ledger edit.2. Corpus — 98 nodes converted by codemod across 22 files
Mechanical, offset-spliced, quoting style preserved. The codemod's own dry run reported
97 visited / 97 converted(the 96 non-test nodes + theblock-schema.mdxexample); the two nodes inbasic-renderers.test.tsxwere excluded from the sweep and triaged by hand — see below. Post-migration sweep: zero authoredname-as-glyph nodes remain anywhere; every residual hit is prose documenting the legacy shape or a deliberate legacy-shape fixture.3. Renderer —
renderers/basic/icon.tsxreadsschema.icon⛔ No
icon ?? namefallback.schema.nameis read at exactly one place — to make the migration diagnostic specific — and is never a glyph source: a node carrying it still renders the placeholder. PR #5959's placeholder behaviour is otherwise untouched.4. Docs + registry
The registry
inputsentry now declaresicon, and #5959's comment explaining why it deliberately still saidnameis retired, as the ruling directs.content/docs/components/basic/icon.mdxdocuments the key, the identity distinction, and the migration.The break is loud in three places, never silent
expected string, received undefined.ui:iconglyph does not resolve #5959's visible placeholder and itsconsole.warnnow carries the exact rename (icon: "save_icon") plus the converter's name; the accessible name says so too, and the host gainsdata-objectui-icon-legacy-name-keyso a gate can tell "unmigrated node" from "glyph that does not resolve".migrateIconNodeKeys(new,@object-ui/types) converts stored tenant metadata in bulk. Deployed metadata is unmeasured and the prior is high reliance, so the conversion is shipped rather than assumed.On that converter — it is a one-shot conversion, not a read-path fallback, and the distinction is the ruling's. Nothing calls it during rendering or parsing; it is zod-free, lives on the main entry beside the
dashboard-filter-alias.tsprecedent, and is run by a deployer over stored documents. It reports rather than guesses on the two cases it will not touch: a node already declaring both keys (iconwins,namestays the identity it is — overwriting would replace a working glyph with an identity string), and a node naming no glyph at all.Fixture triage — three dispositions, not one spelling sweep
icon-unresolvable-placeholder.test.tsxand both cases inbasic-renderers.test.tsx.describeblock pins the legacy shape as non-resolving, usingname: 'check'deliberately:checkis a real glyph, so a tolerant fallback would resolve it and the case would go green. A nonsense name could not tell a fallback from a miss.basic-renderers.test.tsxasserted only "somesvgrendered", which the placeholder satisfies too: an assertion that survives this migration by being unable to fail. Both cases now assert the absence of the placeholder marker.icon-renderer-declared-default.test.tsselects the registry input by key, so the rename left it selectingundefined. Selector updated; its Two more retired lucide spellings reach theicons-record resolver —editin DetailView's mobile Edit action,smileas theiconrenderer's own default — and only one of the four resolver copies is pinned #5622 subject and controls untouched.Reverse verification
Predicted direction: red, because the new declaration makes
iconrequired. Pasting the retired spelling into a consumer package (packages/components), against the rebuilt.d.ts:Run from a committed state, with a
trap … EXIT INT TERMrestoring the probe; the tree was verified byte-identical toHEADafterwards. This also proves the consumer read the rebuilt declaration rather than a cached one.Sequencing — the
BARE_SPREAD_ON_SVGrow is untouchedPer the fence, #5632's burn-down keeps excluding
ui:icon's ledger row and this PR does not "fix" it. It is unchanged at 14 attributes includingname, and the sweep is green with no ledger edit: the renderer still spreads the authored identity onto the SVG host, and that is #5632's to close. The newdata-objectui-icon-legacy-name-keymarker cannot perturb the row —data-is anOPEN_PREFIXESentry inpackages/test-support/src/dom-leak-judge.ts, so it is allowlisted by construction, and the sweep confirms it.Verification
All on
cc4d700a, after mergingorigin/main(which brought in #6003;packages/types/src/index.tsauto-merged cleanly).Repo gates re-derived from
package.json+.github/workflows/(this repo has nodispatch-gates.mjs) and re-run on the final head — each line is the gate's own verdict, not$?after a pipe:Earlier full runs on the pre-merge tree:
vitest run packages/components/— 183 files, 1681 tests (the one failure wasicon-renderer-declared-default.test.ts, fixed inf4bd018);vitest run examples/schema-catalog/ packages/react/… packages/layout/— 31 files, 1859 tests, green.Scope narrowing, declared: repo-wide
turbo run lintand the fullvitest runwere left to CI; local runs were filtered to the affected packages plus every suite the post-migration sweep proved could contain an icon node. The sweep is what bounds that narrowing — after conversion, zero icon nodes exist outsidepackages/types,packages/components,examples/schema-catalog,apps/siteand the docs, all of which were run.Out of scope, filed not fixed
#6009 —
ui:iconbecomes a record-reading resolver here, but its type is absent fromcheck-lucide-icon-record-names.mjs'sRECORD_READING_TYPEScensus, so 96 authored glyph names go unjudged. Measured from the gate's own--reportcounter, before and after the conversion on the same tree (reverted under a restoring trap, tree verified byte-identical toHEADafter):+96 declined,
judgedunmoved — exactly the 96 non-.mdxconverted nodes, which moved from "invisible to the gate" to "seen, and declined". The gate stays green either way. Adding the census entry would turn it red on the threecheck-circlenodes, which would drag #5622's retired-spelling repair into this PR — hence filed, unassigned, not folded in.Not folded in
icons-record resolver —editin DetailView's mobile Edit action,smileas theiconrenderer's own default — and only one of the four resolver copies is pinned #5622 /ViewSwitcherrenders NO icon forchartandganttviews: both icon names were dropped from lucide'siconsrecord #5586 — retired lucide spellings (wrong name). The threecheck-circlenodes are theirs; their behaviour is unchanged here.ui:iconbecoming a ninth record-reading site is an input to it.ui:buttoninlines its own copy of resolveIcon instead of importing it, so any future fix to the shared resolver silently misses it #5993 —ui:button's inlined copy ofresolveIcon.packages/components/src/renderers/**, grouped by mechanism #5632 — the SVG burn-down; itsui:iconrow is deliberately left as measured.This card was the wrong schema key. A fix for any one of those leaves the others intact.
Generated by Claude Code