diff --git a/.changeset/9414-lazy-icon-letter-digit-boundary.md b/.changeset/9414-lazy-icon-letter-digit-boundary.md new file mode 100644 index 0000000000..de6832999c --- /dev/null +++ b/.changeset/9414-lazy-icon-letter-digit-boundary.md @@ -0,0 +1,41 @@ +--- +'@object-ui/components': patch +--- + +Teach the icon seam's tokeniser the `letter -> digit` boundary (objectui#9414). + +`toKebabIconName` split `lower-or-digit -> Upper` and `acronym-run -> Word`, and never +`letter -> digit`. So `Building2` — the PascalCase component `lucide-react` exports, the +spelling lucide's own site shows an author, and the spelling an author copies — tokenised +to `building2` while lucide's canonical key is `building-2`. The name matched nothing and +`getLazyIcon` degraded it to the `Database` glyph with **no error, no warning and no log**: +the author saw *an* icon and had no signal that it was not theirs. Every digit-bearing +canonical name was affected — `BarChart3`, `CheckCircle2`, `ArrowDown01`, `Axis3D`, +`Grid2x2` and the rest. + +One rule closes it, and it is deliberately not unconditional. A negative lookbehind holds +the split off when the letter is itself preceded by a digit, because that is lucide's grid +spelling: `Grid2x2` is the Pascal form of `grid-2x2`, where the `x` sits *inside* a segment +rather than starting one. That is what lets the same single rule land on lucide's primary +`grid-2x2` instead of its `grid-2-x-2` alias, and it is why `Grid3x2` — for which lucide +ships no `-3-x-2` alias at all — is reached too. + +**Nothing that resolved before resolves differently.** The change is a strict superset: +every name the previous tokeniser accepted still tokenises to the byte-identical result, +and every canonical kebab spelling is still returned untouched. Both legs, and the +capability itself, are re-derived from the installed `lucide-react` on every run by +`packages/components/src/__tests__/lazy-icon-digit-boundary-9414.test.ts` rather than +written down here — the property pinned there is that **every** canonical icon name is +reachable from its own exported PascalCase spelling. + +**Correction to an earlier migration note.** The changeset for objectui#7472 lists +"digit-suffixed spellings such as `Building2`" among lucide's *alias* forms that stopped +resolving, alongside the `HouseIcon` suffix and `LucideHouse` prefix shapes. The two alias +shapes are correctly described; `Building2` never belonged with them. It is a canonical +spelling, not an alias — `building-2` is a live key of lucide's dynamic surface, and only +this tokeniser could not reach it. Authors who moved off `Building2` on that advice lost +nothing (`building-2` is the same icon), but the spelling itself was never the problem and +works again. + +Released behaviour of `getLazyIcon`, `isLucideIconName` and `LazyIcon` for a name lucide +genuinely does not have is unchanged: it still degrades rather than throwing. diff --git a/packages/components/src/__tests__/lazy-icon-digit-boundary-9414.test.ts b/packages/components/src/__tests__/lazy-icon-digit-boundary-9414.test.ts new file mode 100644 index 0000000000..b08e6a48c0 --- /dev/null +++ b/packages/components/src/__tests__/lazy-icon-digit-boundary-9414.test.ts @@ -0,0 +1,185 @@ +/** + * 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#9414 — the icon seam's tokeniser had no letter-to-digit boundary. + * + * ## What was wrong + * + * `toKebabIconName` split two boundaries — `lower-or-digit -> Upper` and + * `acronym-run -> Word` — and never `letter -> digit`. So `Building2`, the + * spelling `lucide-react` exports and the spelling lucide's own site shows an + * author, tokenised to `building2`; lucide's canonical key is `building-2`; + * the name matched nothing and `getLazyIcon` degraded it to the `Database` + * glyph. ⛔ No error, no warning, no log — the author saw *an* icon with no + * signal that it was not theirs. + * + * ## Why no gate saw it, and why this file is a CAPABILITY pin + * + * `scripts/check-lucide-icon-record-names.mjs` censuses the names that ARE + * AUTHORED in this tree and judges them against lucide's runtime `icons` + * record. It was green on the defect and green correctly: nothing here authored + * a digit-suffixed name, and `lazy-icon.tsx` is a DECLARED_DYNAMIC_READER, + * which that gate reports and does not judge. What IS authored and what the + * tokeniser CAN accept are different populations, and only the second one is + * the seam's contract. ⇒ this file pins the second. + * + * ## The populations are re-derived, never written down (AGENTS.md #5 / #9) + * + * Every assertion below is computed from the installed `lucide-react` on every + * run. A count copied in here would state a fact derived once and never again, + * and would go on reading as live after lucide moved under it — which is the + * exact failure this seam already had. + */ +import { describe, expect, it } from 'vitest'; +import * as lucide from 'lucide-react'; +import { iconNames } from 'lucide-react/dynamic.mjs'; + +// Through the package's PUBLIC entry, which is the surface every consumer gets +// — never `../lib/lazy-icon`, which would pass on a seam that had stopped +// re-exporting these names. +import { getLazyIcon, isLucideIconName, toKebabIconName } from '../index'; + +/** The canonical vocabulary the seam accepts — lucide's own dynamic surface. */ +const CANONICAL: readonly string[] = iconNames as unknown as string[]; +const CANONICAL_SET = new Set(CANONICAL); + +/** Every spelling `lucide-react` exports, minus the runtime `icons` record. */ +const EXPORT_KEYS = Object.keys(lucide).filter((key) => key !== 'icons'); +const EXPORT_KEY_SET = new Set(EXPORT_KEYS); + +/** + * lucide's OWN `toPascalCase`, which is what turns a canonical icon name into + * the component name it exports: drop each hyphen, upper-case the character + * that followed it. Re-spelled here rather than imported because it lives in + * `@lucide/shared` and is not on `lucide-react`'s public surface; the guard in + * the first case below is what keeps this honest — it fails if this spelling + * ever stops agreeing with the export keys lucide actually ships. + */ +const toLucidePascalCase = (canonical: string): string => + canonical.replace(/(^|-)(\w)/g, (_match, _sep, char: string) => char.toUpperCase()); + +/** + * The PRE-#9414 tokeniser, frozen. + * + * ⛔ This is not a second copy of the implementation and must never be re-synced + * with it. It is the HISTORICAL baseline the no-loss leg is measured against: + * the change had to be a strict superset, so every name this resolved must + * still resolve to the byte-identical result. Re-syncing it would turn that + * measurement into a tautology. + */ +const PRE_9414 = (name: string): string => + name.includes('-') + ? name.toLowerCase() + : name + .replace(/([a-z0-9])([A-Z])/g, '$1-$2') + .replace(/([A-Z]+)([A-Z][a-z])/g, '$1-$2') + .toLowerCase(); + +describe('objectui#9414 — the seam accepts lucide’s canonical names in the spelling lucide exports', () => { + it('has both populations to judge, so the legs below are readings rather than vacuous', () => { + // A silently emptied population would satisfy every `toEqual([])` below. + expect(CANONICAL.length).toBeGreaterThan(0); + expect(EXPORT_KEYS.length).toBeGreaterThan(CANONICAL.length); + }); + + it('reaches EVERY canonical icon name from its own exported PascalCase spelling', () => { + // THE CAPABILITY. Derived over lucide's whole canonical vocabulary, so it + // moves with lucide instead of pinning a snapshot of it. + const pairs = CANONICAL.map((canonical) => [canonical, toLucidePascalCase(canonical)] as const); + + // Guard on the derivation itself: these PascalCase spellings must be the + // ones lucide really exports, or the leg below judges names nobody can + // author. This is what makes re-spelling `toPascalCase` above safe. + expect(pairs.filter(([, pascal]) => !EXPORT_KEY_SET.has(pascal)).map(([c]) => c)).toEqual([]); + + expect(pairs.filter(([, pascal]) => !isLucideIconName(pascal)).map(([c]) => c)).toEqual([]); + }); + + it('changes NOTHING the pre-#9414 tokeniser already resolved — byte-identical', () => { + // The must-stay-unchanged leg. A count that only goes up is not a reading: + // the change is a strict superset or it is a regression. + const alreadyResolved = EXPORT_KEYS.filter((key) => CANONICAL_SET.has(PRE_9414(key))); + expect(alreadyResolved.length).toBeGreaterThan(0); + + const moved = alreadyResolved.filter((key) => toKebabIconName(key) !== PRE_9414(key)); + expect(moved).toEqual([]); + }); + + it('leaves every canonical kebab spelling untouched', () => { + // The other half of the no-loss leg: authors write kebab too, and the + // early return for a hyphenated name must keep meaning what it meant. + expect(CANONICAL.filter((name) => toKebabIconName(name) !== name)).toEqual([]); + }); + + // The shapes the card named, pinned one per case so a red says WHICH shape + // died. The canonical spelling is asserted to be a live member in the same + // case, so a lucide retirement reds here loudly instead of leaving the + // mapping assertion restating a dead name. + it.each([ + ['Building2', 'building-2'], + ['BarChart3', 'bar-chart-3'], + ['CheckCircle2', 'check-circle-2'], + ['CircleSlash2', 'circle-slash-2'], + ['ArrowDown01', 'arrow-down-01'], + ['Clock1', 'clock-1'], + ['Axis3D', 'axis-3-d'], + ['Axis3d', 'axis-3d'], + ])('digit-suffixed `%s` tokenises to the canonical `%s`', (pascal, canonical) => { + expect({ kebab: toKebabIconName(pascal), live: CANONICAL_SET.has(canonical) }).toEqual({ + kebab: canonical, + live: true, + }); + expect(isLucideIconName(pascal)).toBe(true); + }); + + // The `Grid2x2` family — IN, deliberately (objectui#9414 fenced this as the + // taker's call). They are reached by the SAME single rule, not a second one: + // the negative lookbehind holds the split off inside a `2x2` run, so the + // result is lucide's PRIMARY spelling `grid-2x2` rather than its `grid-2-x-2` + // alias. `Grid3x2` is the shape that proves the difference matters — lucide + // ships no `grid-3-x-2` alias at all, so a rule that routed through the alias + // spellings would recover five of these six and leave this one behind. + it.each([ + ['Grid2x2', 'grid-2x2'], + ['Grid2x2Check', 'grid-2x2-check'], + ['Grid2x2Plus', 'grid-2x2-plus'], + ['Grid2x2X', 'grid-2x2-x'], + ['Grid3x2', 'grid-3x2'], + ['Grid3x3', 'grid-3x3'], + ])('digit-letter-digit `%s` tokenises to the canonical `%s`', (pascal, canonical) => { + expect({ kebab: toKebabIconName(pascal), live: CANONICAL_SET.has(canonical) }).toEqual({ + kebab: canonical, + live: true, + }); + expect(isLucideIconName(pascal)).toBe(true); + }); + + it('still rejects what is not an icon, so the legs above are not a yes-machine', () => { + // CONTROLS, known direction, HIT in this same run. + // `useLucideContext` is a hook and `default` is the module's default + // export; both are namespace keys and neither is an icon. They are the + // residue objectui#9414 deliberately leaves rejected. + expect(isLucideIconName('useLucideContext')).toBe(false); + expect(isLucideIconName('default')).toBe(false); + expect(isLucideIconName('NotAnIconAnywhere')).toBe(false); + expect(isLucideIconName('Building9999')).toBe(false); + expect(isLucideIconName('')).toBe(false); + expect(isLucideIconName(undefined)).toBe(false); + }); + + it('stops degrading a digit-suffixed name to the `Database` fallback', () => { + // The user-visible half, stated on the resolver rather than the tokeniser: + // the wrong-glyph outcome is what an author actually met. + const { Database } = lucide as unknown as Record; + expect(getLazyIcon('Building2')).not.toBe(Database); + // CONTROL: a name lucide really does not have still degrades, which is the + // documented behaviour for server-driven schemas naming other libraries. + expect(getLazyIcon('NotAnIconAnywhere')).toBe(Database); + }); +}); diff --git a/packages/components/src/__tests__/lazy-icon-generated-app-contract-7472.test.ts b/packages/components/src/__tests__/lazy-icon-generated-app-contract-7472.test.ts index 2459c68a5d..3a2c073903 100644 --- a/packages/components/src/__tests__/lazy-icon-generated-app-contract-7472.test.ts +++ b/packages/components/src/__tests__/lazy-icon-generated-app-contract-7472.test.ts @@ -124,12 +124,35 @@ describe('the icon seam the CLI generates code against', () => { it('does NOT accept lucide alias spellings, which is where the change narrows', () => { // Measured, not assumed, and pinned because it is the migration note the // change owes its readers. lucide's namespace exports each icon three ways - // — `House`, `HouseIcon`, `LucideHouse` — plus digit-suffixed spellings - // like `Building2`. Only the canonical one is an `icons` key, so the alias - // spellings resolved in a generated app and nowhere else in the platform. - // Aligning the template is what removes them. - for (const alias of ['HouseIcon', 'LucideHouse', 'Building2']) { + // — `House`, `HouseIcon`, `LucideHouse`. Only the canonical one is an + // `icons` key, so the alias spellings resolved in a generated app and + // nowhere else in the platform. Aligning the template is what removes them. + for (const alias of ['HouseIcon', 'LucideHouse']) { expect(isLucideIconName(alias), alias).toBe(false); } }); + + it('accepts the digit-suffixed canonical spellings, which were never an alias case', () => { + // objectui#9414 corrected this file's OWN reading. `Building2` sat in the + // list above as though it were a fourth alias shape, and it never was: it + // is the PascalCase component lucide exports for the canonical key + // `building-2`, exactly as `House` is for `house`. What rejected it was + // this seam's tokeniser, which split `lower-or-digit -> Upper` and + // `acronym-run -> Word` and never `letter -> digit` — so `Building2` + // became `building2`, matched no canonical name, and a generated app + // authoring the spelling lucide's own site shows got the `Database` glyph + // with no error, no warning and no log. + // + // ⚠ The two facts this file already had were both true and the WRONG + // conclusion was drawn from them: the alias spellings really did resolve + // only in a generated app, and `Building2` really did resolve there too. + // The first is a narrowing the migration owes its readers; the second was + // a defect on this side of the seam, wearing the first one's clothes. + // + // ⛔ Nothing about the alias narrowing above moved. `HouseIcon` and + // `LucideHouse` are still out, and still for the reason stated there. + for (const canonical of ['Building2', 'Grid2x2', 'BarChart3']) { + expect(isLucideIconName(canonical), canonical).toBe(true); + } + }); }); diff --git a/packages/components/src/lib/lazy-icon.tsx b/packages/components/src/lib/lazy-icon.tsx index 5ce39b479f..e25f2199ee 100644 --- a/packages/components/src/lib/lazy-icon.tsx +++ b/packages/components/src/lib/lazy-icon.tsx @@ -23,12 +23,44 @@ import React from 'react'; import { Database } from 'lucide-react'; import { DynamicIcon, iconNames } from 'lucide-react/dynamic.mjs'; -/** Convert PascalCase / camelCase / mixed names to kebab-case for DynamicIcon. */ +/** + * Convert PascalCase / camelCase / mixed names to kebab-case for DynamicIcon. + * + * The transform is the INVERSE of lucide's own `toPascalCase`, which is what + * builds every exported component name out of a canonical icon name: it drops + * each hyphen and upper-cases the character that followed it. So the job here + * is to put a boundary back wherever `toPascalCase` removed one, and nowhere + * else. Three rules do it, and each one answers a different shape of boundary: + * + * 1. `lower-or-digit -> Upper` — the ordinary word boundary (`ChevronRight`). + * 2. `acronym-run -> Word` — a run of capitals followed by a word + * (`SVGIcon`), where rule 1 would swallow the last capital. + * 3. `letter -> digit` — the boundary a digit-suffixed name carries + * (`Building2` -> `building-2`). ⚠️ It is NOT unconditional: the negative + * lookbehind holds the rule off when the letter is itself preceded by a + * digit, because that is lucide's grid spelling — `Grid2x2` is the Pascal + * form of `grid-2x2`, where the `x` sits INSIDE a segment rather than + * starting one. Splitting there produces `grid-2x-2`, which is not a name. + * + * ⛔ Rule 3 is the boundary the seam went without until objectui#9414, and the + * cost was not visible anywhere: `Building2` — the spelling `lucide-react` + * exports and lucide's own site shows — tokenised to `building2`, matched no + * canonical name, and `getLazyIcon` degraded it to the `Database` glyph with no + * error, no warning and no log. The author saw *an* icon and no signal that it + * was not theirs. + * + * ⛔ Do not write down how many names rule 3 recovers. The property that matters + * is re-derived from the installed lucide on every run by + * `__tests__/lazy-icon-digit-boundary-9414.test.ts`: every canonical icon name + * is reachable from its own exported PascalCase spelling, and every name the + * pre-#9414 tokeniser resolved still resolves to the byte-identical result. + */ export function toKebabIconName(name: string): string { if (name.includes('-')) return name.toLowerCase(); return name .replace(/([a-z0-9])([A-Z])/g, '$1-$2') .replace(/([A-Z]+)([A-Z][a-z])/g, '$1-$2') + .replace(/(? digit` boundary, so `CheckCircle2` became `check-circle2`. The + * row is left standing because it is what made the defect legible, and the + * control at the bottom of this file is where the move is recorded. ⛔ Do not + * read the row as live — the instrument is `isLucideIconName`, not this table + * (AGENTS.md #9). * * ## Why the table is read from source instead of imported * @@ -154,15 +164,27 @@ describe('record-alert SEVERITY_STYLES icons resolve (objectui#7593)', () => { expect(isLucideIconName('circle-check')).toBe(true); expect(isLucideIconName('CircleCheck')).toBe(true); - // DARK: names that must NOT resolve. The first is the exact spelling that - // shipped the defect — `CheckCircle2` is still a live NAMED EXPORT of - // `lucide-react` (it is an alias of `CircleCheck`; the two are the same - // object), which is why it looks correct at a glance and why static - // `import { CheckCircle2 }` call-sites elsewhere in the repo are fine. It - // is dead only for NAME-BASED lookup, which is the route this constant - // takes. Should lucide ever add `check-circle2` to the dynamic surface, - // this line goes red — deliberately, because that would be worth a look. - expect(isLucideIconName('CheckCircle2')).toBe(false); + // The tripwire this file armed FIRED, and this is the look it asked for. + // + // It used to read `expect(isLucideIconName('CheckCircle2')).toBe(false)`, + // with the note: "Should lucide ever add `check-circle2` to the dynamic + // surface, this line goes red — deliberately, because that would be worth + // a look." It went red for a cause the note did not anticipate and which + // is worth exactly the same look: lucide added nothing, and objectui#9414 + // taught the SEAM the `letter -> digit` boundary it never had. The dynamic + // surface held `check-circle-2` the whole time; `toKebabIconName` produced + // `check-circle2` and missed it. ⇒ `CheckCircle2` moves from DARK to LIT, + // and it is a LIT case now rather than a deleted one because the spelling + // that shipped the defect resolving is the whole outcome of that card. + expect(isLucideIconName('CheckCircle2')).toBe(true); + + // DARK: names that must NOT resolve. `LucideCheckCircle2` keeps this half + // of the control on the SAME footing the old line stood on — a live NAMED + // EXPORT of `lucide-react` that is dead for NAME-BASED lookup, which is the + // route `SEVERITY_STYLES` takes — so a seam that started accepting + // anything at all still reds here. ⛔ It is a lucide PREFIX ALIAS, not a + // canonical key, which is why #9414 did not and must not reach it. + expect(isLucideIconName('LucideCheckCircle2')).toBe(false); expect(isLucideIconName('no-such-glyph-xyz')).toBe(false); }); });