Make WordPress Core

Opened 9 months ago

Closed 3 months ago

Last modified 3 months ago

#58996 closed enhancement (fixed)

Expand "imagemin" Grunt task to cover default themes

Reported by: azaozz's profile azaozz Owned by: swissspidy's profile swissspidy
Milestone: 6.5 Priority: normal
Severity: normal Version:
Component: Build/Test Tools Keywords: has-patch commit
Focuses: Cc:

Description

WordPress uses Imagemin in Grunt to minify new images. However it seems the last time new (non svg) images were added was 7 years ago. See:

https://core.trac.wordpress.org/browser/trunk/src/wp-includes/images?order=date&desc=1
https://core.trac.wordpress.org/browser/trunk/src/wp-admin/images?order=date&desc=1

Seems the imagemin task in Grunt has been redundant for a long time. Perhaps would be good to retire it and go back to minifying new images "by hand" before committing them, if non-svg images are ever added again.

Attachments (1)

58996.diff (426 bytes) - added by desrosj 8 months ago.

Download all attachments as: .zip

Change History (13)

@desrosj
8 months ago

#1 follow-up: @desrosj
8 months ago

  • Keywords has-patch 2nd-opinion added

I'm not sure that relying on "by hand" is the best path here. Even for experienced contributors and committers, it's very easy to forget steps like this when it's not often images are added.

It's true that within those two directories images have not been added, but there have been lots of images added to the wp-content/themes directory. I think it's probably worth looking at expanding the imagemin task to include themes.

I attached a patch that expands to theme directories, and running npm run grunt precommit:image results in 106 images being minified, and seems to save ~1KB per image within the theme.

The fact that the images within themes can be further minified is a good example of forgetting to manually perform minifying prior to committing.

#2 in reply to: ↑ 1 @SergeyBiryukov
8 months ago

Replying to desrosj:

It's true that within those two directories images have not been added, but there have been lots of images added to the wp-content/themes directory. I think it's probably worth looking at expanding the imagemin task to include themes.

I agree, it seems like the task would still be beneficial for themes.

#3 @jorbin
6 months ago

  • Milestone changed from 6.4 to 6.5

I agree with @desrosj that we should expand this rather than eliminate it.

However, I do think this might be better for 6.5 as we are now in RC. Feel free to move back to 6.4 if someone feels strongly otherwise.

#4 @swissspidy
3 months ago

  • Keywords commit added; 2nd-opinion removed
  • Owner set to swissspidy
  • Status changed from new to accepted

#5 @swissspidy
3 months ago

  • Summary changed from Consider removing the "imagemin" task from Grunt to Expand "imagemin" Grunt task to cover default themes

#6 @swissspidy
3 months ago

  • Resolution set to fixed
  • Status changed from accepted to closed

In 57322:

Build/Test Tools: Expand "imagemin" Grunt task to cover default themes.

Runs npm run grunt precommit:image to minify/compress images in the repository.

Props desrosj.
Fixes #58996.

#7 @poena
3 months ago

Please correct me but did this not make the image sizes larger, not smaller?

#8 @swissspidy
3 months ago

It shouldn‘t have 👀 At least I wouldn‘t expect this script to do that.

Do you have an example of an image that got bigger?

Maybe we need to tweak a config or update a dependency somewhere.

#10 @afercia
3 months ago

@swissspidy hello. Looking at the commit on GitHub together with @SergeyBiryukov and @poena we did notice most of the images actually have a _larger_ size now.

See https://github.com/WordPress/wordpress-develop/commit/5cd843e235ba9fa40cf716c7e52abc823a48ba7c

I'm guessing that also means the larger images have now a _lower_ quality because the added pixels are the result of some interpolation.

We're all for automating this process but ideally it should make sure to not make images size bigger :)

#11 @swissspidy
3 months ago

Thanks, very helpful!

I'm guessing that also means the larger images have now a _lower_ quality because the added pixels are the result of some interpolation.

imagemin is supposed to do lossless compression according to the documentation, so quality shouldn't decrease.


I am now trying npm run grunt imagemin again with different configuration and even with mozjpeg but weirdly enough I now can't get it to process those files again 🤔

Of course I could manually optimize them again but then what's the point.

#12 @afercia
3 months ago

imagemin is supposed to do lossless compression according to the documentation, so quality shouldn't decrease.

Maybe I wasn't clear.
When the image size actually increases, that means that some pixels are _added_ to the image (which is already optimized, it's not the original high-quality image). That means imagemin actualy 'rebuilds' some pixels adn adds them to the image, as a result of some interpolation process. These 'rebuilt' pixels can't in any way _improve_ the quality of the image.

Note: See TracTickets for help on using tickets.