withastro / starlight

🌟 Build beautiful, accessible, high-performance documentation websites with Astro
https://starlight.astro.build
MIT License
4.59k stars 493 forks source link

SVG Logo during build doubles in dist/_astro folder #2161

Open ztxone opened 1 month ago

ztxone commented 1 month ago

What version of starlight are you using?

0.25.3

What version of astro are you using?

4.12.2

What package manager are you using?

pnpm

What operating system are you using?

Mac

What browser are you using?

Chrome

Describe the Bug

I have logo.svg in /src/assets/logo.svg

Added link to logo in astro.config.mjs

export default defineConfig({
  integrations: [
    starlight({
        title: 'Документация Интеграм',
            logo: {
                src: './src/assets/logo.svg',
            },
   })]
})

Added same logo in /src/content/docs/index.mdx as image for hero at frontmatter

---
hero:
   image: 
       file: ../../../src/assets/logo.svg
---

After pnpm run build i'm getting 2 versions of same logo.svg in /dist/_astro/ folder with 2 type of hashes in filename as shown on screenshot

Screenshot 2024-07-30 at 22 20 00

Same stuff in reproduced stackblitz - adding logo to header through astro.config.mjs - doubles houston.webp in dist folder

Probably it is unnecessary doubling of asset or am i doing smth wrong?

Link to Minimal Reproducible Example

https://stackblitz.com/edit/github-spzcpm?file=astro.config.mjs

Participation

lorenzolewis commented 1 month ago

Hi @ztxone 👋

What this looks like behind the scenes is Starlight is creating a component for:

1) Site logo in the navbar: https://github.com/withastro/starlight/blob/b4345ce24aaf8040f5b761d1565906e3343daa5a/packages/starlight/components/SiteTitle.astro#L12-L18 (plus another if using a different logo for dark themes)

2) That hero image on the Hero: https://github.com/withastro/starlight/blob/b4345ce24aaf8040f5b761d1565906e3343daa5a/packages/starlight/components/SiteTitle.astro#L12-L18

In the first case (site title) Astro is using the HTML <img> tag (Astro's docs on <img> within Astro.

In the second (hero) Astro is the Astro Assets <Image> component. This component does a bit more processing of images such as ensuring it is the correct size, compressed appropriately, etc.

With that being said, from a technical perspective, this is behaving as I would expect as they are two different logic routes from my understanding. However, maybe someone else will have some thoughts on this as I agree that in the case SVGs I would expect them to roughly be the same file and this is un-needed duplication. But this is much easier said than to have any changes put in to address this.


Edit: Fixed a link to better highlight the code snippet

ztxone commented 1 month ago

The problem, as i understand from docs, /assets/ folder stands for images that should be transformed/compressed by Astro. And basically - this is the same picture. So why is it processed twice? I mean that it should be one way of using images around the project or through or through , isn't it?

lorenzolewis commented 1 month ago

I believe that is only the case when using the <Image> component. Taking a closer look at that docs page (without looking into the implementation:

We recommend that local images are kept in src/ when possible so that Astro can transform, optimize and bundle them.

(underline above added by me)

However, the section on <img> specifically says that they won't be processed:

These images will not be processed and optimized.

Again, not to disagree with your point that there is duplication here as an end result, but just to say that it is working as I would expect given my personal understand.

lorenzolewis commented 1 month ago

Asked over on Astro and @Princesseuh give a bit more input here:

Yes, the Image component will reuse the image if it has the same transform

However, regarding SVGs (also from @Princesseuh)

Yeah, note that Astro does not process SVGs anyway

So, in theory, it may be possible to use the same method of images (i.e. both using the <Image> tag) in order to help trim the duplicate out.

ztxone commented 1 month ago

Ok, thanks for explanation. I've understood that in this case it works as it should be. But it is not obvious thing that in one case it uses <img> and in other <Image>.

ztxone commented 1 month ago

However, regarding SVGs (also from @Princesseuh)

Yeah, note that Astro does not process SVGs anyway

well but there are two svgs in dist folder from 1 svg used by different methods somehow with different hashes))

lorenzolewis commented 1 month ago

One additional thing I dug up while testing an alternative to the current approach in Starlight is that the SiteTitle logo is actually resolved through a virtual module, but not exactly sure why:

https://github.com/withastro/starlight/blob/b4345ce24aaf8040f5b761d1565906e3343daa5a/packages/starlight/components/SiteTitle.astro#L2

So even after mocking up a test to see if it was possible to de-duplicate these it resulted in the same. I'm not sure if this is due to the virtual module causing some obfuscation that results in Astro not being able to detect they're the same file or not, or if it's a red-herring.

@delucis you had https://github.com/withastro/starlight/pull/51 that added that virtual module in, do you think it could be a reason for images still being duplicated even after switching the SiteTitle component to use <Image>?

lorenzolewis commented 1 month ago

However, regarding SVGs (also from @Princesseuh)

Yeah, note that Astro does not process SVGs anyway

well but there are two svgs in dist folder from 1 svg used by different methods somehow with different hashes))

Yes, but the contents (at least in my local test) are the same for both files. It's just a different file name in order to prevent any potential filename collision.

lorenzolewis commented 1 month ago

RE: Why using a virtual module, maybe it's because when defining an image in front matter (like the case for Hero) the image helper from Zod is used which will populate the required metadata: https://docs.astro.build/en/guides/images/#images-in-content-collections. When using exclusively a config, it's only a string and not the imported image and it's respective properties that are needed by <Image>, so that's what the virtual module may be doing 🤔

delucis commented 1 month ago

The virtual module shouldn’t make a difference here — that’s just needed to pass the user-configured image path to other parts of Starlight code.

I think the following is happening:

  1. logo is set in the Starlight config and we use its original src object without the <Image> component. Looks like this is the original file name in the build output.

  2. The same file is used in the hero frontmatter and we use <Image> to render it. While not transforming the original SVG, Astro does “fingerprint” the file with a hash so that if it changes, the URL also changes.

We should probably use <Image> in both cases. If I recall correctly, when first writing some of this code, <Image> did not support SVGs at all yet, which probably explains why we aren’t using it consistently.

(Note though that I suspect Astro will still output the file twice in the build output for this reason)

Princesseuh commented 1 month ago

(Note though that I suspect Astro will still output the file twice in the build output for https://github.com/withastro/astro/issues/4961#issuecomment-1271806656)

This is no longer accurate in astro:assets, we get rid of the original image if we don't find any usages of it outside of image optimisation (minus a few cases where we can't determine that reliably for technical reasons). See https://github.com/withastro/astro/pull/8954

chappelo commented 3 weeks ago

I'll try solve this issue.