Feat/add provider lucide#72
Merged
Merged
Conversation
added 6 commits
March 15, 2026 06:06
- Refactor buildIcons to accept options for provider selection - Update cache structure to store icons per provider - Extract icon retrieval logic for extensibility
…ther and Lucide icons - Add `provider` option to select between Feather and Lucide icons - Set default provider to `lucide` - Update build logic to use the selected provider for icon generation - Add runtime configuration for the new provider option
- Add Feather provider implementation - Add Lucide provider implementation - Add providers module index
- Remove hardcoded feather import to enable provider switching - Support dynamic selection between different icon sets
Contributor
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughAdds multi-provider icon support by introducing a provider abstraction (feather|lucide), normalizing provider icon data, updating the build pipeline to accept ModuleOptions (including provider), adding lucide as a dependency, and generating per-provider icon components with per-provider caching. Changes
Sequence DiagramsequenceDiagram
participant Setup as Module Setup
participant Build as buildIcons()
participant Provider as getIcons(provider)
participant Feather as getFeatherIcons()
participant Lucide as getLucideIcons()
participant FS as File System
Setup->>Build: buildIcons(nuxt, options)
Build->>Provider: getIcons(options.provider)
alt provider == 'lucide'
Provider->>Lucide: getLucideIcons()
Lucide->>Lucide: load lucide, normalize icons
Lucide->>Provider: return IconNormalized[]
else provider == 'feather'
Provider->>Feather: getFeatherIcons()
Feather->>Feather: iterate feather.icons, normalize
Feather->>Provider: return IconNormalized[]
end
Provider->>Build: icons[]
Build->>Build: cache per-provider (cache[provider])
Build->>FS: write generated component files
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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 |
![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: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/module.ts (1)
61-67:⚠️ Potential issue | 🟠 MajorProvider default inconsistency with build.ts.
The default provider here is
'lucide', but insrc/runtime/build.tsline 80, the fallback is'feather':const provider = options.provider || 'feather' // build.ts line 80This inconsistency could cause unexpected behavior if
options.provideris somehow undefined when reachingbuildIcons().🔧 Proposed fix in src/runtime/build.ts
export async function buildIcons(nuxt: Nuxt, options: ModuleOptions): Promise<ModuleIconsNames[]> { - const provider = options.provider || 'feather' + const provider = options.provider || 'lucide'🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/module.ts` around lines 61 - 67, The default provider in the module defaults object is 'lucide' but build.ts falls back to 'feather', causing inconsistent behavior; update the defaults.provider value in the defaults object (the defaults: {...} entry in module.ts) to 'feather' so it matches the provider fallback used in buildIcons()/src/runtime/build.ts, ensuring both places use the same default provider.
🧹 Nitpick comments (2)
src/runtime/build.ts (1)
58-65: CSS classes are hardcoded to'feather'regardless of provider.The template component generates CSS classes
'feather'and'feather-${name}'(lines 60-61) even when using the Lucide provider. Consider making these classes provider-aware to avoid confusion and enable provider-specific styling.♻️ Proposed approach
Pass the provider to
templateComponentand generate appropriate class names:-const templateComponent = (attrs: FeatherAttrs, innerHTML: string, componentName: string, name: string) => ` +const templateComponent = (attrs: FeatherAttrs, innerHTML: string, componentName: string, name: string, provider: string) => ` ... const classes = computed(() => { return [ - 'feather', - 'feather-${name || ""}', + '${provider}', + '${provider}-${name || ""}', config.class, props.class ].filter(Boolean).join(' ').trim() })Then update the call site in
buildIcons:const component = templateComponent( icon.attrs, icon.contents, icon.componentPascalName, icon.name, provider // Add provider argument )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/runtime/build.ts` around lines 58 - 65, The computed classes in templateComponent currently hardcode 'feather' and 'feather-${name}' which ignores the selected provider; update the classes generation inside the computed named classes to use the provider variable (e.g., `${provider}` and `${provider}-${name || ""}`) instead of 'feather', add a provider parameter to templateComponent's signature so it can use that value, and update the buildIcons call site where templateComponent is invoked (replace templateComponent(..., icon.name) with templateComponent(..., icon.name, provider)) so provider-aware class names are emitted while still including config.class and props.class.src/runtime/providers/lucide.ts (1)
20-27: Consider using self-closing tags for void SVG elements.The current implementation generates
<path ...></path>instead of<path ... />. While functionally equivalent, self-closing tags are more idiomatic for SVG void elements likepath,circle,line, etc.♻️ Proposed fix for self-closing tags
const contents = nodes .map(([tagName, attrs]: [string, any]) => { const attrString = Object.entries(attrs || {}) .map(([k, v]) => `${k}="${v}"`) .join(' ') - return `<${tagName} ${attrString}></${tagName}>` + return `<${tagName} ${attrString} />` }) .join('')🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/runtime/providers/lucide.ts` around lines 20 - 27, The generated SVG node string in the contents mapping currently always uses an open/close pair; update the mapping in src/runtime/providers/lucide.ts (the nodes => map block that builds contents) to emit self-closing tags for SVG void elements (e.g., path, circle, rect, line, polyline, polygon, stop, use, rect, img, etc.): check tagName against a small set/array of void SVG tag names and, when matched, return `<${tagName}${attrString ? ' ' + attrString : ''} />` (no closing tag); otherwise keep the existing `<${tagName}${attrString ? ' ' + attrString : ''}></${tagName}>` behavior so non-void elements remain unchanged. Ensure attrString is built the same way so spacing is correct when there are no attributes.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/runtime/build.ts`:
- Around line 79-84: The provider fallback in buildIcons is incorrect: inside
the async function buildIcons(nuxt: Nuxt, options: ModuleOptions) the line that
sets const provider = options.provider || 'feather' should use the module
default 'lucide' instead; change the fallback to 'lucide' so provider is
computed as options.provider || 'lucide' and existing cache[provider] checks and
returns continue to work with the corrected default.
- Line 17: The declaration for the module icons cache uses mutable binding but
is never reassigned; change the top-level variable declaration named "cache"
from let to const so it remains a constant reference (keeping the same type
Record<string, ModuleIconsNames[]>), ensuring internal mutations still work
while preventing reassignment of the identifier.
In `@src/runtime/providers/lucide.ts`:
- Around line 3-13: Replace the runtime CommonJS require in getLucideIcons by
using a top-level ESM import for the lucide package (import the named export
that provides the icons map instead of calling require('lucide')), update the
thrown Error in getLucideIcons to an English message (e.g., "Failed to load
Lucide icons. Ensure the 'lucide' package is installed."), and convert the
Portuguese inline comments around lucide, iconsMap and subsequent blocks to
English to match the codebase; reference getLucideIcons, lucide (icons export)
and iconsMap when making these changes.
---
Outside diff comments:
In `@src/module.ts`:
- Around line 61-67: The default provider in the module defaults object is
'lucide' but build.ts falls back to 'feather', causing inconsistent behavior;
update the defaults.provider value in the defaults object (the defaults: {...}
entry in module.ts) to 'feather' so it matches the provider fallback used in
buildIcons()/src/runtime/build.ts, ensuring both places use the same default
provider.
---
Nitpick comments:
In `@src/runtime/build.ts`:
- Around line 58-65: The computed classes in templateComponent currently
hardcode 'feather' and 'feather-${name}' which ignores the selected provider;
update the classes generation inside the computed named classes to use the
provider variable (e.g., `${provider}` and `${provider}-${name || ""}`) instead
of 'feather', add a provider parameter to templateComponent's signature so it
can use that value, and update the buildIcons call site where templateComponent
is invoked (replace templateComponent(..., icon.name) with
templateComponent(..., icon.name, provider)) so provider-aware class names are
emitted while still including config.class and props.class.
In `@src/runtime/providers/lucide.ts`:
- Around line 20-27: The generated SVG node string in the contents mapping
currently always uses an open/close pair; update the mapping in
src/runtime/providers/lucide.ts (the nodes => map block that builds contents) to
emit self-closing tags for SVG void elements (e.g., path, circle, rect, line,
polyline, polygon, stop, use, rect, img, etc.): check tagName against a small
set/array of void SVG tag names and, when matched, return
`<${tagName}${attrString ? ' ' + attrString : ''} />` (no closing tag);
otherwise keep the existing `<${tagName}${attrString ? ' ' + attrString :
''}></${tagName}>` behavior so non-void elements remain unchanged. Ensure
attrString is built the same way so spacing is correct when there are no
attributes.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: a64a3e85-3a7d-47f2-b5a0-c244f240ad45
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (7)
.github/CONTRIBUTING.mdpackage.jsonsrc/module.tssrc/runtime/build.tssrc/runtime/providers/feather.tssrc/runtime/providers/index.tssrc/runtime/providers/lucide.ts
- Switch to ES6 import syntax - Add fallback for icons object - Use optional chaining for children property - Translate error messages to English
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
package.json (1)
3-3: Prefer a minor version bump for this feature release.Line 3 uses
1.4.2, but this PR introduces a new public capability (multi-provider support). Consider releasing as1.5.0to keep SemVer signaling clear for consumers.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@package.json` at line 3, Update the package version to a minor bump to reflect the new public capability: change the "version" field in package.json from "1.4.2" to "1.5.0" so the release correctly signals a minor SemVer change for the added multi-provider support.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@package.json`:
- Line 3: Update the package version to a minor bump to reflect the new public
capability: change the "version" field in package.json from "1.4.2" to "1.5.0"
so the release correctly signals a minor SemVer change for the added
multi-provider support.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 0c964146-7875-4ed2-a91b-67b33f9e3724
📒 Files selected for processing (2)
CHANGELOG.mdpackage.json
✅ Files skipped from review due to trivial changes (1)
- CHANGELOG.md
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
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.
Summary by CodeRabbit