fix(gatsby-transformer-sharp): removed unnecessary conversion to webp

[ungvert]

Description

In process-file.js conversion to webp happends two times:

• With Sharp after .resize() and .png()
• With imageminWebp in compressWebP()

If we convert png image with transparency to webp and then we pass this webp image to imageminWebp - it returns image with weird artefacts and black background (tested on Windows 10).

I don't comprehend why we doing the conversion two times. Parameters of conversion seems identical.

Related Issues

Fixes #14497

[polarathene]

EDIT: Cleaned up my reviews and discussions for future review to focus on what's important/relevant.

Note: In my own project imagemin-webp has:

• Minimal file reduction benefits
• Results in poorer quality(blocky artifacts from compound compression vs single pass lower quality setting)
• Increased build times when processing images(50% longer).

Old review

The conditionals for `transformArgs.toFormat` seem to be more of a problem. At least for webp, that's where quality is taking a dive for minor savings. With the conditional removed, and the webp processing from sharp kept, the file is still saved to disk.

This area of code is unfamiliar to me, I assume imagemin is being used as a non-optional additional compression stage, but if it's attempting to recompress an image with matching quality setting, that seems like a bad idea?

Opting out of the compression stage reduces processing by ~30% for my project, 30 seconds down to 18. The PR would probably be better to make compression opt-in/out(depending on core devs opinion), I'd personally prefer the additional compression being opt-in, unless proven that reducing quality further and extra processing time is worth what appears to be marginal gains(and potential bugs?).

[polarathene]

I do not find this to be an acceptable change or fix.

[polarathene]

imagemin-webp was added by mistake and somehow made it through the review process.. details below:

Click to see investigation into why imagemin is used for each format

I've gone through the history via git blame and tracked down where imagemin was added, the commit message and large PR isn't particularly insightful. This change was done at the end of 2016, viewing the PR individual commit history, a TODO message is found for why imagemin was added:

gatsby/packages/gatsby-sharp/index.js

Lines 36 to 37 in 1f41853

	// TODO compress pngs with imagemin-pngquant as libvps doesn't do
	// lossy compression for pngs.

That's the sole purpose we have it.

In 2018, that still appeared to be the case with a lack of compression support for PNG. Looking at the changelog for sharp, we can see support for libimagequant arrived at Jan 2019, along with metadata stripping(default) and in June 2020 a further optimization for indexed palettes(auto enabled if quality/colors/dither options are set).

libimagequant needs to be provided on the system, it's not distributed with libvips or sharp binaries due to GPL license(there is a way to get a linux binary via env var however). That is required to benefit from optimizations like the indexed palettes, as well as the quality setting having any actual impact apparently for PNG via sharp.

Thus imagemin may still be relevant for compressing PNGs.

---

mozJPEG as an alternative encoder for providing better compression. This is supported in a manner that it only uses sharp or mozJPEG, not both.

When imagemin webp was added, presumably under the assumption that since other formats used imagemin, this should too. Kyle completely overlooked it himself while reviewing the PR.. There doesn't appear to be any reason/benefit to use this plugin.

The imagemin provides a variety of alternative PNG options as well as some for jpeg, the addition of imagemin-webp was likely a a mistake so it should be removed entirely.

[polarathene]

As per investigation above, please remove compressWebP() and it's imagemin-webp dependency in it's entirety.

Optional compression for PNG can be raised in a new PR if anyone creates a new issue about it being a problem.

[ungvert]

@polarathene Wow. Great investigation! I understand more clearer what you meant in previous comments. Thank you for that!

I made new commit which deletes everything related to imagemin-webp.

[polarathene]

LGTM! Thanks for tracking this down and sending in the PR :)

[wardpeet]

Thanks, @ungvert for this awesome change! I saw some size reductions and build time wins.

@polarathene awesome review! This makes my life so much easier! <3

[gatsbot]

Holy buckets, @ungvert — we just merged your PR to Gatsby! 💪💜

Gatsby is built by awesome people like you. Let us say “thanks” in two ways:

1. We’d like to send you some Gatsby swag. As a token of our appreciation, you can go to the Gatsby Swag Store and log in with your GitHub account to get a coupon code good for one free piece of swag. We’ve got Gatsby t-shirts, stickers, hats, scrunchies, and much more. (You can also unlock even more free swag with 5 contributions — wink wink nudge nudge.) See gatsby.dev/swag for details.
2. We just invited you to join the Gatsby organization on GitHub. This will add you to our team of maintainers. Accept the invite by visiting https://github.com/orgs/gatsbyjs/invitation. By joining the team, you’ll be able to label issues, review pull requests, and merge approved pull requests.

If there’s anything we can do to help, please don’t hesitate to reach out to us: tweet at @gatsbyjs and we’ll come a-runnin’.

Thanks again!