unplugin / unplugin-icons

🤹 Access thousands of icons as components on-demand universally.
https://www.npmjs.com/package/unplugin-icons
MIT License
3.66k stars 131 forks source link

v0.15.2 break the default width, height attributes in custom icons #266

Closed DevilTea closed 1 year ago

DevilTea commented 1 year ago

In the project examples/vite-vue3

v0.15.1 is normal in this section

image

v0.15.2 is broken

image

This custom icon issue in examples/vite-vue3/App.vue L42 caused by the height attribute is missing, and options.scale seems not work on custom icons only

I guess it is caused by the new version of @iconify/utils

Ragura commented 1 year ago

I can confirm this also happens in our production project. We have the following Vite plugin settings:

Icons({
      compiler: 'vue3',
      customCollections: {
        clt: FileSystemIconLoader(
          './src/assets/icons',
          svg => svg.replace(/^<svg /, '<svg fill="currentColor" '),
        ),
      },
    }),
antfu commented 1 year ago

/cc @cyberalien in case you are aware of some changes

cyberalien commented 1 year ago

Can you post code samples of your custom icons that fail, so I could figure out what's causing it?

Ragura commented 1 year ago

Thanks for the swift reaction!

I'm not sure which code samples would help, but I will do my best. I've posted my Vite config above, but I can also add the following information:

The HTML of the icon is rendered without a height and a width, which used to be there before. It now looks like this:

<svg fill="currentColor" xmlns="http://www.w3.org/2000/svg" viewBox="0 0 512 512" data-testid="icon" class="">

Notice there is no height or width set. However, before upgrading the package it rendered this:

<svg width="1.2em" height="1.2em" fill="currentColor" xmlns="http://www.w3.org/2000/svg" viewBox="0 0 512 512" data-testid="icon" class="">

In this case there is a width and a height present in the svg.

In both cases the svg is wrapped inside a div that sets the font-size, which further scaled the svg correctly before the update. It doesn't do this anymore after the update, font-size has no effect and the svg has a width and height of 0 (invisible on page, but present in DOM).

I hope this has shed some light on the situation. Feel free to ask for more specific details if you need anything!

cyberalien commented 1 year ago

Thanks. By code samples, I mean original icon's code. It would allow me to see which attributes existed before they were processed by loader, how they were formatted. Often small things like spacing or extra bad code could cause conflicts, though it shouldn't, but well... there is a bug somewhere.

cyberalien commented 1 year ago

Managed to trigger the bug. I'll fix that!

cyberalien commented 1 year ago

I think bug is fixed now. Added tests to make sure it works.

Can you test by installing @iconify/utils@dev?

If something is broken after updating Utils package, can you post SVG (as it is in your assets folder) that still fails?

Ragura commented 1 year ago

Thank you for the quick work! I will try it out first thing tomorrow and report back as soon as possible.

Ragura commented 1 year ago

I installed the package @iconify/utils@dev, but nothing's changed. Do I have to take some other steps? I didn't have @iconify/utils installed as a direct dependency before, only unplugin-icons. Does it know to utilize the updated package's util functions? Just to make sure it wasn't using phantom dependencies I removed node_modules and reinstalled everything, to no avail.

Anyway, I'll also paste one of the svgs here as it is in the assets folder (but all of them have this problem), pre-transformation:

<svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 512 512"><path d="M139.61 35.5a12 12 0 0 0-17 0L58.93 98.81l-22.7-22.12a12 12 0 0 0-17 0L3.53 92.41a12 12 0 0 0 0 17l47.59 47.4a12.78 12.78 0 0 0 17.61 0l15.59-15.62L156.52 69a12.09 12.09 0 0 0 .09-17zm0 159.19a12 12 0 0 0-17 0l-63.68 63.72-22.7-22.1a12 12 0 0 0-17 0L3.53 252a12 12 0 0 0 0 17L51 316.5a12.77 12.77 0 0 0 17.6 0l15.7-15.69 72.2-72.22a12 12 0 0 0 .09-16.9zM64 368c-26.49 0-48.59 21.5-48.59 48S37.53 464 64 464a48 48 0 0 0 0-96zm432 16H208a16 16 0 0 0-16 16v32a16 16 0 0 0 16 16h288a16 16 0 0 0 16-16v-32a16 16 0 0 0-16-16zm0-320H208a16 16 0 0 0-16 16v32a16 16 0 0 0 16 16h288a16 16 0 0 0 16-16V80a16 16 0 0 0-16-16zm0 160H208a16 16 0 0 0-16 16v32a16 16 0 0 0 16 16h288a16 16 0 0 0 16-16v-32a16 16 0 0 0-16-16z"/></svg>
cyberalien commented 1 year ago

I've tested that icon with new version of utils package and your config, seems to work correctly now. Added more unit tests to make sure this doesn't happen again.

Published update for Iconify Utils and sent PR to this repo.

cyberalien commented 1 year ago

Try 0.15.3, it should work correctly now.

Ragura commented 1 year ago

I can confirm it works perfectly with the update! Thank you @cyberalien for the quick work and @antfu for the fast PR merge!