fix: let sharp process svg images

[borgoat]

Hello!

We have a repo where I wanted to use our logo as favicon. SVGs may finally be used directly! 🤗
But we do have to provide PNGs as fallback. And even .ico, to be really safe.

The idea was to then commit just the SVG as asset and single source of truth, and convert that through getImage.

Generating the .ico is kind of special¹, but this approach worked, so I figured it'd be pretty much the same for getImage.

Interestingly, however, when we tried to do the same in the component using getImage we would just get the same SVG out, untouched.

I was then able to find #7643 where this was deliberately implemented. From what I understand, other transformation services may not support SVG, but I'm not sure why it would be prevented in Sharp, too?

I couldn't really figure out the rationale, but maybe I'm missing something. Hope this makes sense!

Changes

• Let Sharp process SVG files

Testing

I tested this against my repo, where I had identified the bug, with the following component:

---
import faviconSrc from "@assets/logo.svg";

const favicon = (size: number) =>
  getImage({
    src: faviconSrc,
    format: "png",
    width: size,
    height: size,
  });

const appleTouchIcon = await favicon(180);
const favicon32 = await favicon(32);
const favicon16 = await favicon(16);
---

<link rel="apple-touch-icon" href={appleTouchIcon.src} />
<link rel="icon" type="image/png" sizes="32x32" href={favicon32.src} />
<link rel="icon" type="image/png" sizes="16x16" href={favicon16.src} />

Docs

I'm not sure whether something should be added to the docs, to be honest, I didn't find anything mentioning that SVG would not be supported as an input to getImage in the docs, but that's the case right now. With this fix, nothing should be mentioned as things should just work out of the box.

Footnotes

1. https://github.com/sealambda/ostaro/blob/main/src/pages/favicon.ico.ts ↩

[changeset-bot]

🦋 Changeset detected

Latest commit: 2739aad

The changes in this PR will be included in the next version bump.

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[codspeed-hq]

CodSpeed Performance Report

Merging #13069 will not alter performance

_{Comparing borgoat:fix/sharp-svg-support (2739aad) with main (e36837f)}

Summary

✅ 6 untouched benchmarks

[ematipico]

Astro has experimental support for SVGs now. Is there a particular reason why you can't use that? https://docs.astro.build/en/reference/experimental-flags/svg/

[borgoat]

Astro has experimental support for SVGs now. Is there a particular reason why you can't use that? https://docs.astro.build/en/reference/experimental-flags/svg/

I don't know: from what I understand this is to embed <svg> components straight into Astro components? But what I require is rather the opposite. I need both a link to the SVG, and 1 or more PNG files to use as favicons. These are <link> to embed in the head, they don't inline the SVG.

[Princesseuh]

This would be great to support, but it requires more code changes because we block SVGs in a few places in the image service. It's also somewhat breaking, so might need to be implemented more carefully.

[borgoat]

This would be great to support, but it requires more code changes because we block SVGs in a few places in the image service. It's also somewhat breaking, so might need to be implemented more carefully.

Hehe yeah I figured my fix was too dumb to be ok 🥲 I mean, surely that check was there for a reason... but yet I was surprised to see it just work in the naive example from above, and so I went ahead and committed it like that 😅

If you could provide the error cases maybe I can come up with some ideas...

I guess there may be some issues with output formats other than PNG? Or is it something with alpha?

I know SVG can be finicky and yield unexpected results, at the same time the existing behaviour where it just ignores the configuration seems rather confusing. I believe it should either error out saying it's unsupported (although this specific example did work as expected), or it could print a warning saying that there may be unexpected results, but without blocking users when things actually work for their use case.

What do you think?