Skip to content

refactor(tooling): build the lucide gate's two lookup maps lazily, taking rule 2's baseline to zero - #6156

Merged
yinlianghui-tw merged 2 commits into
mainfrom
claude/issue-6147-lucide-lazy-maps
Aug 24, 2026
Merged

yinlianghui-tw merged 2 commits into
mainfrom
claude/issue-6147-lucide-lazy-maps

Conversation

@yinlianghui-tw

@yinlianghui-tw yinlianghui-tw commented Aug 24, 2026 •

Copy link
Copy Markdown
Collaborator

Fixes #6147
Closes #6092

scripts/check-lucide-icon-record-names.mjs built its two lookup maps in top-level for loops, so importing the module for its exports ran them. It was the sole entry in check-entry-guard.mjs's KNOWN_IMPORT_UNSAFE. They are now built lazily on first read inside liveSpellingFor, memoised, and that baseline goes to zero.

With #6145 merged (it emptied KNOWN_HAND_TYPED_GUARDS) and this PR emptying KNOWN_IMPORT_UNSAFE, both of #6092's shrink-only baselines are now at zero, which is that card's last open item. The merged-head verdict line says so in one sentence:

✓ check:entry-guard: 42 scripts/ file(s) — no entry guard outside the baseline; 0 file(s) still
  hand-type one (0 occurrence(s), ⛔ SHRINK-ONLY, objectui#6092); 37 export bindings, 37 of them
  inert on import (0 known-unsafe, ⛔ SHRINK-ONLY).

Everything below was re-measured at the merged head cbeabc28c, working tree clean.

The remedy this is not

The obvious fix — move the loops behind the entry guard — was measured and rejected by #6092's PR 2, and this PR does not revisit that. Guarding them turns rule 2 green and reports its own baseline line as STALE, and fails the importing suite 5 of 25, because the maps are then empty for importers: describeName('BarChart3') stops naming the live spelling and says "no live key names the same glyph" instead — a wrong diagnosis for a real violation, from a gate that still exits 1. A green baseline bought with a lying error message is worse than an honest debt line.

Lazy satisfies both sides: nothing runs at module top level, so rule 2's line can be deleted honestly, and every importer gets exactly what it got before.

Behaviour preservation — the thing that was actually at stake

The rejected ablation broke one specific message, so that message is the measurement. Same probe, on the pre-change tree and at the merged head:

describeName(BarChart3)="\"BarChart3\" -> `BarChart3` is not a key of the runtime `icons` record.
lucide keeps it only as a DEPRECATED EXPORT of the same glyph — write `chart-column`
(the spelling the record carries)."

The whole probe output (five calls: describeName on a retired alias, a second retired alias, a non-icon, and liveSpellingFor twice) is byte-identical, same sha256 94050e0c5ffd… before the change, after the change, and after the merge. It still says write `chart-column` .

The CLI leg is byte-identical too — the gate run against origin/main's version of the file and against this one both produce sha256 331ca92069…, exit 0:

OK  lucide icon names: 64 authored/declared names reaching 8 record-reading resolvers are live
`icons` keys (record 1767 keys; dynamic surface 4 sites, 2025 names, not judged here).

The import leg prints 0 bytes.

Non-vacuity on the laziness itself

A memoisation that runs eagerly anyway would pass every test above while changing nothing, so the laziness is measured two independent ways. Both were re-run at the merged head, because a merge that resolves cleanly can still change behaviour.

1. A non-mutating probe wraps Map.prototype.set and counts per phase. The two builds are 1767 + 2025 = 3792 sets, so where they land is unmissable. The same probe was run against origin/main's version of the file — which is post-#6145, so it carries the isEntrypoint guard and the eager loops (mutation confirmed on disk: top-level const keyByComponent 0 to 1, function glyphLookups 1 to 0, isEntrypoint(import.meta.url) still 1; restored by trap … EXIT INT TERM with git checkout HEAD --):

phase eager (origin/main) lazy (this PR, merged head)
after import delta 6147 delta 2355
after 1st liveSpellingFor delta 0 delta 3792
after 2nd liveSpellingFor delta 0 delta 0
after describeName delta 0 delta 0

6147 minus 2355 is exactly 3792. Same total work, moved off the import path — and the probe demonstrably reports an eager tree as eager, so it is not vacuous. Identical numbers before and after the merge.

2. Direct instrumentation of the build body (injected marker confirmed on disk = 1, restored by trap, marker remaining = 0):

[phase] module imported, control returned
[phase] calling liveSpellingFor #1
[BUILD] glyphLookups body entered — building both maps now
[phase] calling liveSpellingFor #2
[phase] calling describeName
[result] "BarChart3" -> … write `chart-column` (the spelling the record carries).

The build does not run at import; it runs on the first read; it runs once.

The emptied baseline is still consulted

An empty debt list that had also stopped being read would look identical to this one. Both directions, re-run at the merged head, each confirmed on disk first and each restored by trap:

mutation on-disk confirmation gate
re-add the line to the empty set empty form 1 to 0, repopulated form present exit 1 — 1 stale KNOWN_IMPORT_UNSAFE entry/entries
list stays empty, file back to origin/main (eager) top-level const keyByComponent = 1, list still empty exit 1 — 1 scripts/ file(s) export bindings AND run on import, naming :243 and :245

Those are the card's own line numbers, which it stated as correct "on main after #6092's PR 2" — and post-merge they are.

The set is kept as an empty Set rather than deleted, and its comment now records which remedy emptied it — because the rejected one empties it too.

The merge, and how the conflict was resolved

#6145 merged at 21:45 and origin/main was merged in (⛔ never rebased, never force-pushed — the push was a fast-forward 804afaa66..cbeabc28c).

Verification, each quoting its own verdict line

All at merged head cbeabc28c; exit codes captured before any pipe.

node scripts/check-entry-guard.mjs --self-test        exit=0
  ✓ check-entry-guard self-test: 63 cases pass — … and the import-safety rule recognised on both sides.
node scripts/check-entry-guard.mjs                    exit=0
  ✓ check:entry-guard: 42 scripts/ file(s) … 0 file(s) still hand-type one (0 occurrence(s) …);
    37 export bindings, 37 of them inert on import (0 known-unsafe, ⛔ SHRINK-ONLY).
node scripts/invoked-as.mjs --self-test               exit=0
  ✓ invoked-as self-test: 12 cases pass (real symlink, different-name symlink, percent-encoding …
node scripts/check-lucide-icon-record-names.mjs       exit=0
  OK  lucide icon names: 64 authored/declared names … are live `icons` keys (record 1767 keys …).
node scripts/check-control-bytes.mjs                  exit=0
  ✅  check-control-bytes: OK (scanned 5098 tracked text file(s); skipped 85 binary).
node scripts/check-changeset-presence.mjs             exit=0
  ✅  No source of a released package changed in this range, so no changeset is owed.
npx vitest run scripts/__tests__/ --maxWorkers=2      exit=0
  Test Files  70 passed (70) · Tests  1914 passed (1914)     # repo ROOT, never package-scoped (#3378)
pnpm type-check:scripts                               exit=0     # tsc -p tsconfig.scripts.json, no diagnostics
pnpm lint:root                                        exit=0     # the FULL root scan covering scripts/
  ✖ 28 problems (0 errors, 28 warnings)                          # all pre-existing

check-lucide-icon-record-names.test.ts on its own: 25 passed (25) — before the change, after the change, and after the merge. The same 25 the rejected ablation took to 20.

No lint narrowing was needed: pnpm lint:root ran the full population. Its 28 warnings are all pre-existing — eslint --format json over the two changed files reports exactly 2 file objects, errorCount=0 and warningCount=0 each, so none of the 28 is in this diff.

Control bytes: the repo scanner is green above, and grep -naP for the control-byte class over both changed files returns no matches.

⛔ Not widened

icons and iconNames are themselves top-level await import('lucide-react'), so this module is never cheap to import. This PR removes the two loops, not that cost, and deliberately does not restructure the four top-level awaits — that is a separate and larger question, and touching it here would repeat the mistake #6092 stopped to avoid. Both the file's comment and this description say so.

Not done, and not owed by this seat: CI convergence. The PM verifies the real gate jobs and lands it; this stays a draft.


Generated by Claude Code

…t at import

`check-lucide-icon-record-names.mjs` built `keyByComponent` and `kebabByKey` in
two top-level `for` loops, so importing the module for its exports ran them.
That was the sole entry in `check-entry-guard.mjs`'s `KNOWN_IMPORT_UNSAFE`.

The obvious remedy — move the loops behind the entry guard — was measured and
rejected: it leaves the maps empty for importers, so `describeName('BarChart3')`
prints a WRONG diagnosis for a real violation from a gate that still exits 1.

They are now built on first read inside `liveSpellingFor`, memoised. Nothing
runs at module top level, every importer gets what it got before, and the CLI
output is byte-identical. `KNOWN_IMPORT_UNSAFE` goes to zero in the same commit,
because the baseline is shrink-only in both directions.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019b5UBNMtTzKbVtZZGvFuxe
…ide-lazy-maps

# Conflicts:
#	scripts/check-entry-guard.mjs
@yinlianghui-tw
yinlianghui-tw marked this pull request as ready for review August 24, 2026 22:13
@yinlianghui-tw
yinlianghui-tw added this pull request to the merge queue Aug 24, 2026
Merged via the queue into main with commit 94bc6e5 Aug 24, 2026
22 checks passed
@yinlianghui-tw
yinlianghui-tw deleted the claude/issue-6147-lucide-lazy-maps branch August 24, 2026 22:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment