JonasKruckenberg / imagetools

Load and transform images using a toolbox :toolbox: of custom import directives!
MIT License
909 stars 56 forks source link

fix: avif format #708

Closed AdamMerrifield closed 5 months ago

AdamMerrifield commented 5 months ago

The new test for cache.avifFormat will fail on imagetools@7.0.0

changeset-bot[bot] commented 5 months ago

🦋 Changeset detected

Latest commit: e868666a2812e9ed931ec946a12f9005b4c57355

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

This PR includes changesets to release 1 package | Name | Type | | --------------- | ----- | | vite-imagetools | Patch |

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

benmccann commented 5 months ago

When restoring from the cache we only restore the image and not the metadata, which we re-read from the image. This could be an issue for other properties as well, which will end up missing when restore from the cache. Though in practice I'm not sure any properties besides format and width are used and I guess that format is the only one that doesn't match what is read from the image, so this fix is probably fine for most use cases. I wonder if anyone is relying on getting the directives from the metadata. Perhaps we should remove them as I don't think they need to be there and the output will be different depending on whether you restore from the cache or not

Anyway, I think this change is fine, but we should probably update the comment to better clarify exactly what is happening

AdamMerrifield commented 5 months ago

@benmccann comment has been updated, but I agree. I'm unsure of any other formats that something similar to this would occur with.

codecov[bot] commented 5 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 95.49%. Comparing base (37857f4) to head (e868666).

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #708 +/- ## ========================================== + Coverage 95.47% 95.49% +0.01% ========================================== Files 33 33 Lines 1283 1288 +5 Branches 224 226 +2 ========================================== + Hits 1225 1230 +5 Misses 58 58 ``` | [Flag](https://app.codecov.io/gh/JonasKruckenberg/imagetools/pull/708/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Jonas+Kruckenberg) | Coverage Δ | | |---|---|---| | [imagetools-core](https://app.codecov.io/gh/JonasKruckenberg/imagetools/pull/708/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Jonas+Kruckenberg) | `95.49% <100.00%> (+0.01%)` | :arrow_up: | | [vite-imagetools](https://app.codecov.io/gh/JonasKruckenberg/imagetools/pull/708/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Jonas+Kruckenberg) | `95.49% <100.00%> (+0.01%)` | :arrow_up: | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Jonas+Kruckenberg#carryforward-flags-in-the-pull-request-comment) to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.