Fix processing same image multiple times

[ken0x0a]

Changes

• add a check before generating image with asset service to prevent generating same image multiple time

Fix #6564, close #6489

Testing

I didn't test only this change.
I'm using astro with few more patches and just extracted only this part, but this changes are very simple and don't even need to verify, I think.

Docs

No changes for usage

[changeset-bot]

🦋 Changeset detected

Latest commit: e746af1

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

[Princesseuh]

Hmm, this is fine, but I wonder if there isn't an issue higher in the chain that we should fix before doing this fix that would hide other issues.

For instance, in the Markdown one, there's 9 empty Markdown files, and yet a bunch of images are being added. Maybe there's some sort of global state not being reseted correctly.

[ken0x0a]

Hmm, maybe.

It's also possible to avoid adding duplicated value for Map by iterate for keys of the map and check objects equality by value of object. I mean something like this https://lodash.com/docs/#isEqual, not ===, as transform objects are created each time.

[andersk]

The real underlying bug here is an unnecessarily nested for loop. Here’s a fix:

• Fix redundant image asset optimization #6596

[Princesseuh]

Thank you for contributing! As this logic was completely removed in #6604, this PR is no longer relevant, so I'll be closing this. Hopefully, the new logic should fix this issue as well!

[ken0x0a]

@Princesseuh Thank you for your fix, a problem I couldn't figure out the cause had been fixed.
But it seems this also necessary.
This reduces my build time from 53.62s to 15.08s and reduces lots of duplicated command to generate same image.
It's definitely because I'm using patch to resize image using custom assets service to create bunch of image for different sizes to use img's srcset attribute though, this has huge impact for me.

My issue due to this line.

astro/packages/astro/src/assets/vite-plugin-assets.ts

Line 176 in cc90d72

globalThis.astroAsset.staticImages.set(options, filePath);

Using an object as Map's key, and this object is created each time this function is called.

So, I guess this issues not only me, but also users who using import someImg from '../assets/myimg.jpg' from multiple astro file and more (see implementation of getImage)

astro/packages/astro/src/assets/internal.ts

Line 52 in 1e7d6af

export async function getImage(options: ImageTransform): Promise<GetImageResult> {

Without this PR's change, every time getImage called, new image generation occurs, even if users requested same image.

So, I appreciate if you reopen this or fix this issue in more performant way :D

[Princesseuh]

This will be tackled in a separate PR, because there was two issues at play. There was an issue that was specific to Markdown (which is what my PR fixed, and what the other PR also fixed) and there's the issue you're having with getImage.

The issue for that one is still open here: #6489. I'm hoping to fix this in a more "deep" way