fix(deps): per-icon lucide-solid imports to shrink vite transform burst (#389) - #433
Open
omridevk wants to merge 1 commit into
Open
fix(deps): per-icon lucide-solid imports to shrink vite transform burst (#389)#433omridevk wants to merge 1 commit into
omridevk wants to merge 1 commit into
Conversation
…st (#389) Barrel imports from lucide-solid pulled all ~1700 icon modules through the vite transform on every cold start (92% of @conciv/app's vitest-browser cold-start import burst). Converted every value import across the 54 files that used the barrel to per-icon default imports resolved from lucide-solid's actual export map, and added a no-restricted-imports guard so the barrel can't come back (type-only imports still allowed). lucide-solid ships no "type": "module" field and shares one ambient .d.ts across its import/require/browser export conditions, so under this repo's strict NodeNext module resolution the per-icon default export types as the whole module namespace instead of the icon component (TS2604). A wildcard ambient override (`declare module 'lucide-solid/icons/*'`) does not fix this: TypeScript only consults wildcard ambient modules when normal resolution fails, and it succeeds here (just with the wrong shape). Exact-specifier module augmentation does override the resolved type, so each affected package gets one generated src/lucide-solid-icons.d.ts (or src/cards/lucide-solid-icons.d.ts where the cards tsconfig scopes separately) with one declare-module block per icon in the repo-wide union, identical content everywhere. apps/conciv needs no shim: its tsconfig uses moduleResolution: "bundler", which isn't affected. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
📝 WalkthroughWalkthroughThe PR restricts runtime imports from the ChangesLucide direct import migration
Estimated code review effort: 2 (Simple) | ~15 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint install failed. For unrecoverable errors, disable the tool in CodeRabbit configuration. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/ui-kit-system/src/lucide-solid-icons.d.ts (1)
1-475: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy lift11 byte-identical 475-line declaration files. Each file declares the same 66
lucide-solid/icons/*modules with the same body. The shared root cause is the generator: it emits the union of all icons into every package instead of emitting one shared declaration that packages reference. This adds about 5,200 duplicated lines and creates drift risk, because adding one icon requires regenerating 11 files.Two additional consequences:
- Each package declares icons it does not import. For example, the tanstack extension uses one icon.
declare module 'lucide-solid/icons/<name>'is a global augmentation. If two of these packages are ever compiled in one TypeScript program, the duplicate module declarations conflict.Recommended direction: publish one declaration file from a single shared package, then reference it from each package
tsconfig.jsonthroughtypesor an include path. Keep the generator, but have it write one file.Note the path inconsistency as well: 9 files sit at
<package>/src/, but two sit undersrc/cards/. Align them if the per-package files are kept.
packages/ui-kit-system/src/lucide-solid-icons.d.ts#L1-L475: keep this as the single source of truth, or move it to a dedicated types package, and export it for the other packages to reference.packages/core/src/cards/lucide-solid-icons.d.ts#L1-L475: delete and reference the shared declaration; also move out ofsrc/cards/if the file is kept.packages/extensions/recorder/src/lucide-solid-icons.d.ts#L1-L475: delete and reference the shared declaration.packages/extensions/tanstack/src/lucide-solid-icons.d.ts#L1-L475: delete and reference the shared declaration.packages/extensions/terminal/src/lucide-solid-icons.d.ts#L1-L475: delete and reference the shared declaration.packages/extensions/test-runner/src/lucide-solid-icons.d.ts#L1-L475: delete and reference the shared declaration.packages/extensions/try-it/src/lucide-solid-icons.d.ts#L1-L475: delete and reference the shared declaration.packages/extensions/whiteboard/src/lucide-solid-icons.d.ts#L1-L475: delete and reference the shared declaration.packages/tools/src/cards/lucide-solid-icons.d.ts#L1-L475: delete and reference the shared declaration; also move out ofsrc/cards/if the file is kept.packages/ui-kit-chat-tools/src/lucide-solid-icons.d.ts#L1-L475: delete and reference the shared declaration.packages/ui-kit-chat/src/lucide-solid-icons.d.ts#L1-L475: delete and reference the shared declaration.🤖 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 `@packages/ui-kit-system/src/lucide-solid-icons.d.ts` around lines 1 - 475, Replace the generator’s per-package output with one shared declaration containing the 66 lucide-solid icon modules, keeping packages/ui-kit-system/src/lucide-solid-icons.d.ts as the source of truth or moving it to a dedicated types package. Configure every other package to reference that declaration through its tsconfig types/include settings, and delete the duplicated files in packages/core/src/cards/lucide-solid-icons.d.ts, packages/extensions/recorder/src/lucide-solid-icons.d.ts, packages/extensions/tanstack/src/lucide-solid-icons.d.ts, packages/extensions/terminal/src/lucide-solid-icons.d.ts, packages/extensions/test-runner/src/lucide-solid-icons.d.ts, packages/extensions/try-it/src/lucide-solid-icons.d.ts, packages/extensions/whiteboard/src/lucide-solid-icons.d.ts, packages/tools/src/cards/lucide-solid-icons.d.ts, packages/ui-kit-chat-tools/src/lucide-solid-icons.d.ts, and packages/ui-kit-chat/src/lucide-solid-icons.d.ts; if retained, move the files currently under src/cards/ to the common declaration location.
🤖 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.
Nitpick comments:
In `@packages/ui-kit-system/src/lucide-solid-icons.d.ts`:
- Around line 1-475: Replace the generator’s per-package output with one shared
declaration containing the 66 lucide-solid icon modules, keeping
packages/ui-kit-system/src/lucide-solid-icons.d.ts as the source of truth or
moving it to a dedicated types package. Configure every other package to
reference that declaration through its tsconfig types/include settings, and
delete the duplicated files in packages/core/src/cards/lucide-solid-icons.d.ts,
packages/extensions/recorder/src/lucide-solid-icons.d.ts,
packages/extensions/tanstack/src/lucide-solid-icons.d.ts,
packages/extensions/terminal/src/lucide-solid-icons.d.ts,
packages/extensions/test-runner/src/lucide-solid-icons.d.ts,
packages/extensions/try-it/src/lucide-solid-icons.d.ts,
packages/extensions/whiteboard/src/lucide-solid-icons.d.ts,
packages/tools/src/cards/lucide-solid-icons.d.ts,
packages/ui-kit-chat-tools/src/lucide-solid-icons.d.ts, and
packages/ui-kit-chat/src/lucide-solid-icons.d.ts; if retained, move the files
currently under src/cards/ to the common declaration location.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 14fa6ae0-3cd3-474c-865d-2a70ef8f2304
📒 Files selected for processing (66)
.oxlintrc.jsonapps/conciv/src/composer/actions.tsxapps/conciv/src/composer/launch-menu.tsxapps/conciv/src/composer/model-selector.tsxapps/conciv/src/composer/session-selector.tsxapps/conciv/src/pane/grab-reference.tsxapps/conciv/src/pane/indicators.tsxapps/conciv/src/pane/pane-composer.tsxapps/conciv/src/pane/tool-fallback-card.tsxapps/conciv/src/routes/panel.$sessionId.tsxapps/conciv/src/routes/quick.tsxapps/conciv/src/shell/notices.tsxpackages/core/src/cards/code-run-card.tsxpackages/core/src/cards/lucide-solid-icons.d.tspackages/extensions/recorder/src/client.tsxpackages/extensions/recorder/src/lucide-solid-icons.d.tspackages/extensions/recorder/src/tool/card.tsxpackages/extensions/tanstack/src/client/inspector-chip.tsxpackages/extensions/tanstack/src/lucide-solid-icons.d.tspackages/extensions/terminal/src/client.tsxpackages/extensions/terminal/src/client/terminal-actions.tsxpackages/extensions/terminal/src/lucide-solid-icons.d.tspackages/extensions/test-runner/src/lucide-solid-icons.d.tspackages/extensions/test-runner/src/tool/card.tsxpackages/extensions/try-it/src/client/connect-pane.tsxpackages/extensions/try-it/src/lucide-solid-icons.d.tspackages/extensions/whiteboard/src/client.tsxpackages/extensions/whiteboard/src/client/inbox.tsxpackages/extensions/whiteboard/src/lucide-solid-icons.d.tspackages/extensions/whiteboard/src/tool/canvas/card.tsxpackages/extensions/whiteboard/src/tool/comment/card.tsxpackages/tools/src/cards/lucide-solid-icons.d.tspackages/tools/src/cards/ui-card.tsxpackages/ui-kit-chat-tools/src/lucide-solid-icons.d.tspackages/ui-kit-chat-tools/src/styled/tools/apply-patch-diff.tsxpackages/ui-kit-chat-tools/src/styled/tools/bash-card.tsxpackages/ui-kit-chat-tools/src/styled/tools/discovered-apis-card.tsxpackages/ui-kit-chat-tools/src/styled/tools/file-edit-card.tsxpackages/ui-kit-chat-tools/src/styled/tools/file-read-card.tsxpackages/ui-kit-chat-tools/src/styled/tools/search-card.tsxpackages/ui-kit-chat-tools/src/styled/tools/todo-card.tsxpackages/ui-kit-chat-tools/src/styled/tools/tool-lookup-card.tsxpackages/ui-kit-chat/src/lucide-solid-icons.d.tspackages/ui-kit-chat/src/styled/action-bar.tsxpackages/ui-kit-chat/src/styled/activity.tsxpackages/ui-kit-chat/src/styled/attachment-dispatch.tsxpackages/ui-kit-chat/src/styled/attachment-ui.tsxpackages/ui-kit-chat/src/styled/branch-picker.tsxpackages/ui-kit-chat/src/styled/chain-of-thought.stories.tsxpackages/ui-kit-chat/src/styled/chain-of-thought.tsxpackages/ui-kit-chat/src/styled/composer.tsxpackages/ui-kit-chat/src/styled/model-selector.tsxpackages/ui-kit-chat/src/styled/now-line.tsxpackages/ui-kit-chat/src/styled/thread.tsxpackages/ui-kit-chat/src/tools/primitives/status-visual.tsxpackages/ui-kit-chat/src/tools/styled/collapsible-card.tsxpackages/ui-kit-chat/src/tools/styled/collapsible-section.tsxpackages/ui-kit-chat/src/tools/styled/json-tree.tsxpackages/ui-kit-chat/src/tools/styled/meta-tool-card.tsxpackages/ui-kit-chat/src/tools/styled/note-row.tsxpackages/ui-kit-chat/src/tools/styled/permission-card.tsxpackages/ui-kit-chat/src/tools/styled/tool-fallback.tsxpackages/ui-kit-chat/src/tools/styled/tool-group.tsxpackages/ui-kit-chat/src/tools/styled/tool-icon.tsxpackages/ui-kit-system/src/lucide-solid-icons.d.tspackages/ui-kit-system/src/toast.tsx
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
1 participant
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.
Root cause (#389 strand)
The @conciv/app vitest-browser suite's cold-start import burst was 92% lucide-solid: barrel imports pulled all ~1700 icon modules through the vite transform on every cold run (solid packages are excluded from dep-prebundle, so the icons never hit the .vite cache). On starved x86 CI runners that 60-530s window is where Chromium's network service kills all page websockets at one instant, the vite client reloads the tester frame, and vitest browser mode cannot reconnect (upstream vitest-dev/vitest#10791) — the rotating-victim
Browser connection was closedfailure.Profiled with
DEBUG=vite:transform: 1969 of 2759 transformed modules were lucide-solid icons. With this change: 826 modules; local cold import 27.2s → 12.8s; Rosetta rig (x86, --cpus 1, cold .vite) import 190-290s → 46-69s, strand 0/6 vs 1/6 control (historical cold baseline ~35-50%/run).Change
lucide-solid/icons/<name>) across 54 files — lucide's own recommended pattern for vite dev perf.no-restricted-importsguard in .oxlintrc.json so the barrel can't come back (type-only imports still allowed)."type"field and one ambient .d.ts shared across export conditions, so NodeNext types the per-icon default export as the module namespace (TS2604) — unfixed as of lucide-solid@1.31.0. Wildcard ambient overrides don't work (TS only consults them on resolution miss); each NodeNext package gets one generatedlucide-solid-icons.d.tswith exact-specifier augmentation per icon (repo-wide union, identical content). apps/conciv needs none (bundler resolution). Deletable once upstream ships dual declarations.Verification
Closes #389
🤖 Generated with Claude Code
Summary by CodeRabbit