Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix image regeneration after changing image service #6680

Merged
merged 1 commit into from Mar 28, 2023

Conversation

koriwi
Copy link
Contributor

@koriwi koriwi commented Mar 27, 2023

Changes

I experienced very strange behaviour which resulted in images being rotated or not depending on how i change the widths or alt tags of images. I tracked this down to the image cache not being regenerated after i switched to sharp.
The behaviour of sharp differs in some ways. In my case sharp rotates the image while removing the exif data. squoosh does remove the exif data without rotating the images. When my image was the exact same image as before I got served the cached version built previously by squoosh so some images were still up side down etc. Changing the alt tag invalidated the cache, as it is used for hashing.

  • Don't use the alt tag for hashing as it has nothing to do with the actual content of the image
  • Include the serviceEntryPoint in the hashKey to invalidate the image cache when changing from squoosh to sharp for example.

Testing

I did not add unit tests as my experience in this repo is very slim right now.
I verified that the behaviour is as intended by reproducing the mentioned behaviour manually.

Docs

Now the behaviour is as intended so i don't think with a fix like this we need any additional docs

@changeset-bot
Copy link

changeset-bot bot commented Mar 27, 2023

🦋 Changeset detected

Latest commit: c0f21ad

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

@github-actions github-actions bot added pkg: integration Related to any renderer integration (scope) pkg: astro Related to the core `astro` package (scope) labels Mar 27, 2023
Copy link
Member

@Princesseuh Princesseuh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great, thank you for making the change in both astro:assets and @astrojs/image!

@koriwi
Copy link
Contributor Author

koriwi commented Mar 28, 2023

My pleasure. I forgot it in the first commit and then packed both changes into one after realizing so only one changeset is necessary

@Princesseuh Princesseuh merged commit 3863364 into withastro:main Mar 28, 2023
13 checks passed
@astrobot-houston astrobot-houston mentioned this pull request Mar 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pkg: astro Related to the core `astro` package (scope) pkg: integration Related to any renderer integration (scope)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants