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.

Sign up for GitHub

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

Jump to bottom

feat(vercel): Use Sharp in dev instead of Squoosh by default #8445

Merged
merged 10 commits into from
Sep 13, 2023
Merged

feat(vercel): Use Sharp in dev instead of Squoosh by default #8445

merged 10 commits into from
Sep 13, 2023

Conversation

Princesseuh
Copy link
Member

Changes

This changes the image service in dev to be Sharp powered like Astro's base service instead of Squoosh. The Squoosh one is still available if needed for people who can't install Sharp for various reasons.

This also fixes #8322

Testing

Added a test for SVGs and current tests still pass

Docs

The README was updated with the new config!

@Princesseuh Princesseuh requested a review from a team as a code owner September 7, 2023 12:26
@changeset-bot
Copy link

changeset-bot bot commented Sep 7, 2023 •
edited
Loading

🦋 Changeset detected

Latest commit: 121a249

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 the pkg: integration Related to any renderer integration (scope) label Sep 7, 2023
sarah11918
sarah11918 approved these changes Sep 13, 2023
View reviewed changes
Copy link
Member

@sarah11918 sarah11918 left a comment

Choose a reason for hiding this comment

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

Hi @Princesseuh -- Double check because it sounded like the changeset was saying the opposite of what the README was? Or I just missed something?

Made some quick suggestions below for your consideration!

packages/integrations/vercel/README.md Outdated Show resolved Hide resolved
.changeset/cold-flies-clean.md Outdated Show resolved Hide resolved
packages/integrations/vercel/README.md Show resolved Hide resolved
@Princesseuh
Copy link
Member Author

Hi @Princesseuh -- Double check because it sounded like the changeset was saying the opposite of what the README was? Or I just missed something?

Made some quick suggestions below for your consideration!

Sorry, this PR went through a lot and some things ended up being outdated while I thought everything was done with it 😔 I'd love if you could re-review this, as the option changed, instead of being a boolean for Squoosh, it now allows you to choose between Sharp and Squoosh or your own image service, which leads to more flexibility

@Princesseuh Princesseuh merged commit 9138037 into main Sep 13, 2023
12 of 13 checks passed
@Princesseuh Princesseuh deleted the fix/vercel-dev-image branch September 13, 2023 16:40
@astrobot-houston astrobot-houston mentioned this pull request Sep 13, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sarah11918 sarah11918 sarah11918 approved these changes

@bluwy bluwy bluwy left review comments

@ematipico ematipico ematipico approved these changes

Assignees
No one assigned
Labels
pkg: integration Related to any renderer integration (scope)
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Vercel image service breaks svg when used with astro:assets
4 participants
@Princesseuh @ematipico @sarah11918 @bluwy