-
Notifications
You must be signed in to change notification settings - Fork 67
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. Weβll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: allow using IconifyJSON as custom collection #366
base: main
Are you sure you want to change the base?
feat: allow using IconifyJSON as custom collection #366
Conversation
4f99b19
to
688f174
Compare
|
||
function loadCollection(prefix: string) { | ||
if (customCollectionNames.has(prefix)) { | ||
const collection = customCollections.find(c => c.prefix === prefix) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
.find
should be alright here. We enter this path only once per custom collection. Optimizing lookup seems like an overkill for the sensible length of customCollections
.
if (customCollectionNames.has(prefix)) { | ||
const collection = customCollections.find(c => c.prefix === prefix) | ||
if (collection) { | ||
return Promise.resolve(collection) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using the Promise.resolve
here to keep the return type Promise<IconifyJSON | undefined>
and play nicely with existing logic of iconifyCollectionMap
.
const data = getIconData(collection, name) | ||
if (data) { | ||
addIcon(collection.prefix, name, data) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the change which properly merges icon settings with collection settings. Previously, each SVG icon had all the metadata (mostly width and height) embedded in its declaration. With IconifyJSON
, it is common to keep common values on the collection level.
// SVG collection as loaded from `dir`
{
prefix: 'svg',
icons: {
'nuxt-v2': { body: '...', height: 512, width: 512 },
'nuxt-v3': { body: '...', height: 512, width: 512 },
}
}
// _Optimized_ IconifyJSON for premium icon set
{
prefix: 'optimized',
icons: {
'nuxt-v2': { body: '...' },
'nuxt-v3': { body: '...' },
},
height: 512,
width: 512,
}
π Linked issue
N/A
β Type of change
π Description
Motivation
Using non-public
IconifyJSON
collections is common when working with premium icon sets or aiming for build optimizations by avoiding repetitive SVG parsing on each build.Current issue
Currently, custom icon collections can be included using the
serverBundle.collections
option. However, this approach has significant drawbacks since it disables importantnuxt-icon
features such as server bundle auto-detection and client-side bundling.Proposed solution
This PR updates
nuxt-icon
to recognize and properly handleIconifyJSON
collections as custom collections. This ensures seamless integration, preserves all existing features (including auto-detection and client-side bundling), and provides a more robust solution for users who want to leverage private or optimized icon sets.Major changes
customCollections
now supports direct use ofIconifyJSON
objectsclientBundle.scan
totrue
now also includes scanning for custom icon collectionsMinor fixes
In the playground, when
includeCustomCollections: true
is not set,nuxt-v3
icon without a prefix breaks. I suspect it may have something to do withuseResolvedName
, as a network call tonuxt.json?icon=v3
can be spotted when opening the playground.This PR causes minimal conflicts with #360. I am willing to resolve them if needed.