Consider running iconify's replaceIDs function on SVGs

[charlie-hadden]

Hi again and sorry for the wave of issues here. This is another one for v1.

Some of the SVGs I'm using reference elements by ID. This works fine when there's a single SVG on the page, but with multiples it's possible for the IDs to collide causing issues with things such as masks. Iconify provides a function which resolves this for me: https://iconify.design/docs/icon-components/svg-framework/replace-ids.html

I currently have a pnpm patch which adds the import and replaces

astro-icon/packages/core/components/Icon.astro

Line 104 in e66e3ad

const normalizedBody = renderData.body;

with

const normalizedBody = replaceIDs(renderData.body);

This is enough to fix the issue for me by generating unique IDs for each. I'd be happy to submit a PR for this unless it's more convenient for somebody else to do so, but before I do, is there any preference on what prefix to use? Using replaceIDs as above generates ID's like so:

Thanks!

[stramel]

How are you referencing these ids if they're random? You can also use the prefixIds plugin to add prefixes. Something like:

  plugins: [
    'cleanupIds',
    {
      name: 'prefixIds',
      params: {
        prefix() {
        this.counter = this.counter || 0;

        return `id-${this.counter++}`;
      }
      }
    }
  ]

I'm going to consider adding an option to enable this. I think it could cause unwanted issues for people looking to directly reference the ids.

[charlie-hadden]

I've set up another demo to explain what I mean: https://github.com/charlie-hadden/astro-icon-reproduction/tree/replace-ids

If you give it a try then please use pnpm as there's a patch for astro-icon there to pass the svgo options through (as per #129).

If you run that project then you'll see there are two SVGs displayed. In astro.config.mjs, if you uncomment the cleanupIds plugin then you'll see the issue. There's some other comments there with examples of prefixIds and why, as far as I can tell at least, that won't work for this use case. I'm not an expert with svgo though, so it's possible there's something I'm missing.

I then have another branch which has a pnpm patch to add the replaceIDs call, and you can see the difference in results: https://github.com/charlie-hadden/astro-icon-reproduction/tree/replace-ids-patched

This issue was also quite confusing for me when I first ran across it, because at that point I didn't have any custom svgo config at all - the default preset is enough to trigger the issue with these SVGs.

Hopefully that explains what I mean a little better but let me know if there's anything you'd like me to expand on.

[fvieira]

I was also bitten by this issue, and it was a pain to understand what was going on since the SVGs are fine on their own, and I didn't activate svgo manually, so had no idea that it could be the culprit.
I just had some broken icons in some pages and not in others and had to do a lot of comparing differences before I understood it was having another icon with an id in the same page that was breaking the icons below it.

In the end I ended up fixing this with the code below on astro.config.ts, but this feels like something that shouldn't happen by default.

    icon({
      svgoOptions: {
        plugins: [
          { name: 'preset-default' },
          {
            name: 'prefixIds',
            params: {
              prefix: () => generateUniqueId(),
            },
          },
        ],
      },
    }),

[delucis]

+1 to doing something here. I was just trying to update a project to v1 and noticed this as a regression. With astro-icon v0 something like <linearGradient id="a" ...> is rendered as <linearGradient id="astroicon:icon-namea" ...> and references to it like fill="url(#a)" are updated too, but with v1 the IDs are preserved as authored making it pretty easy for IDs to clash between icons.

The prefixIds solution does work, but AFAICT it doesn’t provide access to stuff like the current icon name, so you can’t easily create deterministic IDs, which would be part of solving the desire for these not to just be random.