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

feat: remove the squoosh image service #11770

Merged
merged 6 commits into from
Aug 20, 2024
Merged

feat: remove the squoosh image service #11770

merged 6 commits into from
Aug 20, 2024

Conversation

Princesseuh
Copy link
Member

@Princesseuh Princesseuh commented Aug 19, 2024

Changes

This PR removes the Squoosh image service. Since it's not the default anymore, the underlying library is unmaintained, it has memory leak issues, it's very slow, it's barely used, Sharp has WASM support now, generally has no purpose, etc. there's no big reasons to keep it around.

Testing

N/A

Docs

Will do!

Copy link

changeset-bot bot commented Aug 19, 2024

🦋 Changeset detected

Latest commit: fb00c39

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) pr: docs A PR that includes documentation for review labels Aug 19, 2024
@Princesseuh Princesseuh marked this pull request as ready for review August 19, 2024 11:46
@github-actions github-actions bot added the semver: major Change triggers a `major` release label Aug 19, 2024
Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

This PR is blocked because it contains a major changeset. A reviewer will merge this at the next release if approved.

Comment on lines 114 to 124
...(copyWASM
? [
copy({
resolveFrom: 'cwd',
assets: {
from: ['./src/assets/services/vendor/squoosh/**/*.wasm'],
to: ['./dist/assets/services/vendor/squoosh'],
},
}),
]
: []),
Copy link
Member

Choose a reason for hiding this comment

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

FYI I'm also changing this in main at https://github.com/withastro/astro/pull/11769/files#diff-3295a1ee2ce5eaeef758fac7b3eea7c5413dc7ccc11b42afcfbd25f0f384d753R68 that moves this for the build too, but looks like after this we can completely remove it, the --copy-wasm command, and esbuild-plugin-copy.

Letting you know in case you hit merge conflicts

@Princesseuh Princesseuh merged commit cfa6a47 into next Aug 20, 2024
5 checks passed
@Princesseuh Princesseuh deleted the feat/remove-squoosh branch August 20, 2024 13:12
ematipico pushed a commit that referenced this pull request Aug 21, 2024
* feat: remove the squoosh image service

* fix: build

* chore: changeset
ematipico pushed a commit that referenced this pull request Feb 5, 2025
* feat: remove the squoosh image service

* fix: build

* chore: changeset
ematipico pushed a commit that referenced this pull request Feb 5, 2025
* feat: remove the squoosh image service

* fix: build

* chore: changeset
ematipico pushed a commit that referenced this pull request Feb 6, 2025
* feat: remove the squoosh image service

* fix: build

* chore: changeset
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) pr: docs A PR that includes documentation for review semver: major Change triggers a `major` release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants