natemoo-re / astro-icon

Inline and sprite-based SVGs in Astro made easy!
https://astroicon.dev
Other
1.01k stars 57 forks source link

Missing icon packs #29

Closed sachinraja closed 6 months ago

sachinraja commented 2 years ago

Hey, first of all, thanks for making this, this makes working with icons much easier for me.

Would it be possible to update @iconify/json? https://github.com/natemoo-re/astro-icon/blob/d2e7fafe79369df19f79fe120e00fc829c27ab62/packages/service/package.json#L21-L25

I want to use an icon added in a newer version of a pack but @iconify/json is not on the latest version here.

natemoo-re commented 2 years ago

Thanks for opening an issue! I'm thinking through how to version the service without breaking anyone... do you know which version of @iconify/json you need?

sachinraja commented 2 years ago

I need version 2.0.10, where the Simple Icons collection added the Astro icon. I'm not sure what the breaking change is.

peterlobster commented 2 years ago

I would like to see this updated too. There are actually quite a few icons missing from our current set vs upstream's current set. It looks like the version used here seems to be about 6 months old.

@natemoo-re I believe that they update the upstream repo quite a bit (3 times a week by (a) fully automated script). So it might be worth considering implementing Dependabot to assist?

JulianCataldo commented 2 years ago

Mhhh… I'm missing system-uicons:cubes and others too

mgjules commented 1 year ago

Is there any update on the issue or a roadmap? Thanks

stramel commented 1 year ago

@natemoo-re I just stumbled upon this issue as well. I'm not aware of the original decision behind the service but is the service necessary?

To bypass the versioning issue, would it make more sense to make @iconify/json a peer dependency in which the astro-icon loads from and grabs the icon data? Doing it this way would allow for each application to manage their version range of the @iconify/json and even lock it using a lock file. Switching to this would incur a breaking change still.

Alternatively, maybe we could lean on a CDN such as Skypack to handle the versioning request or unpkg for us and allow the service to accept a version range and pass it along to the CDN? (eg. 'https://cdn.skypack.dev/@iconify/json@~2.0', isn't currently working) (eg. https://unpkg.com/@iconify/json@~2.0.0/json/mdi.json)

zaha commented 1 year ago

I'm another one that just stumbled over this and have finally an explanation why some icons just wouldn't work.

I am in favor of @stramel's suggestion of making this a peer dependency, letting the individual applications manage the version of @iconify/json themself.

DJOCKER-FACE commented 1 year ago

Same problem here is there any quick fix?

zaha commented 1 year ago

Same problem here is there any quick fix?

@DJOCKER-FACE My quick fix is to download the icons as SVG and use them as local icons.

stramel commented 1 year ago

@zaha @DJOCKER-FACE I forked and did a quick implementation using the updated tooling and @iconify/json as a peerDependency. Seems to be working 👍🏼

You can try it out using:

npm install -D 'https://gitpkg.now.sh/stramel/astro-icon/packages/core?main'
npm install @iconify/json

@natemoo-re @matthewp Let me know if you want me to clean it up and submit a PR (https://github.com/natemoo-re/astro-icon/compare/main...stramel:astro-icon:main)

SenseiMarv commented 1 year ago

@zaha @DJOCKER-FACE I forked and did a quick implementation using the updated tooling and @iconify/json as a peerDependency. Seems to be working 👍🏼

You can try it out using:

npm install -D 'https://gitpkg.now.sh/stramel/astro-icon/packages/core?main'
npm install @iconify/json

@natemoo-re @matthewp Let me know if you want me to clean it up and submit a PR (main...stramel:astro-icon:main)

Thank you very much for coming up with a solution! While newer icon packs now work with the dev server running locally, Vercel still seems to have problems as I now get the error Error: [astro-icon] Unable to load icon in the serverless function after deploying your solution when using an icon from the icon set fluent-emoji-high-contrast:

Unknown application error occurred
2022-10-12T15:27:22.179Z    f0b3895b-733f-45df-9359-bf1e7e988530    WARN    WARNING(astro-seo): You passed the same value to `title` and `openGraph.optional.title`. This is most likely not what you want. See docs for more.
2022-10-12T15:27:22.217Z    f0b3895b-733f-45df-9359-bf1e7e988530    ERROR   Unhandled Promise Rejection     {"errorType":"Runtime.UnhandledPromiseRejection","errorMessage":"Error: [astro-icon] Unable to load icon \"fluent-emoji-high-contrast:first-quarter-moon\"!\nError: ENOENT: no such file or directory, open '/var/task/node_modules/@iconify/json/json/fluent-emoji-high-contrast.json'","reason":{"errorType":"Error","errorMessage":"[astro-icon] Unable to load icon \"fluent-emoji-high-contrast:first-quarter-moon\"!\nError: ENOENT: no such file or directory, open '/var/task/node_modules/@iconify/json/json/fluent-emoji-high-contrast.json'","stack":["Error: [astro-icon] Unable to load icon \"fluent-emoji-high-contrast:first-quarter-moon\"!","Error: ENOENT: no such file or directory, open '/var/task/node_modules/@iconify/json/json/fluent-emoji-high-contrast.json'","    at file:///var/task/entry.js:2790:13","    at async renderToIterable (file:///var/task/entry.js:650:21)","    at async renderAstroComponentInline (file:///var/task/entry.js:878:24)","    at async renderChild (file:///var/task/entry.js:485:5)","    at async AstroComponent.[Symbol.asyncIterator] (file:///var/task/entry.js:608:7)","    at async renderAstroComponent (file:///var/task/entry.js:619:20)","    at async renderAstroComponentInline (file:///var/task/entry.js:879:9)","    at async renderChild (file:///var/task/entry.js:485:5)","    at async AstroComponent.[Symbol.asyncIterator] (file:///var/task/entry.js:608:7)","    at async renderAstroComponent (file:///var/task/entry.js:619:20)"]},"promise":{},"stack":["Runtime.UnhandledPromiseRejection: Error: [astro-icon] Unable to load icon \"fluent-emoji-high-contrast:first-quarter-moon\"!","Error: ENOENT: no such file or directory, open '/var/task/node_modules/@iconify/json/json/fluent-emoji-high-contrast.json'","    at process.<anonymous> (file:///var/runtime/index.mjs:1131:17)","    at process.emit (node:events:539:35)","    at process.emit (node:domain:475:12)","    at emit (node:internal/process/promises:140:20)","    at processPromiseRejections (node:internal/process/promises:274:27)","    at processTicksAndRejections (node:internal/process/task_queues:97:32)"]}
Unknown application error occurred
stramel commented 1 year ago

@SenseiMarv So I dug into this a bit today. The issue is that with the Vercel serverless functions, they don't deploy node_modules and the function is going to attempt to read from node_modules/@iconify/json folder upon request.

I'm looking into solutions currently as this would affect all serverless functions that don't have access to the node_modules folder. You can also swap to Vercel static adapter instead for the interim.

SenseiMarv commented 1 year ago

@SenseiMarv So I dug into this a bit today. The issue is that with the Vercel serverless functions, they don't deploy node_modules and the function is going to attempt to read from node_modules/@iconify/json folder upon request.

I'm looking into solutions currently as this would affect all serverless functions that don't have access to the node_modules folder. You can also swap to Vercel static adapter instead for the interim.

Good research! So, the issue in the deployment is actually not related to this issue. I tried to deploy using only icon sets that were existing in the old @iconify/json import and saw the same error. Concerning this, my issue is more related to issue https://github.com/natemoo-re/astro-icon/issues/35, so I will give that a thumbs up.

One note about switching to the static adapter: There are caveats connected to that switch. The following things have to be considered:

stramel commented 1 year ago

@SenseiMarv I was able to get it up and working on Vercel. https://astro-icon-test-qjvhzptnd-stramel.vercel.app/

Huge thanks to @JuanM04 for the PR in the Vercel adapter to make this work. Once the changes in https://github.com/withastro/astro/pull/5085 have been merged, you should be able to do:

 adapter: vercel({
    includeFiles: [
        // replace the path below with path to the correct icon pack you want to include
        './node_modules/@iconify/json/json/heroicons.json', 
    ]
})

You can also test now with the next--include-files version on @astrojs/vercel

manuelmeister commented 1 year ago

What is actually the advantage of having a custom API, as opposed to using Iconify's API directly?

@natemoo-re why did you choose to create an API?

I've tried to create a small subset of this package without an own api: manuelmeister/astro-iconify and it works exactly the same, but without an intermediary api. Could this also be a way forward? (Of course we would need to support the old v1 api as well, now that we have introduced it)

Or would it be best if everyone had to use the iconify-json package as peer dependency if they want to use all packs without including the individual packs?

mseele commented 1 year ago

Any updates on this? Would be cool to use https://icones.js.org/collection/heroicons 👍

surjithctly commented 1 year ago

Related: https://github.com/natemoo-re/astro-icon/issues/73

Why? because json-tools is depreciated in favor of @iconify/utils

nimser commented 1 year ago

I'm seeing the recent #105 and #111 that are supposed to close this but in my tests (using code from the v1 branch) it's still missing icons, e.g. (bi:balloon-heart-fill). Anything special to do on the user's end to make it work ?

stramel commented 1 year ago

@nimser Did you make sure to install @iconify-json/bi and update your config to include the package, icon({ include: { bi: ['*'] } })?

I was able to successfully reference bi:balloon-heart-fill using this fork. I haven't pushed the latest fixes and build to the v1 branch yet.

csjui commented 1 year ago

@stramel Can you please share the correct way to add this to the Astro config?

stramel commented 1 year ago

@stramel Can you please share the correct way to add this to the Astro config?

This should explain it https://github.com/natemoo-re/astro-icon/blob/950f8a56a6493891ec5539a885e438b74965e3dc/packages/core/README.md

csjui commented 1 year ago

Thanks. When I add import icon from "astro-icon"; to my config file and run dev, it gives an error about the Icon.astro file: ts file only able to be used if enabled as imports:

2:37:44 PM [vite] Error when evaluating SSR module /node_modules/astro-icon/index.ts: failed to import "/node_modules/astro-icon/lib/Icon.astro"
|- Error: Parse failure: Unexpected token (2:7)
At file: /node_modules/astro-icon/lib/Icon.astro
Contents of line 2: import load, { fallback, normalizeProps } from "/node_modules/astro-icon/lib/utils.ts";

Then I try adding "allowImportingTsExtensions": true, to tsconfig but get the following

2:54:54 PM [vite] Error when evaluating SSR module /Users/chris/Github/frontend_astro/astro.config.mjs: failed to import "/node_modules/astro-icon/index.ts"
|- Error: Parse failure: Unexpected token (2:7)
At file: /node_modules/astro-icon/lib/Icon.astro

Any help much appreciated

stramel commented 1 year ago

@csjui Sorry, I must have missed this comment in my notifications!

The error you are receiving is due to using the v0 astro-icon as the v1.

Please ensure that you have installed the latest code npm i astro-icon@next if you're trying to use the integration.

nimser commented 1 year ago

@stramel indeed I was missing porper conf. Works great now after a fresh install of astro-icon@next and proper conf. Thank you 🙏🏼