Optimize all images / pngs in core

[bradparbs]

Right now, we can optimize all of the images included in core and save about 177kb - not bad for no loss in quality, etc.

Ran the images through pngcrush, saved about 7% overall.

[bradparbs]

Oops - screwed up the diff, and then the naming - this is the right patch

[jeremyfelt]

I wonder if this is something that can be done through the build process with Grunt.

[bradparbs]

Good point - that would be a way better solution that this bandaid I'm doing here.

I know there's grunt packages for pngcrush / image optimizations - not sure if we want to require a package for it though.

[nacin]

Replying to bradparbs:

I know there's grunt packages for pngcrush / image optimizations - not sure if we want to require a package for it though.

Certainly — this is something we were planning to do.

[bradparbs]

Replying to nacin:

Replying to bradparbs:

I know there's grunt packages for pngcrush / image optimizations - not sure if we want to require a package for it though.

Certainly — this is something we were planning to do.

Cool - have it all set up in Grunt right now, but npm just broke - will have a patch as soon as I fix that locally.

For that patch - same ticket or make a new one?

[nacin]

Replying to bradparbs:

For that patch - same ticket or make a new one?

This one sounds great.

[jorbin]

Luckily, many of our files are already pretty optimized. This only saves saved 143.81 kB, but it also means we don't need to worry as much going forward.

This comes with a bit of a time cost the first time you run it, but subsequent builds are not noticeably different.

I'm using grunt-contrib-imagmin 0.4.1 rather than the newer 0.5 since 0.5 is using a new library that seems to throw errors on subsequent runs. I'm looking into that upstream.

[wonderboymusic]

we use this at the Times, runs every time we build, which is often, not a buzz kill. +1

[wonderboymusic]

Just ran this a bunch of times, it is only sorta slow the very first time it runs. After that, it's way faster than uglify. .2.diff unJorbins the whitespace.

Something to think about - could this just be a dev tool that generates smushed images that we then check in?

[nacin]

In 27172:

Dev tools: Add grunt imagemin:core task for optimizing images pre-commit.

Adds grunt-contrib-imagemin, so an npm install will be required.

props jorbin, wonderboymusic for initial patches.
see #25169.

[nacin]

In 27173:

Optimize 69 images (of 127) using the new Grunt task. (See [27172].)

fixes #25169.

[nacin]

I'm with wonderboymusic on this being a pre-commit tool rather than a build tool. It can essentially be in the same boat as autoprefixer (#27078). Maybe we'll hook it up to build later, but I think just having the task there is a good first step.

Since it'll be applying these changes directly to files in src, we need to be really restrictive of which images we apply them to. I made it imagemin:core as imagemin:themes makes sense in the future. That said, all of our theme screenshots are currently perfect, and the optimization of jpeg header images and such is something that is done by hand — balancing quality for file size. So rather than coming up with a rule that would essentially be just screenshots, I left it out for the moment.

[nacin]

In 27176:

Add grunt precommit for running front-end dev and test tasks before commit.

This includes autoprefixer, imagemin, jshint, and qunit.

props jorbin.
fixes #27121. see #25169 and #27078.

[netweb]

This causes numerous issues with npm install on Windows installs :(

I am reopening the ticket because of the console errors from running npm install. Previously to the addition of grunt-contrib-imagemin to package.json it was an error free build environment for Windows contributors.

Granted I don't think any of the core committers are running Windows to commit against core and the crux of this patch is for the the Grunt precommit task.

I tested both grunt-contrib-imagemin v0.4.1 and v0.5.0 and the errors are very similar for both versions.

Using v0.5.0 fails much more gracefully, you still get similar errors but subsequent runs of npm install does not try to reinstall the missing component.

npm install

Using: "grunt-contrib-imagemin" : "~0.4.1",

• npm install console output log: ​https://gist.github.com/ntwb/8dd0e411650142d721ce
• npm ERR! weird error 8
• npm ERR! not ok code 0

Using: `"grunt-contrib-imagemin" : "~0.5.0",

• npm install console output log: ​https://gist.github.com/ntwb/146da33743792ed85756

• npm WARN optional dep failed, continuing optipng-bin@0.3.1

grunt build

Using: "grunt-contrib-imagemin" : "~0.4.1",

• >> Local Npm module "grunt-contrib-imagemin" not found. Is it installed?
• Done, without errors.

Using: `"grunt-contrib-imagemin" : "~0.5.0",

• Done, without errors.

grunt precommit

Running "imagemin:core" (imagemin) task
Warning: Cannot find module 'optipng-bin' Use --force to continue.

Aborted due to warnings.
C:\xampp\htdocs\develop.wp.nw [trunk]> 

[netweb]

Replying to jorbin:

I'm using grunt-contrib-imagmin 0.4.1 rather than the newer 0.5 since 0.5 is using a new library that seems to throw errors on subsequent runs. I'm looking into that upstream.

From a quick bit of research it looks like the optional dependencies will fail a little more gracefully in v0.5.0 and going further upstream:

grunt-contrib-imagemin currently has a dependency of image-min v0.1.1

• v~0.1.1 has a dependency on optipng-bin
• v~0.1.2 has optipng-bin as an optional dependency
• v~0.1.3 removes the optipng-bin dependency in favour of pngquant-bin

I submitted a pull request ​https://github.com/gruntjs/grunt-contrib-imagemin/pull/152 to bump the grunt-contrib-imagemin dependency of image-min to v0.1.3

Maybe reclose this ticket and create a new one for updating core build to grunt-contrib-imagmin to v0.5.x once all the upstream issues are fixed? (I can survive with the errors in Windows for a while)

[azaozz]

Replying to netweb:

This causes numerous issues with npm install on Windows installs :(

Installs properly here on Win7.

However I'm 50/50 on whether this should be running automatically on each build. We are up to 46MB! of build tools (node_modules), 7,428 files in 1,699 directories... The chances for failed updates, platform specific bugs, local and platform differences, different output because of different version of the build tools, etc. are quite high. Our build process shouldn't be "fragile".

Considering that new images are added to core only a few times per cycle, seems this can be a "manual" task, i.e. should be run with grunt imagemin only when needed.

[netweb]

Replying to azaozz:

Replying to netweb:

This causes numerous issues with npm install on Windows installs :(

Installs properly here on Win7.

After spending the best part of last hour of constantly running npm install (after a npm cache clean) it looks like everything has installed correctly. Looks like the errors were further upstream in not downloading all the packages from https://registry.npmjs.org

All grunt tasks complete successfully with Done, without errors.

Replying to azaozz:

However I'm 50/50 on whether this should be running automatically on each build. We are up to 46MB! of build tools (node_modules), 7,428 files in 1,699 directories... The chances for failed updates, platform specific bugs, local and platform differences, different output because of different version of the build tools, etc. are quite high. Our build process shouldn't be "fragile".

Considering that new images are added to core only a few times per cycle, seems this can be a "manual" task, i.e. should be run with grunt imagemin only when needed.

In theory irrespective of platform we should all be using the same build tool/s version/s per the package.json file, however we do have to do an old fashioned grey matter remember to do task to run npm install occasionly to add/update any tool/s changes in package.json.

Forking ​github-update-checker could be a solution to this, compare versions in your local package.json with the latest in the WordPress Core repo.

The task is only run manually as part of the grunt precommit task, it is NOT part of the grunt build task.

// Pre-commit task. 
grunt.registerTask('precommit', 'Runs front-end dev/test tasks in preparation for a commit.',
['autoprefixer:core', 'imagemin:core', 'jshint', 'qunit:compiled']);

Added 'close' and this can be reclosed, worksforme :)

[jorbin]

However I'm 50/50 on whether this should be running automatically on each build. We are up to 46MB! of build tools (node_modules), 7,428 files in 1,699 directories... The chances for failed updates, platform specific bugs, local and platform differences, different output because of different version of the build tools, etc. are quite high. Our build process shouldn't be "fragile".

Considering that new images are added to core only a few times per cycle, seems this can be a "manual" task, i.e. should be run with grunt imagemin only when needed.

I completely agree that our build process shouldn't be fragile. It also shouldn't require much cognitive load or knowledge to run. Yes, updating images is not a common tasks, but you shouldn't need to think "Well I'm adding images, so I need to run this task, but I'm also modifying CSS, so I need to run this other task". By including all front end tasks that we expect to be run before a commit into one task, we reduce the cognitive load on developers and require them to only run one task.