diff --git a/.changeset/9204-icon-names-from-the-record.md b/.changeset/9204-icon-names-from-the-record.md new file mode 100644 index 0000000000..9268c1ea3f --- /dev/null +++ b/.changeset/9204-icon-names-from-the-record.md @@ -0,0 +1,39 @@ +--- +'@object-ui/components': minor +'@object-ui/app-shell': minor +'@object-ui/console': minor +--- + +Icon-name membership answers from lucide's `icons` record, and a retired +spelling is refused instead of silently degraded (objectui#9204). + +⚠️ **Behaviour change, on purpose.** lucide publishes two icon vocabularies: the +runtime `icons` record it actually ships, and the larger dynamic-import list +that still carries spellings lucide has retired. `getLazyIcon`, `LazyIcon` and +`isLucideIconName` used to judge names against the second one; they now judge +against the record, which is what every other resolver in this package already +read. Measured against the installed lucide, that retires **254 spellings** — +`smile`, `edit`, `filter`, `alert-triangle`, `sort-desc` and the rest. + +- **A retired spelling is now REFUSED OUT LOUD.** It used to become the + `Database` glyph (or, through `isLucideIconName`, a notification's severity + icon) with nothing logged — a page that still rendered and a glyph that looked + deliberate. The console now carries a diagnostic, once per spelling, naming + the spelling, lucide's current name for it, and the exact spelling to write + instead. Both halves are derived from the installed lucide at runtime; there + is no list of retired names in this repo to go stale. +- **`isLucideIconName` keeps its signature and its synchronous contract.** Only + the vocabulary behind it moved. Spellings the two lists agree on — kebab-case, + snake_case, space-separated and PascalCase alike — resolve exactly as before. +- **New export `loadLucideIconNames()`** — the renderable icon vocabulary, + `Promise`-returning, for a picker that needs the whole list to search. The + metadata designer's icon picker loads it when it opens. + +**Why:** importing the icon NAMES imports lucide's dynamic-import map with them, +because lucide derives the names as that map's keys. Four modules did, and the +map sat in the console's eager `ui-components` chunk on every page load. Sourcing +membership from the record — which the same chunk already carries — takes +**8,515 gzipped bytes** off that chunk and **8,520** off the whole eager closure, +measured on two console builds in one container. The `ui-components` row goes +from 1,910 B of headroom (0.02x, red) to 10,425 B (0.11x, green), which pays off +the declared allowance that row has been carrying. diff --git a/apps/console/src/utils/getIcon.ts b/apps/console/src/utils/getIcon.ts index 5ecf906246..4ccb4e82fa 100644 --- a/apps/console/src/utils/getIcon.ts +++ b/apps/console/src/utils/getIcon.ts @@ -1,41 +1,22 @@ /** * Icon utilities * - * Synchronous accessor that returns a lazy-loaded Lucide icon React - * component. Wraps lucide-react's `DynamicIcon` so we don't bloat the - * vendor bundle by statically importing the entire icon namespace. - */ - -import React from 'react'; -import { Database } from 'lucide-react'; -import { DynamicIcon } from 'lucide-react/dynamic'; - -function toKebab(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') - .toLowerCase(); -} - -const cache = new Map(); - -/** - * Resolve a Lucide icon component by name. + * Synchronous accessor that returns a lazy-loaded Lucide icon React component. + * + * ## Delegated rather than transcribed (objectui#9204) * - * The result is memoised per name in the module-level `cache`, so call sites - * get a *stable* component reference across renders — nothing is created during + * This was a third copy of `@object-ui/components`' `getLazyIcon` — the same + * kebab-casing, the same memo, the same `Database` fallback — differing only in + * that it skipped the name check and let lucide log "Name in Lucide DynamicIcon + * not found" for an off-catalog name. Its `lucide-react/dynamic` import put + * lucide's 2,025-entry dynamic-import map on the console's eager path; the + * shared resolver keeps the icon NAMES as data and fetches the map through + * `import()` on first use. + * + * The result is memoised per name inside that resolver, so call sites still get + * a *stable* component reference across renders — nothing is created during * render. `react-hooks/static-components` cannot see through the call, so the * JSX sites that render the result carry a targeted disable pointing back here. */ -export function getIcon(name?: string): React.ElementType { - if (!name) return Database; - const cached = cache.get(name); - if (cached) return cached; - const kebab = toKebab(name); - const Wrapped: React.FC = (props) => - React.createElement(DynamicIcon as any, { name: kebab, fallback: Database, ...props }); - Wrapped.displayName = `LucideIcon(${name})`; - cache.set(name, Wrapped); - return Wrapped; -} + +export { getLazyIcon as getIcon } from '@object-ui/components'; diff --git a/packages/app-shell/src/utils/getIcon.ts b/packages/app-shell/src/utils/getIcon.ts index e98ed6ec50..dc8fd054e0 100644 --- a/packages/app-shell/src/utils/getIcon.ts +++ b/packages/app-shell/src/utils/getIcon.ts @@ -1,65 +1,35 @@ +/** + * 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. + */ + /** * Icon utilities * * Helpers for resolving Lucide icons by name. * - * Implementation: instead of statically importing every icon (~1500 - * components, ~568 KB raw / 140 KB gz), we wrap lucide-react's built-in - * `DynamicIcon` so each icon is fetched as its own tiny chunk on first use. - * * The exported `getIcon(name)` API stays synchronous and returns a React * component, preserving call sites that do `const Icon = getIcon(name); `. - */ - -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. */ -function toKebab(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') - .toLowerCase(); -} - -// Lucide ships ~3900 icon names; storing as a Set keeps lookups O(1). -const VALID_ICON_NAMES: Set = new Set(iconNames as string[]); - -const cache = new Map(); - -/** - * Resolve a Lucide icon by name (kebab-case or PascalCase). * - * Returns a React component that lazy-loads the underlying SVG icon on - * mount. Falls back to the `Database` icon (statically imported) when no - * `name` is given, or when the requested name is not a valid Lucide icon - * — server-driven metadata frequently references icons from other libraries - * (e.g. `box-open` from Font Awesome), and we silently degrade to the - * fallback rather than letting Lucide log a console error. + * ## One resolver, not a second copy (objectui#9204) * - * The returned component is memoised per `name` so repeated calls with the - * same name yield the same component reference (stable for React.memo). + * This file used to carry its own transcription of `@object-ui/components`' + * `getLazyIcon`: the same kebab-casing, the same name-membership Set, the same + * per-name memo, the same `Database` fallback. The copy is now a delegation, + * for two reasons that are the same reason: + * + * - the membership Set was built from `iconNames`, and lucide derives that + * from its 2,025-entry dynamic-import map — so this module's import alone + * put that whole map on the console's eager path; + * - two transcriptions of one lookup are two chances to disagree about which + * lucide vocabulary a name is judged against, which is precisely what + * `scripts/check-lucide-icon-record-names.mjs` censuses. + * + * The shared resolver keeps the names as data and reaches the map through + * `import()`. Behaviour here is unchanged: same normalisation, same fallback. */ -export function getIcon(name?: string): React.ElementType { - if (!name) return Database; - const cached = cache.get(name); - if (cached) return cached; - - const kebab = toKebab(name); - if (!VALID_ICON_NAMES.has(kebab)) { - cache.set(name, Database); - return Database; - } - const Wrapped: React.FC = (props) => - React.createElement(DynamicIcon as any, { - name: kebab, - fallback: Database, - ...props, - }); - Wrapped.displayName = `LucideIcon(${name})`; - cache.set(name, Wrapped); - return Wrapped; -} +export { getLazyIcon as getIcon } from '@object-ui/components'; diff --git a/packages/app-shell/src/views/metadata-admin/IconPickerWidget.test.tsx b/packages/app-shell/src/views/metadata-admin/IconPickerWidget.test.tsx index ffe05e9900..1ad4b5db07 100644 --- a/packages/app-shell/src/views/metadata-admin/IconPickerWidget.test.tsx +++ b/packages/app-shell/src/views/metadata-admin/IconPickerWidget.test.tsx @@ -1,7 +1,12 @@ // Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. import { describe, it, expect, afterEach, vi } from 'vitest'; -import { render, screen, fireEvent, cleanup } from '@testing-library/react'; +import { render, screen, fireEvent, cleanup, waitFor } from '@testing-library/react'; +// The grid's vocabulary arrives through `import('lucide-react/dynamic.mjs')` +// (objectui#9204). Importing that module here, at module scope, pays for it in +// the import phase — which is under no test or hook timeout — instead of +// letting a saturated transform pipeline spend a `findBy` budget on it. +import 'lucide-react/dynamic.mjs'; import { WIDGETS } from './widgets'; afterEach(cleanup); @@ -11,9 +16,15 @@ const Icon = WIDGETS['icon']; /** * `icon` widget — a searchable Lucide icon picker for page/app/object `icon` * fields (replaces the raw text input). Built inline (no Radix portal) so the - * trigger, search box and result grid all render eagerly in jsdom. The icon - * previews lazy-load their SVG chunk and degrade to a fallback glyph, so these - * tests assert on the catalog wiring rather than the rendered . + * trigger, search box and result grid all render without a portal in jsdom. The + * icon previews lazy-load their SVG chunk and degrade to a fallback glyph, so + * these tests assert on the catalog wiring rather than the rendered . + * + * ⚠️ The GRID is asynchronous since objectui#9204 — the vocabulary is lucide's + * dynamic-import map, fetched when the dialog opens rather than held on the + * eager path. The trigger is not: it asks `isLucideIconName`, which reads the + * `icons` record and needs nothing loaded. That split is why the rows below + * await the options but not the combobox. */ describe('icon widget', () => { it('is registered in the WIDGETS map', () => { @@ -27,34 +38,67 @@ describe('icon widget', () => { expect(trigger).toHaveTextContent('calendar'); }); - it('opens a search box and filters the icon grid by query', () => { + it('says it is loading before the catalogue lands, not "no matching icons"', () => { + // The synchronous frame. An empty grid mid-fetch must not read as a query + // that found nothing — that sentence would be a false answer to a question + // nobody has asked yet. + render( {}} schema={{ type: 'string' }} />); + fireEvent.click(screen.getByRole('combobox')); + expect(screen.queryAllByRole('option')).toHaveLength(0); + expect(screen.getByText('Loading options…')).toBeInTheDocument(); + expect(screen.queryByText('No matching icons.')).toBeNull(); + }); + + it('opens a search box and filters the icon grid by query', async () => { render( {}} schema={{ type: 'string' }} />); fireEvent.click(screen.getByRole('combobox')); const search = screen.getByLabelText('Search icons…'); - const before = screen.getAllByRole('option').length; + const before = (await screen.findAllByRole('option')).length; expect(before).toBeGreaterThan(0); fireEvent.change(search, { target: { value: 'ampersand' } }); + await waitFor(() => { + expect(screen.getAllByRole('option').length).toBeLessThan(before); + }); const after = screen.getAllByRole('option'); expect(after.length).toBeGreaterThan(0); - expect(after.length).toBeLessThan(before); // Every surviving option matches the query. for (const opt of after) { expect(opt.getAttribute('title')).toContain('ampersand'); } }); - it('writes the selected icon name through onChange', () => { + it('writes the selected icon name through onChange', async () => { const onChange = vi.fn(); render(); fireEvent.click(screen.getByRole('combobox')); + await screen.findAllByRole('option'); fireEvent.change(screen.getByLabelText('Search icons…'), { target: { value: 'ampersand' } }); + await waitFor(() => { + expect(screen.getAllByRole('option')[0]?.getAttribute('title')).toContain('ampersand'); + }); fireEvent.click(screen.getAllByRole('option')[0]); expect(onChange).toHaveBeenCalledTimes(1); expect(String(onChange.mock.calls[0][0])).toContain('ampersand'); }); + it('offers only names the renderer will resolve — the vocabulary is the record', async () => { + // objectui#9204: the picker and `getLazyIcon` read ONE vocabulary now. A + // retired spelling in this grid would be an author-facing trap — pickable, + // then refused at render time. + render( {}} schema={{ type: 'string' }} />); + fireEvent.click(screen.getByRole('combobox')); + fireEvent.change(screen.getByLabelText('Search icons…'), { target: { value: 'filter' } }); + const options = await screen.findAllByRole('option'); + const titles = options.map((o) => o.getAttribute('title')); + // `filter` is a spelling lucide still LOADS but has dropped from the record + // (it is `funnel` now). ⭐ The control is `list-filter`: the query DOES match + // live names, so the absence below is a refusal rather than an empty grid. + expect(titles).toContain('list-filter'); + expect(titles).not.toContain('filter'); + }); + it('preserves an out-of-catalog value (renders it, offers a keep option)', () => { render( {}} schema={{ type: 'string' }} />); const trigger = screen.getByRole('combobox'); diff --git a/packages/app-shell/src/views/metadata-admin/widgets.tsx b/packages/app-shell/src/views/metadata-admin/widgets.tsx index 4d40dd825d..b4266f824e 100644 --- a/packages/app-shell/src/views/metadata-admin/widgets.tsx +++ b/packages/app-shell/src/views/metadata-admin/widgets.tsx @@ -30,7 +30,9 @@ import { Button, Label, Switch, + isLucideIconName, LazyIcon, + loadLucideIconNames, toKebabIconName, Popover, PopoverTrigger, @@ -43,7 +45,6 @@ import { } from '@object-ui/components'; import type { ComponentMeta } from '@object-ui/core'; import { AlertTriangle, ChevronDown, ChevronsUpDown, ChevronUp, Eye, EyeOff, Plus, Search, Trash2 } from 'lucide-react'; -import { iconNames } from 'lucide-react/dynamic.mjs'; import { toast } from 'sonner'; import { useObjectTranslation } from '@object-ui/i18n'; import { useMetadataLocale, t, tFormat } from './i18n.js'; @@ -1553,12 +1554,21 @@ function FieldRefMultiWidget({ value, onChange, readOnly, context, ariaLabelledB /* icon — searchable Lucide icon picker */ /* -------------------------------------------------------------------------- */ -// Lucide ships ~1500+ kebab-case icon names; freeze once for O(1) reuse. -const LUCIDE_ICON_NAMES: readonly string[] = iconNames as string[]; -const LUCIDE_ICON_SET: Set = new Set(LUCIDE_ICON_NAMES); +// ⛔ No module-scope catalogue. The vocabulary arrives from +// `loadLucideIconNames()` (@object-ui/components) when the dialog opens: it is +// the LIVE `icons` record intersected with lucide's dynamic spellings, derived +// from the installed lucide rather than kept as a list. Importing `iconNames` +// from `lucide-react/dynamic.mjs` to get the same strings drags lucide's +// 2,039-entry dynamic-import map onto the console's eager path, and a generated +// mirror of those strings measured DEARER than the map it replaced +// (objectui#9204). Membership of a single name is `isLucideIconName`, which +// reads the record synchronously and needs nothing loaded. // Cap the rendered grid — each cell mounts a lazily-loaded icon, so showing all // ~1500 at once would fire a flood of chunk requests. The search box narrows it. const ICON_RESULT_LIMIT = 120; +// A stable empty array, so the `useMemo` below does not see a new identity on +// every render before the catalogue lands. +const EMPTY_CATALOGUE: readonly string[] = []; /** * Searchable icon picker for `widget: 'icon'` string fields (page/app/object @@ -1572,8 +1582,15 @@ const ICON_RESULT_LIMIT = 120; * plain text (LazyIcon degrades to a fallback glyph) and is offered as the first * "keep" option so re-opening the picker never silently drops it. * - * Built inline (no Radix portal) so the search + grid render eagerly — the same - * jsdom-friendly choice the other pickers' tests rely on. + * Built inline (no Radix portal) so the search + grid render without a portal — + * the same jsdom-friendly choice the other pickers' tests rely on. + * + * ⚠️ The GRID is asynchronous since objectui#9204: the icon vocabulary arrives + * from `loadLucideIconNames()` when the dialog opens, because the only list of + * renderable spellings lucide publishes is the dynamic-import map, and holding + * that eagerly is what put 8,253 gzipped bytes on the console's first payload. + * The trigger stays synchronous — `isLucideIconName` reads the `icons` record, + * which needs nothing loaded. */ export function IconPickerWidget({ id, value, onChange, readOnly }: WidgetProps) { const locale = useMetadataLocale(); @@ -1582,12 +1599,25 @@ export function IconPickerWidget({ id, value, onChange, readOnly }: WidgetProps) const [query, setQuery] = React.useState(''); const currentKebab = current ? toKebabIconName(current) : ''; - const inCatalog = !current || LUCIDE_ICON_SET.has(currentKebab); + const inCatalog = !current || isLucideIconName(current); + + // Fetched when the dialog first opens, then kept — `loadLucideIconNames` + // memoises the underlying `import()`, so re-opening costs nothing. + const [catalogue, setCatalogue] = React.useState(EMPTY_CATALOGUE); + React.useEffect(() => { + if (!open || catalogue.length) return undefined; + let alive = true; + loadLucideIconNames().then( + (names) => { if (alive) setCatalogue(names); }, + (error) => { console.error('[metadata-admin] failed to load the lucide icon catalogue', error); }, + ); + return () => { alive = false; }; + }, [open, catalogue.length]); const q = toKebabIconName(query.trim()); const allMatches = React.useMemo( - () => (q ? LUCIDE_ICON_NAMES.filter((n) => n.includes(q)) : LUCIDE_ICON_NAMES), - [q], + () => (q ? catalogue.filter((n) => n.includes(q)) : catalogue), + [q, catalogue], ); const results = allMatches.slice(0, ICON_RESULT_LIMIT); const truncated = allMatches.length > results.length; @@ -1690,11 +1720,22 @@ export function IconPickerWidget({ id, value, onChange, readOnly }: WidgetProps) ); })} - {results.length === 0 && ( + {results.length === 0 && catalogue.length > 0 && (

{t('engine.form.noMatchingIcons', locale)}

)} + {/* ⚠️ The two empty states are NOT the same sentence. An empty grid + while the catalogue is still in flight is not "no matching + icons" — that reading would tell an author their query found + nothing when nothing had been searched yet. Literal keys on both + arms, because the i18n call-site gate reads the key, not the + expression that chose it. */} + {results.length === 0 && catalogue.length === 0 && ( +

+ {t('engine.form.loadingOptions', locale)} +

+ )} {truncated && ( diff --git a/packages/components/src/__tests__/icon-name-vocabulary-9204.test.ts b/packages/components/src/__tests__/icon-name-vocabulary-9204.test.ts new file mode 100644 index 0000000000..d10cb077ad --- /dev/null +++ b/packages/components/src/__tests__/icon-name-vocabulary-9204.test.ts @@ -0,0 +1,204 @@ +/** + * 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. + */ + +/** + * Icon-name membership answers from the `icons` RECORD, and a retired spelling + * is REFUSED out loud rather than silently degraded (objectui#9204, maintainer + * decision batch #125 item 3, 2026-09-13). + * + * ## What this file pins, and what it deliberately does not + * + * The byte claim — that lucide's dynamic-import map left the console's eager + * path — is enforced where bytes are decided: `check-eager-closure-budget.mjs` + * on the emitted chunk, and `check-lucide-icon-record-names.mjs`'s empty + * `DECLARED_EAGER_DYNAMIC_IMPORTERS` on the source shape. ⛔ Neither is this + * file's subject; a render test cannot tell a static import from a deferred one. + * + * What IS this file's subject is the behaviour the ruling bought those bytes + * with: the vocabulary narrowed from lucide's DYNAMIC list to its RECORD, so + * the spellings the two disagree about stop resolving — and the author has to + * find out at once instead of reading a plausible fallback glyph. + * + * ## Every population here is DERIVED from the installed lucide + * + * ⛔ No list of retired spellings is written down. `check-lucide-icon-record-names.mjs` + * refuses one in its own header — "a hand-kept vocabulary is the same defect one + * level up: it ages the moment lucide retires the next name, and it ages + * SILENTLY" — and a test is not exempt from its own gate's rule. The two + * vocabularies are read here and the difference between them IS the population, + * so this file cannot go stale against a lucide bump: it can only change what + * it reports. + */ + +import { beforeEach, describe, expect, it, vi, afterEach } from 'vitest'; +import { icons } from 'lucide-react'; +// The refusal diagnostic derives the replacement name through +// `import('lucide-react/dynamic.mjs')`. Importing it at MODULE scope pays for +// it in the import phase, which is under no test or hook timeout — the +// `await import()` inside a bounded assertion window is the flaky shape +// AGENTS.md's testing section is about. +import { dynamicIconImports } from 'lucide-react/dynamic.mjs'; + +import { getLazyIcon, isLucideIconName, loadLucideIconNames } from '../lib/lazy-icon'; +import { describeIconLookup, liveIconNameOf } from '../renderers/action/resolve-icon'; + +/** Names lucide can still LOAD but no longer publishes in its `icons` record. */ +const RETIRED = Object.keys(dynamicIconImports).filter( + (name) => !Object.prototype.hasOwnProperty.call(icons, describeIconLookup(name).key), +); +/** Names both vocabularies agree on. */ +const LIVE = Object.keys(dynamicIconImports).filter( + (name) => Object.prototype.hasOwnProperty.call(icons, describeIconLookup(name).key), +); + +/** + * `console.error` is mocked for the WHOLE file, not only where it is asserted. + * The sweeps below refuse every retired spelling on purpose, and each refusal + * resolves asynchronously — left unmocked they would land in another test's + * output, after this file's assertions had finished. + */ +let errorSpy: ReturnType; +beforeEach(() => { + errorSpy = vi.spyOn(console, 'error').mockImplementation(() => {}); +}); +afterEach(() => { + vi.restoreAllMocks(); +}); + +/** + * Wait for the refusal's own `import()` chain to settle. + * + * ⛔ Not a fixed sleep and ⛔ not one tick: deriving the replacement name walks + * a module import and then an icon import, so the number of turns is lucide's + * to decide, not this file's. Polling for the observable effect is the shape + * that does not encode a guess about it. + */ +async function settle(predicate: () => boolean, turns = 200): Promise { + for (let i = 0; i < turns && !predicate(); i += 1) { + await new Promise((resolve) => { setTimeout(resolve, 1); }); + } +} + +describe('a retired spelling is refused OUT LOUD', () => { + // ⚠️ This block runs FIRST, and the ordering is load-bearing: a spelling is + // refused once per process, so the population sweeps below — which ask about + // every retired name — would spend these specimens' one refusal before the + // assertions could see it. Vitest runs describes in file order. + + it('names the spelling, lucide’s current name for it, and what to write', async () => { + // `smile` is a specimen, ⛔ not a pinned fact: the sweep below judges the + // whole retired population, and this row checks the MESSAGE. Its premise is + // asserted rather than assumed. + expect(RETIRED).toContain('smile'); + + expect(isLucideIconName('smile')).toBe(false); + await settle(() => errorSpy.mock.calls.length > 0); + + expect(errorSpy).toHaveBeenCalledTimes(1); + const message = String(errorSpy.mock.calls[0]?.[0]); + expect(message).toContain('"smile"'); + expect(message).toContain('RETIRED'); + // lucide's current NAME (the live record key)… + expect(message).toContain('"FaceSlightlySmiling"'); + // …and the spelling that actually renders, which is not always its kebab. + expect(message).toContain('"face-slightly-smiling"'); + }); + + it('suggests a spelling `getLazyIcon` accepts, for every retired name', async () => { + // ⭐ The half a single specimen cannot show. 16 of the 243 live replacements + // are digit-bearing (`Grid2x2`, `Axis3d`, `Rows2`…) and their record key + // kebabs to a name lucide does not carry — so a diagnostic that echoed the + // key would send an author to a name that passes membership and then fails + // to load. This walks the whole retired population rather than trusting the + // one above. + const suggestions = await Promise.all( + RETIRED.map(async (retired) => { + const live = await dynamicIconImports[retired as keyof typeof dynamicIconImports](); + return liveIconNameOf((live as { default?: unknown }).default); + }), + ); + const unrenderable = suggestions.filter((key) => key !== null && !isLucideIconName(key)); + // Derived, so it reports rather than pins: these are the keys whose OWN + // spelling would not render, i.e. exactly why the message carries both. + expect(unrenderable.length).toBeGreaterThan(0); + const offered = await loadLucideIconNames(); + for (const key of suggestions) { + if (key === null) continue; + expect(offered.some((name) => describeIconLookup(name).key === key)).toBe(true); + } + }); + + it('says so for a name that was never lucide’s, WITHOUT inventing a replacement', async () => { + // The control for the first row: "RETIRED" has to be a claim this + // diagnostic can decline to make, or it means nothing when it does. + expect(isLucideIconName('box-open-from-another-library')).toBe(false); + await settle(() => errorSpy.mock.calls.length > 0); + + const message = String(errorSpy.mock.calls[0]?.[0]); + expect(message).toContain('"box-open-from-another-library"'); + expect(message).toContain('No live lucide icon answers to it'); + expect(message).not.toContain('RETIRED'); + }); + + it('says it ONCE per spelling, not once per render', async () => { + expect(RETIRED).toContain('sort-desc'); + for (let i = 0; i < 5; i += 1) getLazyIcon('sort-desc'); + await settle(() => errorSpy.mock.calls.length > 0); + expect(errorSpy).toHaveBeenCalledTimes(1); + }); + + it('stays silent for a live name — the volume is the signal', async () => { + expect(isLucideIconName('circle-check')).toBe(true); + getLazyIcon('circle-check'); + await settle(() => errorSpy.mock.calls.length > 0, 20); + expect(errorSpy).not.toHaveBeenCalled(); + }); +}); + +describe('the vocabulary is the `icons` record', () => { + it('has a non-empty disagreement to judge — the control for everything below', () => { + // Without this the two rows underneath would pass on an empty population + // and read exactly like a satisfied assertion. A lucide release that + // retired nothing would make this file vacuous, and this is where that + // shows up. + expect(RETIRED.length).toBeGreaterThan(0); + expect(LIVE.length).toBeGreaterThan(0); + }); + + it('refuses EVERY spelling the record has dropped', () => { + const accepted = RETIRED.filter((name) => isLucideIconName(name)); + expect(accepted).toEqual([]); + }); + + it('accepts every spelling the record still carries', () => { + const rejected = LIVE.filter((name) => !isLucideIconName(name)); + expect(rejected).toEqual([]); + }); + + it('still answers PascalCase, snake_case and space-separated spellings', () => { + // The tokeniser did not change with the vocabulary. `Home` is the seam's one + // rename entry and has to keep resolving, which is also a control on the + // row above: it is a name the record does NOT carry. + expect(Object.prototype.hasOwnProperty.call(icons, 'Home')).toBe(false); + expect(isLucideIconName('home')).toBe(true); + expect(isLucideIconName('CircleCheck')).toBe(true); + expect(isLucideIconName('circle_check')).toBe(true); + expect(isLucideIconName('circle check')).toBe(true); + expect(isLucideIconName('no-such-glyph-xyz')).toBe(false); + }); + + it('offers pickers only the live half, in lucide’s own spelling', async () => { + const offered = await loadLucideIconNames(); + expect(offered.length).toBe(LIVE.length); + expect(offered).not.toContain(RETIRED[0]); + // …and every offered name is one `getLazyIcon` will accept, which is the + // property a picker actually depends on. + expect(offered.filter((name) => !isLucideIconName(name))).toEqual([]); + }); +}); + diff --git a/packages/components/src/__tests__/lazy-icon-deferred-map-9204.test.tsx b/packages/components/src/__tests__/lazy-icon-deferred-map-9204.test.tsx new file mode 100644 index 0000000000..31996bf1c4 --- /dev/null +++ b/packages/components/src/__tests__/lazy-icon-deferred-map-9204.test.tsx @@ -0,0 +1,71 @@ +/** + * 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. + */ + +/** + * `LazyIcon` still resolves a real glyph once lucide's dynamic-import map + * arrives (objectui#9204). + * + * The map moved behind an `import()`, which is a byte claim — and the byte + * claim is enforced where bytes are decided: the emitted chunk, by + * `scripts/check-eager-closure-budget.mjs`, and the source shape by + * `scripts/check-lucide-icon-record-names.mjs`'s empty + * `DECLARED_EAGER_DYNAMIC_IMPORTERS`. ⛔ Neither of those is what this file + * tests, and a render test could not: a static import renders identically. + * + * What deferral ADDS is a frame, and that is this file's subject. Before the + * import lands there is no `DynamicIcon` to render, so the icon shows its + * `fallback` — the same glyph `DynamicIcon` itself shows while fetching the + * per-icon chunk, one level down. The failure this pins is the one that would + * ship silently: a slot that renders NOTHING while the map is in flight, or one + * that never leaves the fallback because the promise was dropped. + */ + +import { describe, expect, it } from 'vitest'; +import { render, waitFor, cleanup } from '@testing-library/react'; + +import { LazyIcon, getLazyIcon, isLucideIconName } from '../lib/lazy-icon'; + +/** lucide renders the `Database` fallback with its own `lucide-database` class. */ +const isFallbackGlyph = (svg: Element | null) => !!svg?.getAttribute('class')?.includes('lucide-database'); + +describe('LazyIcon with the import map deferred', () => { + it('shows the fallback glyph first, then the resolved icon', async () => { + const { container } = render(); + + // The synchronous frame: something is rendered, and it is the fallback. + const first = container.querySelector('svg'); + expect(first, 'the slot rendered nothing at all while the map was in flight').not.toBeNull(); + expect(isFallbackGlyph(first)).toBe(true); + + // …and the promise is not dropped: the real glyph replaces it. + await waitFor(() => { + expect(isFallbackGlyph(container.querySelector('svg'))).toBe(false); + }); + expect(container.querySelector('svg')?.getAttribute('class')).toContain('lucide'); + cleanup(); + }); + + /** + * The control for the row above. `isFallbackGlyph` going false is only + * evidence of a resolved icon if it STAYS true for a name that cannot + * resolve — otherwise the assertion would pass on any re-render. + */ + it('keeps the fallback for a name outside the catalogue', async () => { + expect(isLucideIconName('no-such-glyph-xyz')).toBe(false); + const { container } = render(); + await waitFor(() => expect(container.querySelector('svg')).not.toBeNull()); + expect(isFallbackGlyph(container.querySelector('svg'))).toBe(true); + cleanup(); + }); + + it('keeps `getLazyIcon` synchronous and memoised per name', () => { + const first = getLazyIcon('circle-check'); + expect(typeof first === 'function' || typeof first === 'object').toBe(true); + expect(getLazyIcon('circle-check')).toBe(first); + }); +}); diff --git a/packages/components/src/index.ts b/packages/components/src/index.ts index a05a471bd5..6dbe6b6773 100644 --- a/packages/components/src/index.ts +++ b/packages/components/src/index.ts @@ -40,6 +40,11 @@ export { cn } from './lib/utils'; export { renderChildren, renderNodeSlot, isEmptyNodeSlot } from './lib/utils'; export { cva } from 'class-variance-authority'; export { getLazyIcon, isLucideIconName, LazyIcon, toKebabIconName } from './lib/lazy-icon'; +// The renderable icon vocabulary, loaded on demand. Published because the +// metadata designer's icon picker needs the whole list to search, and reading +// `iconNames` from `lucide-react/dynamic.mjs` to get it drags lucide's +// 2,039-entry dynamic-import map onto the eager path with them (objectui#9204). +export { loadLucideIconNames } from './lib/lazy-icon'; // The member-action visibility gate — "did this action DECLARE a `visible` gate // at all?", the single definition objectui#3492 established and PR #3816 / diff --git a/packages/components/src/lib/lazy-icon.tsx b/packages/components/src/lib/lazy-icon.tsx index 5ce39b479f..43a16b49c2 100644 --- a/packages/components/src/lib/lazy-icon.tsx +++ b/packages/components/src/lib/lazy-icon.tsx @@ -17,11 +17,67 @@ * The exported `getLazyIcon(name)` API stays synchronous and returns a * React component, preserving call-sites that do * `const Icon = getLazyIcon(name); `. + * + * ## ONE vocabulary: the record that actually ships (objectui#9204) + * + * Membership — "is this a real icon name?" — is answered by the seam in + * `../renderers/action/resolve-icon`, which reads lucide's runtime `icons` + * record. ⛔ It is NOT answered from `iconNames`, and the reason is both + * behavioural and measured: + * + * - BEHAVIOURAL. lucide publishes two vocabularies. The `icons` RECORD is + * what ships and what every other resolver in this repo reads; the DYNAMIC + * list is a strict superset that still carries 254 spellings lucide has + * retired (`smile`, `sort-desc`, `alarm-check`, `arrow-down-az`, measured + * against the installed lucide). Judging membership on the superset blesses + * exactly the names `scripts/check-lucide-icon-record-names.mjs` exists to + * keep out of authored metadata. + * - MEASURED. lucide derives `iconNames` as `Object.keys(dynamicIconImports)`, + * so a static import of EITHER name drags the dynamic-import map into + * whatever chunk holds this module — the console's eager `ui-components` + * chunk, where it cost 8,253 B gzipped. Shipping the names as a generated + * list instead costs MORE (9,176 B gz spliced into that same chunk, with a + * front-coded encoding probed and rejected at 8,397 B): the map's KEYS are + * the names, so a list of them is dearer than lucide's map of the same + * names. Sourcing membership from the record costs zero NEW eager bytes, + * because `resolve-icon.ts` already pays for that record. + * + * The maintainer ruled this on 2026-09-13 (objectui#9204, decision batch #125 + * item 3), with the retirement of the 254 aliases as the stated cost and the + * loud refusal below as its condition. + * + * ## What still loads lazily, and why it is only the MAP + * + * `DynamicIcon` and the import map behind it are reached through `import()`. + * ⛔ Do not restore a static `import ... from 'lucide-react/dynamic.mjs'` here + * or anywhere else: `scripts/check-lucide-icon-record-names.mjs` fails on one + * (its `DECLARED_EAGER_DYNAMIC_IMPORTERS` is empty and that emptiness IS the + * assertion), because it puts the map back on the first payload with nothing + * else red. + * + * While the map is in flight the icon renders its `fallback` — the same frame + * `DynamicIcon` itself shows while fetching the per-icon chunk, so this adds a + * loading STATE to nothing that did not already have one. + * + * ## ⚠️ One measured seam between membership and rendering + * + * Membership reads the RECORD (PascalCase keys); `DynamicIcon` reads the + * DYNAMIC map (kebab spellings). Measured against the installed lucide, every + * live record key has a dynamic spelling (0 of 1,781 do not), so nothing that + * passes membership is unrenderable when it is authored in lucide's own + * kebab spelling. The 95 record keys that carry a DIGIT are the exception, and + * only when authored in PascalCase: `Building2` kebabs to `building2` while + * lucide's spelling is `building-2`. Those 95 spellings answered `false` before + * this change too — they degraded to the fallback glyph silently, and they + * degrade to it loudly now (lucide's own "Name in Lucide DynamicIcon not found" + * on the render leg). ⛔ Not repaired here: a digit-aware tokeniser would change + * what resolves for names nobody has asked for, and `axis-3d` vs the retired + * `axis-3-d` shows the two spellings can BOTH exist with only one live. */ import React from 'react'; import { Database } from 'lucide-react'; -import { DynamicIcon, iconNames } from 'lucide-react/dynamic.mjs'; +import { describeIconLookup, liveIconNameOf, resolveIcon } from '../renderers/action/resolve-icon'; /** Convert PascalCase / camelCase / mixed names to kebab-case for DynamicIcon. */ export function toKebabIconName(name: string): string { @@ -32,36 +88,224 @@ export function toKebabIconName(name: string): string { .toLowerCase(); } -// Lucide ships ~3900 icon names; storing as a Set keeps lookups O(1). -const VALID_ICON_NAMES: Set = new Set(iconNames as string[]); - -/** Returns true when `kebab` matches a real Lucide icon. */ +/** + * Returns true when `kebab` names a LIVE glyph in lucide's `icons` record. + * + * Silent on its own — the refusal is raised by the call sites below, which know + * whether a `false` is a probe (`isLucideIconName`, whose whole job is to be + * asked) or an authored name that will not render. + */ function isLucideIcon(kebab: string): boolean { - return VALID_ICON_NAMES.has(kebab); + return resolveIcon(kebab) !== null; } +/* -------------------------------------------------------------------------- */ +/* The loud refusal */ +/* -------------------------------------------------------------------------- */ + /** - * Whether `name` (kebab-case or PascalCase) resolves to a real Lucide icon. + * Spellings already refused, so one bad name in a render loop says its piece + * once rather than once per frame. + */ +const refused = new Set(); + +/** + * Say, once per spelling, that a name does not resolve — and say what to write + * instead when lucide renamed it. + * + * ## Why a refusal and not the old silent degrade + * + * Until objectui#9204 an unresolvable name became the `Database` glyph here and + * the severity glyph in `../notifications/severity.ts`, with nothing logged. + * That is survivable for a name from another icon library — the case the old + * comment was written for — and it is NOT survivable for a spelling lucide + * retired: the author wrote a name that used to work, the page still renders, + * and the only signal is a glyph that looks deliberate. Narrowing membership to + * the record retires 254 such spellings at once, so the maintainer's ruling + * made ending that silence its condition: "a retired spelling is refused, not + * silently degraded", with "a diagnostic naming the spelling and its current + * name, so an author learns at once". + * + * ## Why the replacement is DERIVED and why that makes this async + * + * The retired export and its live spelling are the same object, so the current + * name is a reverse lookup in the record — but the retired name only reaches a + * component through the dynamic map, which this module deliberately does not + * hold eagerly. So the diagnostic awaits the same `import()` the render path + * uses (already in flight whenever any icon is on the page) and prints once it + * can name the replacement. ⛔ The alternative — a retired-to-live table in this + * file — is the hand-kept vocabulary `scripts/check-lucide-icon-record-names.mjs` + * refuses in its own header, and it would put ~5 KB gzipped back on the very + * path this card is emptying. + * + * ⚠️ It stays a console diagnostic rather than a thrown error: a single bad + * icon name in server-driven metadata must not take the page down, and + * `isLucideIconName` is a predicate callers ask BEFORE choosing what to draw. + * The refusal is in the volume and in the named replacement, not in a crash. + */ +function refuseIconName(name: string): void { + if (refused.has(name)) return; + refused.add(name); + const kebab = toKebabIconName(name); + const { key } = describeIconLookup(kebab); + void currentNameFor(kebab).then((current) => { + const head = `[@object-ui/components] icon name ${JSON.stringify(name)} does not resolve` + + ` (looked up as ${JSON.stringify(key)} in lucide's runtime icons record)`; + console.error( + current + ? `${head}. lucide RETIRED that spelling; its current name is ${JSON.stringify(current.key)}` + + ` — write ${JSON.stringify(current.spelling)} in the metadata.` + : `${head}. No live lucide icon answers to it.`, + ); + }); +} + +/** + * What a retired kebab spelling is called today, or `null` when the spelling is + * not lucide's at all. Diagnostics only; never on a render path. + * + * Returns BOTH halves because they can differ and only one of them is safe to + * copy into metadata: + * + * - `key` is lucide's own current name — the live `icons` record key, derived + * by identity from the module the retired spelling still loads. + * - `spelling` is a name this module can actually RENDER: the record key's + * kebab spelling as lucide itself spells it, found in the map rather than + * computed from the key. ⚠️ Computing it would be wrong for 16 of the 243 + * live replacements, all of them digit-bearing — `Grid2x2` kebabs to + * `grid2x2` while lucide's spelling is `grid-2x2`, so an author told to + * "write Grid2x2" would land on a name that passes membership and then + * fails to load. + */ +async function currentNameFor(kebab: string): Promise<{ key: string; spelling: string } | null> { + try { + const module = await loadLucideDynamic(); + const map = module.dynamicIconImports as Record Promise> | undefined; + const loader = map?.[kebab]; + if (typeof loader !== 'function') return null; + const icon = (await loader()) as { default?: unknown }; + const key = liveIconNameOf(icon?.default); + if (!key) return null; + const spelling = Object.keys(map!).find( + (candidate) => candidate !== kebab && describeIconLookup(candidate).key === key, + ); + return { key, spelling: spelling ?? key }; + } catch { + // A diagnostic that cannot be completed is still not a failure of the + // render it describes. Say what is known rather than nothing. + return null; + } +} + +/** + * Whether `name` (kebab-case or PascalCase) resolves to a live Lucide icon. * * Exported because `getLazyIcon` degrades an unknown name to the `Database` * icon, which is the right default for a data-shaped schema slot but wrong * where a caller has a BETTER fallback of its own — a notification, for * instance, would rather show its severity icon than a stray database glyph. * Ask first, then choose. + * + * A non-empty name that answers `false` is REFUSED out loud (see + * `refuseIconName`): choosing the severity glyph over an authored icon is one + * of the two silent degrades objectui#9204's ruling ended, and the caller's + * better fallback is exactly what used to hide it. */ export function isLucideIconName(name?: string): boolean { - return !!name && isLucideIcon(toKebabIconName(name)); + if (!name) return false; + if (isLucideIcon(toKebabIconName(name))) return true; + refuseIconName(name); + return false; +} + +/* -------------------------------------------------------------------------- */ +/* The deferred half: lucide's dynamic-import map */ +/* -------------------------------------------------------------------------- */ + +type LucideDynamicModule = typeof import('lucide-react/dynamic.mjs'); + +/** The loaded module, once it has arrived — read synchronously on later mounts. */ +let dynamicModule: LucideDynamicModule | null = null; +/** The in-flight request, so N icons mounting together make ONE import. */ +let dynamicRequest: Promise | null = null; + +function loadLucideDynamic(): Promise { + dynamicRequest ??= import('lucide-react/dynamic.mjs').then((module) => { + dynamicModule = module; + return module; + }); + return dynamicRequest; +} + +/** + * Every icon name this module can RENDER, in lucide's own kebab spelling. + * + * Two vocabularies meet here and neither answers alone: the `icons` record says + * which glyphs are LIVE, and the dynamic map says how each is SPELLED for + * `DynamicIcon`. The intersection is the answer — the live glyphs, spelled the + * way `getLazyIcon` can actually load them — and it is derived on demand from + * the installed lucide rather than kept as a list. ⛔ A generated catalogue was + * measured and rejected: it costs more gzipped than the map it replaces + * (objectui#9204). + * + * Async because it needs the deferred map, which is the point: a picker asks + * for it when it opens, so nothing about a name PICKER reaches the eager path. + */ +export async function loadLucideIconNames(): Promise { + const module = await loadLucideDynamic(); + return Object.keys(module.dynamicIconImports).filter((name) => isLucideIcon(name)); } +/** + * `DynamicIcon` behind an `import()`, with the caller's fallback showing until + * it lands. + * + * A plain `useState` + `useEffect` rather than `React.lazy`, deliberately: + * `React.lazy` would oblige every one of this package's icon call sites to sit + * under a `` boundary it does not have today, and would suspend a + * whole subtree over one glyph. This mirrors what `DynamicIcon` already does + * internally for the per-icon chunk, one level up. + */ +const DeferredLucideIcon: React.FC<{ name: string; fallback: React.ElementType } & Record> = ({ + name, + fallback, + ...rest +}) => { + const [DynamicIcon, setDynamicIcon] = React.useState( + () => (dynamicModule?.DynamicIcon as React.ElementType | undefined) ?? null, + ); + + React.useEffect(() => { + if (DynamicIcon) return undefined; + let alive = true; + loadLucideDynamic().then( + (module) => { + if (alive) setDynamicIcon(() => module.DynamicIcon as React.ElementType); + }, + (error) => { + // Same shape lucide uses for a per-icon chunk that fails to arrive: say + // so once and keep the fallback glyph, rather than blanking the slot. + console.error('[@object-ui/components] failed to load lucide-react/dynamic.mjs', error); + }, + ); + return () => { + alive = false; + }; + }, [DynamicIcon]); + + if (!DynamicIcon) return React.createElement(fallback, rest); + return React.createElement(DynamicIcon, { name, fallback, ...rest }); +}; + const cache = new Map(); /** * Resolve a Lucide icon by name (kebab-case or PascalCase). * Returns a memoised React component that lazily loads the SVG on mount. - * Falls back to the `Database` icon when no `name` is provided or when the - * requested name is not a valid Lucide icon (server-driven schemas often - * reference icons from other libraries — we silently degrade rather than - * letting Lucide log "Name in Lucide DynamicIcon not found"). + * Falls back to the `Database` icon when no `name` is provided, or when the + * requested name is not a live Lucide icon — server-driven schemas do reference + * icons from other libraries, so the slot still renders something, but the name + * is REFUSED OUT LOUD rather than silently degraded (objectui#9204). */ export function getLazyIcon(name?: string): React.ElementType { if (!name) return Database; @@ -69,11 +313,12 @@ export function getLazyIcon(name?: string): React.ElementType { if (cached) return cached; const kebab = toKebabIconName(name); if (!isLucideIcon(kebab)) { + refuseIconName(name); cache.set(name, Database); return Database; } const Wrapped: React.FC = (props) => - React.createElement(DynamicIcon as any, { name: kebab, fallback: Database, ...props }); + React.createElement(DeferredLucideIcon, { name: kebab, fallback: Database, ...props }); Wrapped.displayName = `LucideIcon(${name})`; cache.set(name, Wrapped); return Wrapped; @@ -83,8 +328,11 @@ export function getLazyIcon(name?: string): React.ElementType { export const LazyIcon: React.FC<{ name?: string } & Record> = ({ name, ...rest }) => { if (!name) return React.createElement(Database, rest); const kebab = toKebabIconName(name); - if (!isLucideIcon(kebab)) return React.createElement(Database, rest); - return React.createElement(DynamicIcon as any, { + if (!isLucideIcon(kebab)) { + refuseIconName(name); + return React.createElement(Database, rest); + } + return React.createElement(DeferredLucideIcon, { name: kebab, fallback: Database, ...rest, diff --git a/packages/components/src/renderers/action/resolve-icon.ts b/packages/components/src/renderers/action/resolve-icon.ts index 1fecd0e57d..e26da736d2 100644 --- a/packages/components/src/renderers/action/resolve-icon.ts +++ b/packages/components/src/renderers/action/resolve-icon.ts @@ -130,3 +130,57 @@ export function resolveIcon(name: string | undefined): LucideIcon | null { if (!name) return null; return (icons as Record)[describeIconLookup(name).key] ?? null; } + +/** + * The live record key whose component is `icon`, for DIAGNOSTICS only. + * + * ## Why this exists, and why it is a search rather than a table + * + * lucide retires a spelling by dropping it from the runtime `icons` record + * while keeping the deprecated named export — and the retired export and its + * live spelling are THE SAME OBJECT (`Smile === FaceSlightlySmiling` is + * `true`). So the replacement for a retired name is derivable by identity, and + * `scripts/check-lucide-icon-record-names.mjs` already derives it that way at + * gate time, in as many words: "When it has to name a replacement it derives + * one, by identity: the retired export and its live spelling are the same + * object, so the live key is looked up in the record rather than remembered." + * + * objectui#9204 needs the same answer at RUNTIME, because a retired spelling is + * now refused rather than silently degraded and the refusal has to name the + * current spelling. ⛔ A hand-kept retired-to-live table is not the way to get + * it: it is the same defect one level up — it ages the moment lucide retires + * the next name, it ages SILENTLY, and in this package it would also land on + * the eager path the same card is emptying. This searches the record the caller + * already pays for. + * + * ## ⚠️ Identity is the FIRST answer, not the only one — measured + * + * Identity holds only while both halves came from the same module instance, and + * that is not guaranteed: reached through `lucide-react/dynamic.mjs` the icon + * module can resolve into a DIFFERENT graph from the one `icons` came from, and + * then `===` is false for two components that are the same icon. Measured under + * this repo's vitest: `icons.FaceSlightlySmiling === (await + * dynamicIconImports['smile']()).default` is `false`, while both carry + * `displayName: 'FaceSlightlySmiling'`. + * + * So the fallback is lucide's own `displayName`, which `createLucideIcon` sets + * from the icon's file name — still DERIVED from lucide and still not a table. + * ⛔ It is not trusted blindly: a name is returned only after it is confirmed to + * be a live key of the record, so an unknown component cannot name itself into + * an answer. + * + * ⛔ It decides nothing. Nothing resolves differently because of it; `null` here + * only means the diagnostic omits a replacement. + */ +export function liveIconNameOf(icon: unknown): string | null { + if (!icon) return null; + const record = icons as Record; + for (const [key, component] of Object.entries(record)) { + if (component === icon) return key; + } + const declared = (icon as { displayName?: unknown }).displayName; + if (typeof declared === 'string' && Object.prototype.hasOwnProperty.call(record, declared)) { + return declared; + } + return null; +} diff --git a/scripts/__tests__/check-eager-closure-budget.test.ts b/scripts/__tests__/check-eager-closure-budget.test.ts index 6d2639f900..89858343b5 100644 --- a/scripts/__tests__/check-eager-closure-budget.test.ts +++ b/scripts/__tests__/check-eager-closure-budget.test.ts @@ -859,7 +859,20 @@ describe('ceiling sensitivity, judged live (objectui#5924)', () => { */ describe('a declared row', () => { const CEILING = PER_CHUNK_GZIP_CEILINGS['ui-components']; - const ALLOWANCE = EXHAUSTED_HEADROOM_ALLOWANCES['ui-components']; + /** + * ⭐ A FIXTURE allowance, not a live one — `EXHAUSTED_HEADROOM_ALLOWANCES` + * has been empty since objectui#9204 paid `ui-components` off, and an + * empty table would make every row below vacuous. A mechanism nobody + * exercises is a mechanism nobody can trust the next time a row has to be + * declared, and this describe block is the only place the ratchet's + * hinge is taken at the byte. + * + * 4,289 is kept deliberately: it is the figure `ui-components` actually + * carried, so these rows still sit on a measurement rather than on a + * round number invented for a test. + */ + const ALLOWANCE = 4_289; + const DECLARED = { 'ui-components': ALLOWANCE }; const GRAIN = REGRESSION_THIS_GATE_MUST_CATCH_BYTES * EXHAUSTED_HEADROOM_ALLOWANCE_GRANULARITY_MULTIPLE; @@ -867,6 +880,7 @@ describe('ceiling sensitivity, judged live (objectui#5924)', () => { const atHeadroom = (headroom: number) => evaluateHeadroomSensitivity({ report: sensitivityReport(BASELINE.gzipBytes, { 'ui-components': CEILING - headroom }), + allowances: DECLARED, }); it('is held open at its pinned figure', () => { @@ -935,7 +949,7 @@ describe('ceiling sensitivity, judged live (objectui#5924)', () => { const at = (headroom: number, allowance: number) => evaluateHeadroomSensitivity({ report: sensitivityReport(BASELINE.gzipBytes, { 'ui-components': CEILING - headroom }), - allowances: { ...EXHAUSTED_HEADROOM_ALLOWANCES, 'ui-components': allowance }, + allowances: { 'ui-components': allowance }, }).status; expect(at(Math.floor(paidDown - GRAIN), paidDown)).toBe('error'); @@ -946,17 +960,34 @@ describe('ceiling sensitivity, judged live (objectui#5924)', () => { it('names every declared row in the PASSING verdict, not only when one fires', () => { // A debt list that is only legible on the run that reds is the parenthetical - // this card is about: noticing stays manual, and it already failed twice. + // objectui#8554 is about: noticing stays manual, and it already failed twice. + // Driven from a FIXTURE table because the live one is empty (objectui#9204) + // — the renderer's job does not go away with the last declared row. + const declared = { 'ui-components': 4_289 }; const result = evaluateHeadroomSensitivity({ report: sensitivityReport(BASELINE.gzipBytes), + allowances: declared, }); expect(result.status).toBe('pass'); - for (const [name, allowance] of Object.entries(EXHAUSTED_HEADROOM_ALLOWANCES)) { + for (const [name, allowance] of Object.entries(declared)) { expect(result.message).toContain(`chunk \`${name}\``); expect(result.message).toContain(`declared ${allowance}-byte allowance`); } }); + it('says nothing about allowances when the table is empty — the live state', () => { + // The control for the row above, and the assertion the empty table earns: + // with nothing declared, no row may carry the declared-allowance clause, + // because that clause asserts a row is under the floor. + const result = evaluateHeadroomSensitivity({ + report: sensitivityReport(BASELINE.gzipBytes), + allowances: EXHAUSTED_HEADROOM_ALLOWANCES, + }); + expect(result.status).toBe('pass'); + expect(result.message).not.toContain('declared'); + expect(result.message).not.toContain('held open by'); + }); + /** * The allowance table is a RATCHET, and the whole of its ratchet-ness is * that these numbers can only be paid down. Nothing in the runtime can @@ -964,23 +995,31 @@ describe('ceiling sensitivity, judged live (objectui#5924)', () => { * enforcement: an edit in either direction has to come here and be argued. */ describe('the allowance table is a ratchet, pinned', () => { - it('holds exactly the rows measured under the floor on the day it landed', () => { - // ⚠️ `i18n-locales: 8_804` was here until objectui#7479 and is REMOVED, - // not lowered: its chunk ceased to exist when nine of the ten - // catalogues became `import()`ed, and the one that stays is budgeted - // under `i18n-locale-en` at a headroom ABOVE the floor, needing no - // allowance. That is the only way a row leaves this table. - expect(EXHAUSTED_HEADROOM_ALLOWANCES).toEqual({ - 'ui-components': 4_289, - }); + it('is EMPTY — both rows it carried were paid off, never lowered', () => { + // ⚠️ Two removals, one rule. `i18n-locales: 8_804` left in objectui#7479 + // when nine of its ten catalogues became `import()`ed and the one that + // stays cleared the floor under `i18n-locale-en`. `ui-components: 4_289` + // left in objectui#9204 when lucide's dynamic-import map left the eager + // path and the row went 397,090 -> 388,575 gzipped, headroom 0.02x -> + // 0.11x. Neither figure was LOWERED — a lowered figure is headroom + // supplied to a row that still needs it, which is the one edit this + // table forbids. Clearing the floor on its own is the only way out. + expect(EXHAUSTED_HEADROOM_ALLOWANCES).toEqual({}); }); it('every entry is real debt — strictly under the floor it excuses', () => { // An allowance at or above the floor is not debt, it is a second floor - // for one row, and the row should simply have been dropped from here. + // for one row — and a WEAKER one, which is why `ui-components` had to + // leave rather than be re-pinned upward once it cleared the floor. for (const allowance of Object.values(EXHAUSTED_HEADROOM_ALLOWANCES)) { expect(allowance).toBeLessThan(FLOOR); } + // ⭐ The control. The loop above is vacuous while the table is empty, and + // a vacuous loop reads exactly like a satisfied one. This states the + // predicate on the figure the table actually carried and on the one it + // could not: 4,289 was admissible, the floor itself never is. + expect(4_289).toBeLessThan(FLOOR); + expect(FLOOR).not.toBeLessThan(FLOOR); }); it('is compared at the coarser of the two grids this gate renders on', () => { @@ -1004,10 +1043,12 @@ describe('ceiling sensitivity, judged live (objectui#5924)', () => { expect(grain).toBeGreaterThan(1); expect(grain).toBeLessThan(floor); // Every declared row must still have a reachable trip point above zero, - // or its entry would be decorative. + // or its entry would be decorative. Vacuous while the table is empty, so + // the figure the table last carried stands as the control. for (const allowance of Object.values(EXHAUSTED_HEADROOM_ALLOWANCES)) { expect(allowance - grain).toBeGreaterThan(0); } + expect(4_289 - grain).toBeGreaterThan(0); }); it('every entry names a ceiling that exists', () => { diff --git a/scripts/__tests__/check-lucide-icon-record-names.test.ts b/scripts/__tests__/check-lucide-icon-record-names.test.ts index 8342f1ee93..8f943e19ce 100644 --- a/scripts/__tests__/check-lucide-icon-record-names.test.ts +++ b/scripts/__tests__/check-lucide-icon-record-names.test.ts @@ -7,6 +7,7 @@ import { fileURLToPath } from 'node:url'; import { ANCHORED_MAPS, DECLARED_DYNAMIC_READERS, + DECLARED_EAGER_DYNAMIC_IMPORTERS, DECLARED_RECORD_READERS, DISCOVERY_NEGATIVE_CONTROL, RECORD_READING_TYPES, @@ -107,6 +108,7 @@ interface FixtureOptions { anchors?: typeof ANCHORED_MAPS; declaredRecordReaders?: string[]; declaredDynamicReaders?: string[]; + declaredEagerDynamicImporters?: string[]; negativeControl?: string; recordReadingTypes?: CensusTable; } @@ -146,6 +148,7 @@ function judge(label: string, options: FixtureOptions) { anchors: options.anchors ?? [], declaredRecordReaders: options.declaredRecordReaders ?? [RESOLVER_FILE], declaredDynamicReaders: options.declaredDynamicReaders ?? [], + declaredEagerDynamicImporters: options.declaredEagerDynamicImporters, negativeControl: options.negativeControl, recordReadingTypes: options.recordReadingTypes ?? FIXTURE_TYPES, }); @@ -721,6 +724,11 @@ describe('the surface census is re-derived on every run', () => { // Getting this backwards is worse than having no gate: the dynamic list // still carries `edit`, so a gate pointed at it would bless the exact names // this class is about. + // + // The static spelling is deliberate here and so is the allowance beside it: + // this row is about WHICH vocabulary the site reads, and the eager-import + // rule below is about HOW it reaches it. Keeping them apart is what lets + // either fail alone. const result = judge('dynamic', { files: { 'packages/app/src/lazy.ts': [ @@ -729,6 +737,7 @@ describe('the surface census is re-derived on every run', () => { ].join('\n'), }, declaredDynamicReaders: ['packages/app/src/lazy.ts'], + declaredEagerDynamicImporters: ['packages/app/src/lazy.ts'], }); expect(result.errors).toEqual([]); @@ -736,6 +745,46 @@ describe('the surface census is re-derived on every run', () => { expect(result.discovered.record).toEqual([RESOLVER_FILE]); }); + // ── objectui#9204: HOW a site reaches the dynamic surface is censused too ── + + it('sees the DEFERRED spelling — an `import()` still reads the vocabulary', () => { + // The census exists so a site cannot move between surfaces unnoticed. + // Moving the map behind `import()` must not read as "stopped reading it": + // that would retire the entry and leave the next static import undeclared + // AND unnoticed. + const result = judge('deferred', { + files: { + 'packages/app/src/deferred.ts': [ + "export const load = () => import('lucide-react/dynamic.mjs');", + ].join('\n'), + }, + declaredDynamicReaders: ['packages/app/src/deferred.ts'], + }); + + expect(result.errors).toEqual([]); + expect(result.discovered.dynamic).toEqual(['packages/app/src/deferred.ts']); + expect(result.discovered.eagerDynamic).toEqual([]); + }); + + it('fails on a STATIC import of the dynamic entry — that is the whole map', () => { + // lucide derives `iconNames` from `dynamicIconImports`, so this import puts + // the 2,025-entry map in the importer's chunk. Nothing else in the tree + // reddens: the laziness is in the source and the cost is in a bundle. + const result = judge('eager-dynamic', { + files: { + 'packages/app/src/eager.ts': [ + "import { iconNames } from 'lucide-react/dynamic.mjs';", + 'export const known = new Set(iconNames as string[]);', + ].join('\n'), + }, + declaredDynamicReaders: ['packages/app/src/eager.ts'], + }); + + expect(result.violations).toEqual([]); + expect(result.discovered.eagerDynamic).toEqual(['packages/app/src/eager.ts']); + expect(result.errors.join('\n')).toContain('EAGER `lucide-react/dynamic` import: packages/app/src/eager.ts'); + }); + it('matches the IMPORT, not the name — a local `icons` object is not a resolver', () => { // The blind-probe control for discovery. `plugin-chatbot/src/elements/tool.tsx` // is the live specimen: it builds its own `icons` map of ReactNodes and @@ -944,6 +993,18 @@ describe('this repository', () => { expect(repoResult.discovered.dynamic).toEqual([...DECLARED_DYNAMIC_READERS].sort()); }); + it('has NO module importing lucide\'s dynamic entry statically', () => { + // objectui#9204's whole deliverable, stated where it can go red: the + // declared allowance is empty, and so is what discovery finds. + expect(DECLARED_EAGER_DYNAMIC_IMPORTERS).toEqual([]); + expect(repoResult.discovered.eagerDynamic).toEqual([]); + + // …and the surface it guards has not evaporated. Two lists that are both + // empty because discovery stopped working read exactly like a clean tree, + // so the dynamic census is shown non-empty first. + expect(repoResult.discovered.dynamic.length).toBeGreaterThan(0); + }); + it('does not mistake the live local-`icons` specimen for a resolver', () => { expect(fs.existsSync(path.join(repoRoot, DISCOVERY_NEGATIVE_CONTROL))).toBe(true); expect(repoResult.discovered.record).not.toContain(DISCOVERY_NEGATIVE_CONTROL); diff --git a/scripts/check-eager-closure-budget.mjs b/scripts/check-eager-closure-budget.mjs index 907ab61c7f..d5dcb87af1 100644 --- a/scripts/check-eager-closure-budget.mjs +++ b/scripts/check-eager-closure-budget.mjs @@ -1186,7 +1186,27 @@ export const PER_CHUNK_BASELINE = Object.freeze({ // BASELINE's. Moved with the ceiling in the same commit, per the maintainer // ruling of 2026-09-08 and the rule stated under "Raising one". framework: 72_245, - 'ui-components': 391_095, + // ⭐ RE-PINNED DOWN by objectui#9204, on its OWN console build at `d43f51ee1` + // — ⛔ not with a ceiling move, and that exception is the whole reason this + // line has a comment. It supersedes `2c8474c04`'s 391,095 (objectui#5490). + // + // The ceiling did NOT move and must not: 399,000 still stands over this row, + // unchanged since #5490. What moved is the PAYLOAD — lucide's dynamic-import + // map left the eager path when icon-name membership moved onto the `icons` + // record this chunk already carries (maintainer ruling of 2026-09-13). The + // row went 397,090 -> 388,575 gzipped, measured on two console builds in one + // container, and the aggregate fell 8,520 against the row's 8,515, which is + // what makes it bytes LEAVING the page load rather than moving between + // columns. + // + // ⚠️ Re-pinning was not optional here, and ⛔ not cosmetic. This constant is + // what the unit test builds its sensitivity reports from, so leaving it at + // 391,095 while `ui-components` left {@link EXHAUSTED_HEADROOM_ALLOWANCES} + // would have made the gate's own fixtures assert a row 1,209 bytes UNDER the + // floor that the live build clears by 1,311 — the allowance and this figure are the + // pair that has to move in one commit, exactly as a ceiling and its baseline + // do under "Raising one". + 'ui-components': 388_575, }); /** @@ -1293,17 +1313,50 @@ export const EXHAUSTED_HEADROOM_FLOOR_MULTIPLE = 0.1; * {@link EXHAUSTED_HEADROOM_ALLOWANCE_GRANULARITY_MULTIPLE}, which is the unit * the comparison is made in and the reason a red here is a red a reader can see. */ +/** + * ⚠️ Annotated rather than inferred, and objectui#9204 is why. While this table + * carried a row, `Object.values` of it was `number[]`; the moment the last row + * was paid off, `Object.freeze({})` inferred `Readonly<{}>` and that same call + * became `unknown[]` — so a reader doing arithmetic on an allowance stopped + * compiling because the table was EMPTY, not because anything about it was + * wrong. The element type is a property of what this table holds, not of how + * many rows it holds today. + * + * @type {Readonly>} + */ export const EXHAUSTED_HEADROOM_ALLOWANCES = Object.freeze({ - // ⭐ `i18n-locales: 8_804` stood here until objectui#7479. It is REMOVED, not - // lowered, and the distinction is the whole of why that is allowed: the rule - // above forbids LOWERING a figure, because a lowered figure is headroom - // supplied to a row that still exists. This row's chunk does not exist any - // more — nine of the ten catalogues it weighed are `import()`ed on demand, and - // the one that stays is budgeted under its own key at a headroom of 0.11x, - // ABOVE the floor and needing no allowance at all. That is the debt PAID, in - // the only currency this table takes: the row cleared the floor on its own. - // ⛔ Re-adding a locale row here would mean the catalogues came back. - 'ui-components': 4_289, + // ⭐ EMPTY, and both rows that stood here left the same way — REMOVED, never + // lowered. The rule above forbids lowering a figure because a lowered figure + // is headroom supplied to a row that still exists; a removal is the opposite, + // because the row it excused now clears the floor on its own and goes back to + // being judged at 0.10x like every other. + // + // `i18n-locales: 8_804` left in objectui#7479: nine of the ten catalogues it + // weighed became `import()`ed on demand, and the one that stays is budgeted + // under `i18n-locale-en` at 0.11x. ⛔ Re-adding a locale row would mean the + // catalogues came back. + // + // `ui-components: 4_289` left in objectui#9204, and this is the case the + // paragraph above was written for — the debt PAID in the only currency this + // table takes. lucide's dynamic-import map left the console's eager path when + // icon-name membership moved onto the `icons` record that chunk already + // carries (maintainer ruling of 2026-09-13), taking the row from 397,090 to + // 388,575 gzipped and its headroom from 1,910 B (0.02x) to 10,425 B (0.11x), + // measured on two console builds in one container at `d43f51ee1`. + // + // ⚠️ ⭐ Removal was the ONLY legal move once that landed, and the unit test + // says so rather than this comment: "every entry is real debt — strictly + // under the floor it excuses". A figure at or above the floor is not debt, it + // is a second floor for one row — and a WEAKER one, since this row's declared + // trip point was 3,378 B against the floor's 9,114 B. Paying the row down and + // leaving the entry would have LOOSENED the gate on the chunk the payment was + // for, while the verdict text kept printing "under the 0.10x floor" about a + // row at 0.12x. + // + // ⛔ Nothing here was raised, ⛔ no ceiling moved, and ⛔ the table is not a + // supply of headroom now that it is empty: an undeclared row reds at 0.10x, + // which is what `ui-components` is judged at from here on — with 1,311 B of + // margin over the floor, a figure this card reports rather than pads. }); /** diff --git a/scripts/check-lucide-icon-record-names.mjs b/scripts/check-lucide-icon-record-names.mjs index 3d38830368..bf846285cd 100644 --- a/scripts/check-lucide-icon-record-names.mjs +++ b/scripts/check-lucide-icon-record-names.mjs @@ -41,6 +41,29 @@ * judged here; the dynamic sites are censused (below) precisely so that the * split stays declared and a site cannot move between surfaces unnoticed. * + * ── HOW a site reaches DYNAMIC is also censused (objectui#9204) ───────────── + * lucide derives `iconNames` as `Object.keys(dynamicIconImports)`, so importing + * the names imports the dynamic-import map with them. Four modules did, and the + * map — 8,253 B gzipped, measured — sat in the console's eager `ui-components` + * chunk on every page load. All four are gone: two were transcriptions of + * `getLazyIcon` and are now delegations, the icon picker asks + * `@object-ui/components` for the vocabulary, and the one module left reaches + * the map through `import()`. + * + * ⚠️ A GENERATED MIRROR OF THE NAMES IS NOT THE FIX, and the number is why: the + * map's KEYS are the names, so a bare catalogue of them measured 9,176 B gzipped + * spliced into that same chunk against the 8,253 B map it replaced (a + * front-coded encoding probed and rejected at 8,397 B). The names left the eager + * path by being ANSWERED FROM THE RECORD instead — the maintainer ruling of + * 2026-09-13, which also retires the 254 spellings the two vocabularies disagree + * about. + * + * That makes two spellings discovery has to see — a static import and an + * `import()` — and gives this gate a second census: + * `DECLARED_EAGER_DYNAMIC_IMPORTERS`, which is EMPTY. A static import restores + * the map to the first payload while every other check stays green, because the + * laziness lives in the source and the cost lives in a bundle. Here they meet. + * * ── What it checks (three parts, each self-verifying) ─────────────────────── * 1. SURFACE CENSUS — rediscovers, from source, every module that reads either * vocabulary, and fails when the discovered set differs from the declared @@ -297,13 +320,50 @@ export const DECLARED_RECORD_READERS = [ 'packages/components/src/renderers/action/resolve-icon.ts', ]; +/** + * ⭐ ONE entry, and the one is the module that OWNS the deferred map. + * + * Until objectui#9204 this list had four. Two were transcriptions of + * `getLazyIcon` and are now delegations to it; the fourth, the metadata + * designer's icon picker, asks `@object-ui/components` for the vocabulary + * instead of importing `iconNames` itself. What is left reaches the surface + * only through `import('lucide-react/dynamic.mjs')`, and it reads it for the + * SPELLINGS — which glyph is LIVE is answered by the record, above. + */ export const DECLARED_DYNAMIC_READERS = [ - 'apps/console/src/utils/getIcon.ts', - 'packages/app-shell/src/utils/getIcon.ts', - 'packages/app-shell/src/views/metadata-admin/widgets.tsx', 'packages/components/src/lib/lazy-icon.tsx', ]; +/** `lucide-react/dynamic`, `lucide-react/dynamic.mjs`, `…/dynamic.js`. */ +export const isDynamicEntrySpecifier = (specifier) => specifier.startsWith('lucide-react/dynamic'); + +/** + * Modules allowed to reach `lucide-react/dynamic*` through a STATIC import. + * + * ⛔ Empty, and that is the assertion (objectui#9204). lucide derives + * `iconNames` as `Object.keys(dynamicIconImports)`, so a static import of + * EITHER export puts the whole dynamic-import map in the importer's chunk + * — 8,253 B gzipped of the console's eager `ui-components` chunk, measured on + * the emitted artifact across three builds. Four modules imported it that way; + * membership now comes from the `icons` record and the map loads through + * `import()` on the first icon that renders. + * + * Nothing else in the tree goes red when that regresses: the laziness is in the + * source, the cost is in a bundle, and the eager-closure budget only reports the + * total. This list is where the two meet — a static import here is named on the + * commit that adds it, rather than a kilobyte reading on a ceiling weeks later. + */ +/** + * ⚠️ Annotated rather than inferred. An empty array literal infers `never[]`, + * so the element type of this census would be erased by the very emptiness that + * IS its assertion — and the next reader of it would get `never` instead of a + * path. The declared type is the one this list carries when a row has to be + * added, not the one it happens to have while it is empty. + * + * @type {readonly string[]} + */ +export const DECLARED_EAGER_DYNAMIC_IMPORTERS = []; + /** * A module that builds its OWN `icons` object and indexes it is not a lucide * resolver. `plugin-chatbot/src/elements/tool.tsx` does exactly that, which @@ -787,6 +847,7 @@ function objectProp(objectLiteral, name) { export function discoverResolvers(root, files) { const record = []; const dynamic = []; + const eagerDynamic = []; for (const file of files) { if (isTestPath(file)) continue; const text = readFileSync(join(root, file), 'utf8'); @@ -794,18 +855,38 @@ export function discoverResolvers(root, files) { const sf = parseSource(root, file); let recordLocal = null; let readsDynamic = false; + let importsDynamicStatically = false; sf.forEachChild((node) => { if (!ts.isImportDeclaration(node) || !ts.isStringLiteral(node.moduleSpecifier)) return; const specifier = node.moduleSpecifier.text; - if (specifier.startsWith('lucide-react/dynamic')) readsDynamic = true; - if (specifier !== 'lucide-react') return; + if (isDynamicEntrySpecifier(specifier)) { + readsDynamic = true; + importsDynamicStatically = true; + } const bindings = node.importClause?.namedBindings; if (!bindings || !ts.isNamedImports(bindings)) return; for (const element of bindings.elements) { - if ((element.propertyName ?? element.name).text === 'icons') recordLocal = element.name.text; + const imported = (element.propertyName ?? element.name).text; + if (specifier === 'lucide-react' && imported === 'icons') recordLocal = element.name.text; } }); + // `import('lucide-react/dynamic.mjs')` — the DEFERRED spelling, invisible to + // the import-declaration walk above and the whole point of objectui#9204. + // A census that could not see it would report the map's one remaining + // reader as having stopped reading the surface entirely. + const visitCalls = (node) => { + if ( + ts.isCallExpression(node) + && node.expression.kind === ts.SyntaxKind.ImportKeyword + && node.arguments.length > 0 + && ts.isStringLiteralLike(node.arguments[0]) + && isDynamicEntrySpecifier(node.arguments[0].text) + ) readsDynamic = true; + ts.forEachChild(node, visitCalls); + }; + ts.forEachChild(sf, visitCalls); if (readsDynamic) dynamic.push(file); + if (importsDynamicStatically) eagerDynamic.push(file); if (!recordLocal) continue; let indexes = false; const visit = (node) => { @@ -818,7 +899,7 @@ export function discoverResolvers(root, files) { ts.forEachChild(sf, visit); if (indexes) record.push(file); } - return { record: record.sort(), dynamic: dynamic.sort() }; + return { record: record.sort(), dynamic: dynamic.sort(), eagerDynamic: eagerDynamic.sort() }; } // ── Part 2: authored nodes ─────────────────────────────────────────────────── @@ -1036,10 +1117,19 @@ function judgeAnchoredMaps(root, anchors) { * * @typedef {{ paths: string[], resolver: string, descendants?: boolean, min?: number }} RecordReadingType * + * ⚠️ EVERY census this function reads has to appear here, and objectui#9204 + * proved that the hard way: `declaredEagerDynamicImporters` was implemented, + * defaulted and used below while this typedef did not list it, so the unit + * test's fixture census was a type error at the CALL SITE and the declaration + * that was actually wrong sat in a `.mjs` that `tsconfig.scripts.json` does not + * check (`checkJs: false`). The error therefore lands on the caller, which is + * the one place the omission is not. + * * @typedef {{ * anchors?: readonly any[], * declaredRecordReaders?: readonly string[], * declaredDynamicReaders?: readonly string[], + * declaredEagerDynamicImporters?: readonly string[], * negativeControl?: string, * recordReadingTypes?: Record, * }} AnalyzeOptions @@ -1051,6 +1141,7 @@ export function analyze(root, { anchors = ANCHORED_MAPS, declaredRecordReaders = DECLARED_RECORD_READERS, declaredDynamicReaders = DECLARED_DYNAMIC_READERS, + declaredEagerDynamicImporters = DECLARED_EAGER_DYNAMIC_IMPORTERS, negativeControl = DISCOVERY_NEGATIVE_CONTROL, recordReadingTypes = RECORD_READING_TYPES, } = {}) { @@ -1075,6 +1166,18 @@ export function analyze(root, { censusDiff('dynamic-surface resolver', discovered.dynamic, declaredDynamicReaders, 'It resolves names through `lucide-react/dynamic.mjs`, which still carries retired spellings — a second, more forgiving vocabulary.'); + for (const file of discovered.eagerDynamic) { + if (declaredEagerDynamicImporters.includes(file)) continue; + errors.push( + `EAGER \`lucide-react/dynamic\` import: ${file}\n` + + ' lucide derives `iconNames` from `dynamicIconImports`, so a STATIC import of either name puts the\n' + + ' whole dynamic-import map in this module\'s chunk — 8,253 B gzipped on the console\'s eager\n' + + ' path (objectui#9204). Ask `isLucideIconName` / `loadLucideIconNames` (@object-ui/components) instead:\n' + + ' membership comes from the `icons` record and the map loads through `import()`, the way\n' + + ' `packages/components/src/lib/lazy-icon.tsx` does.', + ); + } + if (discovered.record.length === 0) { errors.push('discovery found NO record-reading resolver at all — it is not matching imports any more, and every "no violations" below is vacuous.'); } @@ -1115,6 +1218,8 @@ if (invokedDirectly) { for (const file of discovered.record) console.log(` ${file}`); console.log(`dynamic-surface resolvers discovered (${discovered.dynamic.length}), NOT judged here:`); for (const file of discovered.dynamic) console.log(` ${file}`); + console.log(`modules importing \`lucide-react/dynamic*\` STATICALLY (${discovered.eagerDynamic.length}; every one puts the import map on the eager path):`); + for (const file of discovered.eagerDynamic) console.log(` ${file}`); console.log(`authored icon names judged: ${counters.authoredJudged} (${counters.authoredDescendantJudged} of them on UNTYPED child items of a declared container) | icon names on nodes this gate declines to judge: ${counters.authoredDeclined}`); console.log(`anchored map entries judged: ${counters.anchoredJudged}`); console.log('');