Skip to content
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

Missing icon packs #29

Closed
sachinraja opened this issue Feb 27, 2022 · 24 comments · Fixed by #105 or #111
Closed

Missing icon packs #29

sachinraja opened this issue Feb 27, 2022 · 24 comments · Fixed by #105 or #111
Assignees
Labels
bug Something isn't working
Milestone

Comments

@sachinraja
Copy link

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?

"dependencies": {
"@iconify/json": "^1.1.439",
"@iconify/json-tools": "^1.0.10",
"etag": "^1.8.1"
},

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

@sachinraja sachinraja changed the title update @iconify/json service not using the latest version of @iconify/json Feb 27, 2022
@natemoo-re
Copy link
Owner

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
Copy link
Author

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
Copy link

peterlobster commented Jun 16, 2022

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
Copy link

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

@mgjules
Copy link

mgjules commented Jul 19, 2022

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

@stramel
Copy link
Collaborator

stramel commented Oct 6, 2022

@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
Copy link

zaha commented Oct 7, 2022

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
Copy link

Same problem here is there any quick fix?

@zaha
Copy link

zaha commented Oct 7, 2022

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 pushed a commit to stramel/astro-icon that referenced this issue Oct 10, 2022
@stramel
Copy link
Collaborator

stramel commented Oct 10, 2022

@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)

SenseiMarv added a commit to SenseiMarv/the-frontview that referenced this issue Oct 12, 2022
* New: Add Emoji component

A fork of `astro-icon` had to be used as workaround for natemoo-re/astro-icon#29.
SenseiMarv added a commit to SenseiMarv/the-frontview that referenced this issue Oct 13, 2022
* Chore: Change astro-icon package

A fork of `astro-icon` had to be used as workaround for natemoo-re/astro-icon#29.
@SenseiMarv
Copy link

@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
Copy link
Collaborator

stramel commented Oct 13, 2022

@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 added a commit to SenseiMarv/the-frontview that referenced this issue Oct 14, 2022
* WIP: Test deploying older icons

This is a test commit to check whether older icons can work with the Vercel serverless function. This is related to natemoo-re/astro-icon#29 (comment).
@SenseiMarv
Copy link

@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 #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:

  • import vercel from "@astrojs/vercel/serverless"; has to be changed to import vercel from "@astrojs/vercel/static"; in astro.config.mjs
  • output: "server" has to be removed from astro.config.mjs
  • It is now required to use getStaticPaths() in dynamic routes
  • Server-side Rendering features like Astro.redirect are no longer available
  • Some variables might become undefined, like: routes

@stramel
Copy link
Collaborator

stramel commented Oct 14, 2022

@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 withastro/astro#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

@stramel stramel added the bug Something isn't working label Oct 20, 2022
@stramel stramel self-assigned this Oct 20, 2022
@stramel stramel mentioned this issue Oct 20, 2022
@manuelmeister
Copy link

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?

@stramel stramel changed the title service not using the latest version of @iconify/json Missing icon packs Mar 15, 2023
@mseele
Copy link

mseele commented Mar 29, 2023

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

mosaicthej added a commit to red-ant-hvac/static-astro that referenced this issue Apr 22, 2023
Updated the `astro-icon` package because many icon packs aren't avaibile

I found this work around from their issue page and applied it:

natemoo-re/astro-icon#29 (comment)
@surjithctly
Copy link

Related: #73

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

@nimser
Copy link

nimser commented Jun 6, 2023

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
Copy link
Collaborator

stramel commented Jun 6, 2023

@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.

This was referenced Jun 15, 2023
@csjui
Copy link

csjui commented Jun 19, 2023

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

@stramel
Copy link
Collaborator

stramel commented Jun 20, 2023

@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
Copy link

csjui commented Jun 20, 2023

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
Copy link
Collaborator

stramel commented Jun 29, 2023

@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
Copy link

nimser commented Jul 15, 2023

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.