natemoo-re / astro-icon

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

The future of `astro-icon` #87

Closed natemoo-re closed 9 months ago

natemoo-re commented 1 year ago

[!IMPORTANT] This issue was to discuss the possible v1.0 version of Astro Icon. That version has since been released. Visit the official documentation for more information.

Hey y'all! Sorry I haven't been on top of the maintenance here.

astro looked very different back when the first version of astro-icon was built. There was no integration system and SSR was just an idea we had. As such, the current version of astro-icon makes some assumptions about your project that are no longer true.

I've started working on a new version of astro-icon that will:

If you're interested, here's a StackBlitz with my initial exploration. I plan to move this into a branch and work towards a v1 of astro-icon in the coming weeks.

If you'd like to get involved, please reach out on the Astro Discord!

https://github.com/natemoo-re/astro-icon/tree/v1

okikio commented 1 year ago

@natemoo-re Looks awesome πŸ‘πŸ½

ShivamJoker commented 1 year ago

@natemoo-re right now astro icons can't be used in the SSR environment?

natemoo-re commented 1 year ago

@natemoo-re right now astro icons can't be used in the SSR environment?

It can, but it introduces some potential network latency and a dependency on the astro-icon API, which could theoretically go down. I'm working on a more optimized approach that removes that network dependency.

arnorhs commented 1 year ago

Awesome, i was just bumping into an issue deploying with ssr on cloudflare, so that's great news.

what is the state of it, is there an RFC? something you'd appreciate help with?

GrantBirki commented 1 year ago

I am very excited for this

GrantBirki commented 1 year ago

πŸ‘‹ Hey @natemoo-re! Super excited for this feature and wondering if there has been any additional progress made on this? Thanks in advance!

AyushSehrawat commented 1 year ago

Great! Faced deployment issues on cloudflare :/. Commenting here so that i can follow up with the progress :)

ddlaws0n commented 1 year ago

Fantastic news! Astro Icon has been very enjoyable to use (other than the svgo issues). Hope to see great things soon :)

surjithctly commented 1 year ago

Awesome. Are there any plans to move this to official Astro Plugins?

sonicjhon1 commented 1 year ago

Any news on this?

stramel commented 1 year ago

I'm working on getting the code to a good state and ensuring we're hitting all the use-cases.

stramel commented 1 year ago

Would anyone be willing to test this version of the v1 branch out for me?

You can install it using: npm i "https://gitpkg.now.sh/natemoo-re/astro-icon/packages/core?v1-alpha"

You can read how to use it here: https://github.com/natemoo-re/astro-icon/blob/v1-alpha/packages/core/README.md

Migration notes: Ensure your Icon imports from astro-icon/components now!

Thanks in advance for any and all feedback!

ddlaws0n commented 1 year ago

Hey @stramel ! Thrilled to see movement on v1 :)

Had a chance to test this out this morning: https://github.com/ddlaws0n/astro-icon-v1-testing Not working for me unfortunately, unless I'm doing something wrong:

image
stramel commented 1 year ago

Thanks @ddlaws0n! Looks like my local version had cached. This was a missing extension which resulted in bad imports.

I have pushed a fix. You may need to remove the existing dependency and then re-install it using the steps I posted previously.

mattpaz commented 1 year ago

Just tried it. Looks promising, but here are my results:

When I start up astro, I get ... [astro-icon] "mdi" does not appear to be a valid iconify collection!

My integrations:

  integrations: [
    icon({
      iconDir: 'src/assets/icons',
      include: {
        mdi: ['check'], // Only loads the Material Design Icon's "account" SVG
      }
    }),
    tailwind({
      config: {
        applyBaseStyles: false
      }
    }),
    svelte()
  ]

And from the page I was loading an icon from ...

import { Icon } from 'astro-icon/components'; <h2><Icon name="mdi:check" /> Testing </h2>

image
Error: Unable to locate "mdi:check" icon!
    at eval (/node_modules/astro-icon/components/Icon.astro:26:17)
    at AstroComponentInstance.Icon [as factory] (/node_modules/astro/dist/runtime/server/astro-component.js:22:12)
    at AstroComponentInstance.init (/node_modules/astro/dist/runtime/server/render/astro/instance.js:26:29)
    at AstroComponentInstance.render (/node_modules/astro/dist/runtime/server/render/astro/instance.js:31:18)
    at render.next (<anonymous>)
    at Module.renderChild (/node_modules/astro/dist/runtime/server/render/any.js:30:18)
    at processTicksAndRejections (node:internal/process/task_queues:96:5)
    at async EagerAsyncIterableIterator.next (/node_modules/astro/dist/runtime/server/render/util.js:148:14)
    at async RenderTemplateResult.[Symbol.asyncIterator] (/node_modules/astro/dist/runtime/server/render/astro/render-template.js:43:7)
    at async Module.renderAstroTemplateResult (/node_modules/astro/dist/runtime/server/render/astro/render-template.js:51:20)

Perhaps I misconfigured something?

stramel commented 1 year ago

@mattpaz Did you install @iconify-json/mdi or @iconify/json?

mattpaz commented 1 year ago

@stramel Ah, you're right. I didn't actually anticipate needing to to that, but after adding @iconify-json/mdi it seemed to work well. Thanks!

stramel commented 1 year ago

Awesome! I'm looking into adding an auto install flag which would handle that for you but we'll see if that happens

djmtype commented 1 year ago

Has anyone tried deploying their Astro project to Netlify with astro-icon v1 alpha, and had success? Mine fails during build with a very vague error. Failed during stage 'building site': Build script returned non-zero exit code: 2

My package.json includes: "astro-icon": "https://gitpkg.now.sh/natemoo-re/astro-icon/packages/core?v1-alpha.1"

Since this is the only thing that has changed in my Astro project, I'm only making an educated guess the cause might be astro-icon v1 alpha.

BTW, locally, everything builds fine. No errors, and the preview works.

stramel commented 1 year ago

Has anyone tried deploying their Astro project to Netlify with astro-icon v1 alpha, and had success? Mine fails during build with a very vague error. Failed during stage 'building site': Build script returned non-zero exit code: 2

My package.json includes: "astro-icon": "https://gitpkg.now.sh/natemoo-re/astro-icon/packages/core?v1-alpha"

Since this is the only thing that has changed in my Astro project, I'm only making an educated guess the cause might be astro-icon v1 alpha.

BTW, locally, everything builds fine. No errors, and the preview works.

Can you provide any other error outputs? In the meantime, I'll see if I can find any issues

djmtype commented 1 year ago

@stramel BTW, I have a src/icons directory in my project, but it's empty because I'm not using it; only icons from @iconify-json. So, while this works on local, it doesn't on Netlify.

Here's the error I'm getting only on Netlify:

[astro-icon] Could not load virtual:astro-icon (imported by node_modules/astro-icon/components/Icon.astro): ENOENT: no such file or directory, scandir 'src/icons/'
7:42:24 PM:  error   Could not load virtual:astro-icon (imported by node_modules/astro-icon/components/Icon.astro): ENOENT: no such file or directory, scandir 'src/icons/'
7:42:24 PM: Error: Could not load virtual:astro-icon (imported by node_modules/astro-icon/components/Icon.astro): ENOENT: no such file or directory, scandir 'src/icons/'

Here's my astro-icon config. I also tried manually adding iconDir afterwards. It didn't matter. Error persists on Netlify.

icon({
      iconDir: 'src/icons',
      include: {
          ri: ['*'],
          ion: ['*'],
          bx: ['*'],
          ic: ['*'],
          'simple-icons': ['*']
      }
  }),
djmtype commented 1 year ago

@stramel BTW, If I remove the src/icons directory from my project, it fails on local and on Netlify which is why I added the directory to my project even though I'm not using it.

In order to avoid the error on Netlify, I had to add some arbitrary svg file to fix the build error.

Is a src/icons directory being required by astro-icon?

stramel commented 1 year ago

@stramel BTW, I have a src/icons directory in my project, but it's empty because I'm not using it; only icons from @iconify-json. So, while this works on local, it doesn't on Netlify.

Here's the error I'm getting only on Netlify:

[astro-icon] Could not load virtual:astro-icon (imported by node_modules/astro-icon/components/Icon.astro): ENOENT: no such file or directory, scandir 'src/icons/'
7:42:24 PM:  error   Could not load virtual:astro-icon (imported by node_modules/astro-icon/components/Icon.astro): ENOENT: no such file or directory, scandir 'src/icons/'
7:42:24 PM: Error: Could not load virtual:astro-icon (imported by node_modules/astro-icon/components/Icon.astro): ENOENT: no such file or directory, scandir 'src/icons/'

Here's my astro-icon config. I also tried manually adding iconDir afterwards. It didn't matter. Error persists on Netlify.

icon({
      iconDir: 'src/icons',
      include: {
          ri: ['*'],
          ion: ['*'],
          bx: ['*'],
          ic: ['*'],
          'simple-icons': ['*']
      }
  }),

Thanks, I'll take a look! I noticed this locally when running the build too.

stramel commented 1 year ago

@djmtype I'm unable to reproduce this with a minimal reproduction. https://github.com/stramel/astro-quickstart/commit/a90d66c1bdf2f54dda6f754c35abe48f9d29eb99 deployed to netlify https://main--peppy-meringue-be9fba.netlify.app/

I did have issues with the virtual: module when I didn't have the integration installed yet. But it seems strange that it wouldn't find your src/icons folder.

Feel free to ping me on the Astro discord username: stramel

stramel commented 1 year ago

We have just published a prerelease version of the v1 to npm. You can install it using npm i astro-icon@next or npm i astro-icon@1.0.0-next.1 if you want the specific version! Please feel free to file an issue or let us know if you run into any issues!

GrantBirki commented 1 year ago

Update: I added a single missing icon pack and now my project is working πŸŽ‰ .

I will keep the following content below (outdated) for others to use


πŸ‘‹ Hey @stramel! I saw your comment above and tried to give the next tag a go. Unfortunately I'm running into errors when running npm run build. The changes (via a PR) and the error can be found here: https://github.com/GrantBirki/astrowind/pull/127#issuecomment-1614026370

Here is what I did for the version upgrade:

  1. Read the README on the v1 branch of this project
  2. Installed icon packs
  3. Updated my astro integrations statement to include the newly added icon packs
  4. Updated astro-icon to the release mentioned here
  5. Updated my Icon import statements in all files like so
  6. Ran npm run build and witnessed a heinous error message

It should also be noted that I see in the v1 readme that there are some TODO sections around migrating so perhaps I charted on my own course and something went wrong.

Also, I get this message a lot in my builds and when doing local development: Unable to load local collection. Ensure you have your iconDir configured properly.

stramel commented 1 year ago

Thank you, we really appreciate the detailed feedback! I'll take a look at trying without a pack to see if we can clean up the error messaging on that. Strange that the error message looked like it was referring to the v0 astro-icon code.

UPDATE: I am getting this when missing a package:

image
GrantBirki commented 1 year ago

It should also be noted that I don't actually use any local icon packs at the moment. I only use the npm packages that get installed from @iconify-json/<value> so I find the error message Unable to load local collection. Ensure you have your iconDir configured properly. to be odd.

Also, @stramel I merged all the changes into main and you can check them out here: https://github.com/GrantBirki/astrowind

The project works as expected apart from that one weird iconDir message so I consider your v1 branch to be a huge success!

stramel commented 1 year ago

@GrantBirki Fantastic! Glad to hear that it's working as expected!

We'll discuss the message about the iconDir. I think there might be a better place to alert about the local collection.

stramel commented 1 year ago

@GrantBirki I just published a new version that only alerts about local directory when using and not having it configured properly. Pushed to the next tag and 1.0.0-next.2. Hopefully this gets rid of the unnecessary warnings when not using local icon sets.

GrantBirki commented 1 year ago

Works perfectly, thanks!

https://github.com/GrantBirki/astrowind/pull/131

tusamni commented 1 year ago

I'm having issues with local icons and subdirectories in the icons folder. I had to put all the icons in the root icons directory and it seems to be working now.

stramel commented 1 year ago

I'm having issues with local icons and subdirectories in the icons folder. I had to put all the icons in the root icons directory and it seems to be working now.

@tusamni Thanks for the details! I will look into this. It should work in subdirectories. Are you using a custom iconDir?

stramel commented 1 year ago

@tusamni I just tested locally with a custom iconDir and the default without being able to reproduce. Can you provide more details to help reproduce the issue?

tusamni commented 1 year ago

I'm referencing the icons like this:

<Icon name="jobs/icg" class:list={["absolute", c.size]} />

I'm bringing in the component here: import { Icon } from "astro-icon/components";

My icon folder below: image

I'm getting this error:


Unable to locate "jobs/icg" icon!
The icon named "jobs/icg" was not found in your local icon directory.```
stramel commented 1 year ago

I'm referencing the icons like this:

<Icon name="jobs/icg" class:list={["absolute", c.size]} />

I'm bringing in the component here: import { Icon } from "astro-icon/components";

My icon folder below: image

I'm getting this error:

Unable to locate "jobs/icg" icon!
The icon named "jobs/icg" was not found in your local icon directory.```

Ahh, this should just be referenced by the icon name directly. We currently don't differentiate icons based on their folder. So your case you would use <Icon name="icg" class:list={["absolute", c.size]} />

tusamni commented 1 year ago

Ah interesting. That did work.

Thanks for clarifying that!

surjithctly commented 1 year ago

I just tried the new version. after adding a custom iconfity package, I started to get this strange error. but then the error did not go even after removing that. Finally, I had to replace old astro-icon to fix the issue.

ReferenceError: An error occurred.
require is not defined
find-up/index.js:2:14
'use strict';
const path = require('path');
surjithctly commented 1 year ago

Also pnpm astro add astro-icon@next doesn't seem to be working as per the Readme.

CleanShot 2023-07-18 at 12 57 06

cachitas commented 1 year ago

Also pnpm astro add astro-icon@next doesn't seem to be working as per the Readme.

CleanShot 2023-07-18 at 12 57 06

I think you should use pnpm/npm/yarn directly and not astro add.

Quoting the README from both main and next branches:

npm i astro-icon
# or
yarn add astro-icon 
surjithctly commented 1 year ago

@cachitas Wait, Isn’t the new branch called v1?

https://github.com/natemoo-re/astro-icon/tree/v1

stramel commented 1 year ago

@cachitas Wait, Isn’t the new branch called v1?

https://github.com/natemoo-re/astro-icon/tree/v1

I don't believe the astro add will work until we publish it under the stable instead of next.

For now use the manual instructions but install using npm i astro-icon@next to get the V1 changes.

stramel commented 11 months ago

Sorry for the lack of communication here. We're currently stalled on the SSR DX and waiting for direction on the core SVG integration into Astro. (https://github.com/withastro/roadmap/issues/699)

jefferykarbowski commented 11 months ago

I am getting "Error: Unable to load the "local" icon set!" when trying to use an local icons in /src/icons I have also tried /assets/icons and also a custom directory. I am on next.2. Please advise, thank you!

Error: Unable to load the "local" icon set! hint: 'It looks like the "local" set was not loaded.\n' + '\n' + 'Did you forget to create the icon directory or to update your config?'

fflaten commented 10 months ago

I'm confused. How are you all testing next when the astro-icon/components export points to ./components/index.js which is missing? From what I can see the components folder is never even compiled and the typescript source is packaged instead. I must be doing something very stupid and wrong - please help πŸ˜„