refactor(dashboard): pass lucide icons as values instead of name strings - #649
Merged
Conversation
Follow-up to #648. Icon took a kebab-case name and resolved it through a curated registry, so an unregistered name rendered a blank SVG instead of failing — exactly the EmptyState "inbox" bug found in review there. Callers now import the icon and pass it (<Icon icon={Pencil} />), which deletes the registry: there is no list to keep in sync, and tree-shaking follows the imports structurally rather than from an enumerated map. Config-driven icons (sidebar nav, theme toggle, confirm dialogs, budget periods) hold the icon itself instead of a name. The bundler does not warn on missing named exports and jsconfig runs with checkJs:false, so a typo would still slip through silently. A guard test resolves every lucide import in the source tree against the package, failing with the offending file name. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Contributor
📝 WalkthroughWalkthroughThe dashboard migrates icon rendering from string-based registry names to imported Lucide icon data passed through ChangesDashboard icon migration
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@web/dashboard/src/lib/components/organisms/EditorDialog.svelte`:
- Line 46: Update all remaining callers of EditorDialog's submitIcon prop to
pass Lucide icon data instead of strings. In AuthKeyEditor.svelte,
VirtualModelEditor.svelte, WorkflowEditor.svelte, and other editor components
(AuthKeyLabelsEditor, MCP, Tenant, Rate-limit, Policy editors), replace the
string values "plus", "save", "check", and "delete" with their corresponding
imported Lucide icons (Plus, Save, Check, Trash respectively). Import each
needed Lucide icon at the top of each file and update the submitIcon assignments
to use the imported icon data in the same format that Save is now used in
EditorDialog.svelte.
In `@web/dashboard/tests/icons.test.js`:
- Around line 31-36: Update importedIcons() to parse aliased named imports by
extracting the exported Lucide name before the “as” clause, so lucide[name] uses
Pencil rather than the local alias EditIcon. Preserve existing handling for
non-aliased imports and apply the same normalization in the related logic at
lines 48–49.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 0db7c7e1-8556-4f56-90fa-96f3d8c8f394
⛔ Files ignored due to path filters (3)
internal/admin/dashboard/static/dist/assets/index-BMBBtagk.jsis excluded by!**/dist/**internal/admin/dashboard/static/dist/assets/index-DzO__mgM.jsis excluded by!**/dist/**internal/admin/dashboard/static/dist/index.htmlis excluded by!**/dist/**
📒 Files selected for processing (57)
web/dashboard/src/lib/components/atoms/CopyButton.svelteweb/dashboard/src/lib/components/atoms/DialogCloseButton.svelteweb/dashboard/src/lib/components/atoms/EmptyState.svelteweb/dashboard/src/lib/components/atoms/Icon.svelteweb/dashboard/src/lib/components/atoms/TableActionButton.svelteweb/dashboard/src/lib/components/atoms/icons.jsweb/dashboard/src/lib/components/molecules/FilterInput.svelteweb/dashboard/src/lib/components/organisms/AuthDialog.svelteweb/dashboard/src/lib/components/organisms/EditorDialog.svelteweb/dashboard/src/lib/components/organisms/Sidebar.svelteweb/dashboard/src/lib/components/organisms/ThemeToggle.svelteweb/dashboard/src/lib/components/organisms/TypedConfirmationDialog.svelteweb/dashboard/src/lib/components/organisms/navigation.jsweb/dashboard/src/lib/stores/confirm.svelte.jsweb/dashboard/src/pages/audit-logs/AuditEntrySummary.svelteweb/dashboard/src/pages/audit-logs/AuditFilters.svelteweb/dashboard/src/pages/audit-logs/AuditPaneTabs.svelteweb/dashboard/src/pages/auth-keys/AuthKeyList.svelteweb/dashboard/src/pages/auth-keys/AuthKeysPage.svelteweb/dashboard/src/pages/budgets/BudgetEditor.svelteweb/dashboard/src/pages/budgets/BudgetList.svelteweb/dashboard/src/pages/budgets/BudgetsPage.svelteweb/dashboard/src/pages/budgets/budgets.svelte.jsweb/dashboard/src/pages/guardrails/GuardrailList.svelteweb/dashboard/src/pages/mcp-servers/McpServerEditor.svelteweb/dashboard/src/pages/mcp-servers/McpServerList.svelteweb/dashboard/src/pages/mcp-servers/McpServersPage.svelteweb/dashboard/src/pages/models/FailoverDrafts.svelteweb/dashboard/src/pages/models/FailoverEditor.svelteweb/dashboard/src/pages/models/ModelGlobalActions.svelteweb/dashboard/src/pages/models/ModelRow.svelteweb/dashboard/src/pages/models/ModelTable.svelteweb/dashboard/src/pages/models/ModelsPage.svelteweb/dashboard/src/pages/models/PricingOverrideEditor.svelteweb/dashboard/src/pages/models/VirtualModelEditor.svelteweb/dashboard/src/pages/models/VmTargetRow.svelteweb/dashboard/src/pages/models/failover.svelte.jsweb/dashboard/src/pages/overview/ProviderStatusCard.svelteweb/dashboard/src/pages/providers-config/ProviderCredentialField.svelteweb/dashboard/src/pages/providers-config/ProviderCredentialList.svelteweb/dashboard/src/pages/providers-config/ProvidersConfigPage.svelteweb/dashboard/src/pages/providers-config/providersConfig.svelte.jsweb/dashboard/src/pages/rate-limits/RateLimitInspector.svelteweb/dashboard/src/pages/rate-limits/RateLimitList.svelteweb/dashboard/src/pages/rate-limits/RateLimitsPage.svelteweb/dashboard/src/pages/settings/BudgetResetSettings.svelteweb/dashboard/src/pages/settings/BudgetSettings.svelteweb/dashboard/src/pages/settings/FailoverSettings.svelteweb/dashboard/src/pages/settings/PricingRecalculation.svelteweb/dashboard/src/pages/settings/RuntimeRefresh.svelteweb/dashboard/src/pages/settings/TaggingSettings.svelteweb/dashboard/src/pages/usage/UsageLog.svelteweb/dashboard/src/pages/workflows/WorkflowCard.svelteweb/dashboard/src/pages/workflows/WorkflowChart.svelteweb/dashboard/src/pages/workflows/WorkflowIdBadge.svelteweb/dashboard/src/pages/workflows/WorkflowsPage.svelteweb/dashboard/tests/icons.test.js
💤 Files with no reviewable changes (1)
- web/dashboard/src/lib/components/atoms/icons.js
| submitLabel = "Save", | ||
| submittingLabel = "Saving...", | ||
| submitIcon = "save", | ||
| submitIcon = Save, |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C 6 '<EditorDialog\b|submitIcon\s*=' \
web/dashboard/src --glob '*.svelte' --glob '*.js'Repository: ENTERPILOT/GoModel
Length of output: 17093
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== EditorDialog relevant source =="
sed -n '1,180p' web/dashboard/src/lib/components/organisms/EditorDialog.svelte
echo
echo "== Icon component definitions/usages =="
fd -i 'Icon\.svelte$|Icon.*\.svelte' web/dashboard/src --exec sh -c 'echo "--- $1"; sed -n "1,220p" "$1"' sh {}Repository: ENTERPILOT/GoModel
Length of output: 5589
Update the remaining submitIcon callers to use Lucide icon data.
Icon.svelte expects Lucide icon data ([tag, attrs, children]), so callers currently passing "plus", "save", "check", or "delete" render an empty SVG node. Import the matching Lucide icons and pass them to EditorDialog on these call sites:
web/dashboard/src/pages/auth-keys/AuthKeyEditor.svelte:20,
web/dashboard/src/pages/models/VirtualModelEditor.svelte:34,
web/dashboard/src/pages/workflows/WorkflowEditor.svelte:21, and the AuthKeyLabelsEditor/MCP/Tenant/Rate-limit/Policy editors where they may also use string icons.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@web/dashboard/src/lib/components/organisms/EditorDialog.svelte` at line 46,
Update all remaining callers of EditorDialog's submitIcon prop to pass Lucide
icon data instead of strings. In AuthKeyEditor.svelte,
VirtualModelEditor.svelte, WorkflowEditor.svelte, and other editor components
(AuthKeyLabelsEditor, MCP, Tenant, Rate-limit, Policy editors), replace the
string values "plus", "save", "check", and "delete" with their corresponding
imported Lucide icons (Plus, Save, Check, Trash respectively). Import each
needed Lucide icon at the top of each file and update the submitIcon assignments
to use the imported icon data in the same format that Save is now used in
EditorDialog.svelte.
Comment on lines
+31
to
+36
| for (const match of source.matchAll(/import\s*\{([^}]*)\}\s*from\s*"lucide";/g)) { | ||
| for (const raw of match[1].split(",")) { | ||
| const name = raw.trim(); | ||
| if (!name) continue; | ||
| if (!found.has(name)) found.set(name, []); | ||
| found.get(name).push(file.slice(SRC.length + 1)); |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== locate file =="
fd -a 'icons.test.js' . || true
echo "== file excerpt =="
if [ -f web/dashboard/tests/icons.test.js ]; then
nl -ba web/dashboard/tests/icons.test.js | sed -n '1,100p'
fi
echo "== lint/import config references =="
rg -n --hidden --glob '!node_modules' --glob '!dist' --glob '!build' "eslint-plugin-import|eslint-config-prettier|single quotes|quote|jsx-a11y|svelte/linter|linter|import/.source|import source" . || true
echo "== changed stats and target diff =="
git diff --stat || true
git diff -- web/dashboard/tests/icons.test.js || true
echo "== deterministic regex behavior probe =="
python3 - <<'PY'
import re
pattern = re.compile(r'import\s*\{([^}]*)\}\s*from\s*"lucide";(?=\s|$)')
samples = ['import { Pencil } from "lucide";', "import { Pencil } from 'lucide';", 'import { Pencil } from "lucide"', 'import { Pencil as EditIcon } from "lucide";']
for s in samples:
m = pattern.search(s)
print(repr(s), "=>", ["[" + ", ".join(f'{name.strip()}: src="A"' for name in (m.group(1) or "").split(",")) + "]" if m else None])
PYRepository: ENTERPILOT/GoModel
Length of output: 286
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== file excerpt =="
if [ -f web/dashboard/tests/icons.test.js ]; then
cat -n web/dashboard/tests/icons.test.js | sed -n '1,100p'
fi
echo "== changed stats and target diff =="
git diff --stat || true
git diff -- web/dashboard/tests/icons.test.js || true
echo "== lint/import config references =="
rg -n --hidden --glob '!node_modules' --glob '!dist' --glob '!build' "eslint-plugin-import|eslint-config-prettier|single quotes|quote|jsx-a11y|svelte/linter|linter|import/.source|import source" . || true
echo "== deterministic regex behavior probe =="
python3 - <<'PY'
import re
# Current scan uses /import\s*\{([^}]*)\}\s*from\s*"lucide";/g.
# This regex stops after from "lucide"; but also runs without lookahead in MatchAll context.
pattern = re.compile(r'import\s*\{([^}]*)\}\s*from\s*"lucide";')
samples = [
'import { Pencil } from "lucide";',
"import { Pencil } from 'lucide';",
'import { Pencil } from "lucide"',
'import { Pencil as EditIcon } from "lucide";',
'import { Pencil as EditIcon, ArrowLeft } from "lucide";',
]
for s in samples:
m = pattern.search(s)
if m:
names = [name.strip() for name in m.group(1).split(",")]
else:
names = None
print(repr(s), "=> matched:", bool(m), "names:", names)
PYRepository: ENTERPILOT/GoModel
Length of output: 10687
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== lock and package manifest files =="
git ls-files | rg '(^|/)(package(-lock)?\.json|pnpm-lock\.yaml|yarn\.lock|bun\.lockb?)$' || true
echo "== lucide dependency declarations =="
rg -n --hidden --glob '!node_modules' --glob '!dist' --glob '!build' '"lucide("' package.json web/dashboard/package.json pnpm-lock.yaml package-lock.json yarn.lock bun.lock bun.lockb 2>/dev/null || true
echo "== source lucide imports =="
rg -n --hidden --glob '!node_modules' --glob '!dist' --glob '!build' 'import\s*\{[^}]*\}\s*from\s+["'\'']lucide["'\'']' web/dashboard/src packages 2>/dev/null || true
echo "== ESLint and Svelte config files =="
git ls-files | rg '(^|/)(eslint|eslint-plugin-import|svelte.config|svelte-check|jsconfig|tsconfig|prettier)(\.(js|cjs|mjs|json|yml|yaml))?$|(\.eslintrc|eslint\.config|\.lintstagedrc|svelte\.config|jsconfig|tsconfig).*\.(js|cjs|mjs|json|yml|yaml)$' || true
echo "== all non-build files with explicit quote rules =="
rg -n --hidden --glob '!node_modules' --glob '!dist' --glob '!build' --glob '!web/dashboard/tests/icons.test.js' "quote|single|jsx-quotes|import source|import from" \
.eslintrc.js .eslintrc.cjs .eslintrc.json .eslintrc.yml .eslintrc.yaml svelte.config.* eslint.config.* jsconfig.* web/dashboard/eslint.config.* web/dashboard/.eslintrc.* 2>/dev/null || trueRepository: ENTERPILOT/GoModel
Length of output: 5799
Handle aliased Lucide imports in importedIcons().
The scan matches the double-quoted form, but import { Pencil as EditIcon } from "lucide"; records the export as Pencil as EditIcon, so the comment’s stated contract is not enforced through lucide[name] for valid alias syntax. Strip the local alias before the export lookup. Also applies to lines 48-49.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@web/dashboard/tests/icons.test.js` around lines 31 - 36, Update
importedIcons() to parse aliased named imports by extracting the exported Lucide
name before the “as” clause, so lucide[name] uses Pencil rather than the local
alias EditIcon. Preserve existing handling for non-aliased imports and apply the
same normalization in the related logic at lines 48–49.
Confidence Score: 4/5
 T-Rex LogsWhat T-Rex did
Reviews (1): Last reviewed commit: "refactor(dashboard): pass lucide icons a..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| <div class="budget-row-period"> | ||
| <span class="budget-period-label {budgetPeriodClass(item)}"> | ||
| <Icon name={budgetPeriodIcon(item)} class="budget-period-icon" /> | ||
| <Icon icon={budgetPeriodIcon(item)} class="budget-period-icon" /> |
There was a problem hiding this comment.
budgetPeriodIcon(item) still returns legacy strings such as "clock" and "calendar", but Icon now iterates its icon prop as a Lucide SVG node array. Rendering a budget-period row therefore throws from Icon.svelte before the budget list mounts. Return imported Lucide icon values for each period instead of strings.
Artifacts
- Review-authored Chromium capture script that loads the focused budget-icon harness and inspects rendered period icon SVG children.
Budget-period rendering harness
- Review-authored Svelte harness that reproduces BudgetList.svelte line 66 for hourly, daily, weekly, monthly, and custom period rows alongside valid lucide controls.
- Review-authored Vite entry page that mounts the focused budget-period rendering harness for Chromium.
Observed Playwright runtime output
- Captured command output showing the Icon.svelte TypeError and confirming that no harness main element mounted.
Original capture invocation output
- Captured output from the original review-authored capture script invocation, recording its working directory and nonzero result when rendering failed.
Review follow-up. Three EditorDialog callers passed submitIcon as a name string, and budgetPeriodIcon() still returned names — both render a blank SVG now that Icon takes the icon itself. The codemod only rewrote <Icon> tags and icon: config literals, so neither shape was touched. Adds two guards: one asserts no icon binding is assigned a name string, one asserts budgetPeriodIcon returns a drawable icon. Both were confirmed to fail on the exact bugs above. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
|
|
||
| for (const file of sourceFiles(SRC)) { | ||
| const source = readFileSync(file, "utf8"); | ||
| for (const [match, prop, value] of source.matchAll(binding)) { |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@web/dashboard/tests/icons.test.js`:
- Around line 67-76: Update the icon-binding check around the binding loop and
produced-value extraction so direct quoted string expressions such as
icon={"inbox"} and single-quoted equivalents are inspected and rejected, while
preserving ternary branch handling. Ensure the extraction does not discard
non-ternary string values before the existing icon-name validation.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 47b950c7-8003-4009-b27e-463e3250f021
⛔ Files ignored due to path filters (2)
internal/admin/dashboard/static/dist/assets/index-G3uz-AWc.jsis excluded by!**/dist/**internal/admin/dashboard/static/dist/index.htmlis excluded by!**/dist/**
📒 Files selected for processing (6)
web/dashboard/src/pages/auth-keys/AuthKeyEditor.svelteweb/dashboard/src/pages/budgets/budgets-helpers.jsweb/dashboard/src/pages/models/VirtualModelEditor.svelteweb/dashboard/src/pages/workflows/WorkflowEditor.svelteweb/dashboard/tests/budgets.test.jsweb/dashboard/tests/icons.test.js
Comment on lines
+67
to
+76
| const binding = /\b(icon|submitIcon)\s*(?:=|:)\s*(\{[^}]*\}|"[^"]*")/g; | ||
| const offenders = []; | ||
|
|
||
| for (const file of sourceFiles(SRC)) { | ||
| const source = readFileSync(file, "utf8"); | ||
| for (const [match, prop, value] of source.matchAll(binding)) { | ||
| const produced = value.startsWith("{") | ||
| ? value.slice(1, -1).split(/\?|:/).slice(1).join(" ") // ternary branches only | ||
| : value; | ||
| if (!/"[a-z][a-z0-9-]*"/.test(produced)) continue; |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Inspect direct string expressions in the icon-binding check.
At Line 74, split(/\?|:/).slice(1) returns an empty string for icon={"inbox"} because the expression has no ternary operator. The test therefore permits a string-valued icon binding. The same gap affects single-quoted strings.
Proposed fix
- const binding = /\b(icon|submitIcon)\s*(?:=|:)\s*(\{[^}]*\}|"[^"]*")/g;
+ const binding =
+ /\b(icon|submitIcon)\s*(?:=|:)\s*(\{[^}]*\}|"[^"]*"|'[^']*')/g;
...
- const produced = value.startsWith("{")
- ? value.slice(1, -1).split(/\?|:/).slice(1).join(" ") // ternary branches only
- : value;
- if (!/"[a-z][a-z0-9-]*"/.test(produced)) continue;
+ const expression = value.startsWith("{")
+ ? value.slice(1, -1)
+ : value;
+ const produced = expression.includes("?")
+ ? expression.split("?").slice(1).join(" ")
+ : expression;
+ if (!/(["'])[a-z][a-z0-9-]*\1/.test(produced)) continue;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const binding = /\b(icon|submitIcon)\s*(?:=|:)\s*(\{[^}]*\}|"[^"]*")/g; | |
| const offenders = []; | |
| for (const file of sourceFiles(SRC)) { | |
| const source = readFileSync(file, "utf8"); | |
| for (const [match, prop, value] of source.matchAll(binding)) { | |
| const produced = value.startsWith("{") | |
| ? value.slice(1, -1).split(/\?|:/).slice(1).join(" ") // ternary branches only | |
| : value; | |
| if (!/"[a-z][a-z0-9-]*"/.test(produced)) continue; | |
| const binding = | |
| /\b(icon|submitIcon)\s*(?:=|:)\s*(\{[^}]*\}|"[^"]*"|'[^']*')/g; | |
| const offenders = []; | |
| for (const file of sourceFiles(SRC)) { | |
| const source = readFileSync(file, "utf8"); | |
| for (const [match, prop, value] of source.matchAll(binding)) { | |
| const expression = value.startsWith("{") | |
| ? value.slice(1, -1) | |
| : value; | |
| const produced = expression.includes("?") | |
| ? expression.split("?").slice(1).join(" ") | |
| : expression; | |
| if (!/(["'])[a-z][a-z0-9-]*\1/.test(produced)) continue; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@web/dashboard/tests/icons.test.js` around lines 67 - 76, Update the
icon-binding check around the binding loop and produced-value extraction so
direct quoted string expressions such as icon={"inbox"} and single-quoted
equivalents are inspected and rejected, while preserving ternary branch
handling. Ensure the extraction does not discard non-ternary string values
before the existing icon-name validation.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Follow-up to #648, which introduced a curated
iconRegistrymapping kebab-case names to lucide icons so the bundler could tree-shake them.That registry has a failure mode: an icon name with no registry entry resolves to an empty node array and renders a blank SVG, with nothing in the build,
svelte-check, or the test suite noticing. Review of #648 caught exactly that —EmptyState's defaulticon = "inbox"was never registered, because a prop default isn't a call site any name-scan looks at.This removes the indirection instead of patching it. Callers import the icon and pass it directly:
import { Pencil } from "lucide"; <Icon icon={Pencil} />icons.jsis deleted — there is no list to keep in sync, and tree-shaking now follows the imports structurally instead of depending on an enumerated map staying complete. Config-driven icons (sidebar nav, theme toggle, confirm dialogs, budget periods, workflow chart nodes) hold the icon itself rather than a name string.A guard test, because the toolchain is quieter than expected
I assumed a bad icon would now be a build error. It isn't — I checked: with a deliberately typo'd
import { Inboxx } from "lucide",vite buildsucceeds with zero warnings (rolldown does not error on missing named exports) andsvelte-checkreports 0 problems (jsconfig.jsonsetscheckJs: false). A typo would still have shipped as a blank SVG.So
tests/icons.test.jsscans everyimport { … } from "lucide"in the source tree and resolves each name against the real package, asserting it exists and is a non-empty node array. Unlike the registry it replaces, it needs no maintenance — it discovers imports rather than being told about them. Verified it fails on the typo above withlucide has no export "Inboxx" (imported by lib/components/atoms/EmptyState.svelte), and it runs in the existing pre-commit hook and CI.User-visible impact
None. Same icons, same rendering. Bundle size is unchanged at ~799 KB (the same icons reach the output either way) — this buys correctness and removes a maintenance burden, not bytes.
Testing
vite buildclean,svelte-check0 errors/0 warnings (314 files), 436/436 tests (435 + the new guard).$statepath:confirm.svelte.jsholds its icon inside a$stateobject, so Svelte deeply proxies the lucide array beforeIconsees it. Mounted that exact shape in a browser and confirmed it renders (3 drawable children, correct SVG namespace).🤖 Generated with Claude Code
Summary by CodeRabbit