Bundled themes: Stylesheets should be minified in block themes

In a ​PR for #63007 I've added initial minification for the style.css in Twenty Twenty-Two and Twenty Twenty-Five. I'm not sure this is the best approach, however. I implemented it in core's overall gruntfile. Perhaps the minification should be part of each theme's build step if they have one.

Also, should we try to generate source maps? Granted this isn't really necessary when SCRIPT_DEBUG can be enabled.

Using the core tool is a no-go because most people will not have a development version of WordPress.
The themes also support versions below 6.9, so if only the core tools are updated, the tools won't be available for all.

Or, could WordPress minify the files without the files needing to be in the theme?
Could it be introduced on a core level, and only flagged in the theme?

@poena This is about using the same build process as core already has for minifying CSS files, but just extending it to bundled themes where currently core is just minifying CSS in wp-admin and wp-includes. By using a development version of WordPress, do you mean running from src without doing a build? This would still be supported since with SCRIPT_DEBUG the non-minified files would be used, thus not necessitating doing a build (via core's existing grunt build).

The minified CSS files would be built with WordPress, and the static files would be shipped with the themes. There would not be any dynamic minification process, so there's no back-compat issue.

You can see an example of the scope of changes, specifically for Twenty Twenty-Two and Twenty Twenty-Five in this ​PR for #63007. The scope of changes for that ticket is primarily about being able to inline the small style.css files (the addition of the calls to wp_style_add_data() in the PR), but minification is included there because there is a limit to the amount of CSS that can be inlined.

I don't think we are understanding each other.

For a core committer, using the existing (updated) script to minify the files is not a problem, besides the increased maintainance burden.

But users who are using a version of WordPress where no tools are included, what they will experience is that they are making changes to style.css and the changes are not being applied, because they will not have the required knowledge nor tools.

@poena I've replied on the other ticket: https://core.trac.wordpress.org/ticket/63007#comment:14

Re-posting relevant parts of my comment from #63007 in reply to @poena:

When the ticket was opened, it included this comment:
"I'm not sure if this is the right place to introduce the minification, as perhaps it should be part of the theme's own build process." [...]
But then without further discussion it seems to have been decided that it must be the minification script that is in core. Which, would require knowledge of this script. I believe that is not an incorrect claim.

Yeah, I included minification of the style.css files for the block themes here because when they are inlined they should be minified to ensure they don't exceed the styles_inline_size_limit limit. So perhaps #63012 should be addressed first as a dependency for this ticket.

I added this as one point of my comment, in addition to documentation. [...]
And the comment about documentation has not been addressed at all.

In regards to documentation, the ​documentation for testing a patch already instructs to run grunt build. So any additional documentation specifically for testing themes should include this if it doesn't already.

Secondly, I described a scenario where a user repurposes the theme and adds CSS to style.css.
It does not matter if the user uses a text editor or the built in theme file editor.

What matters is that style.css is not enqueued, instead the minified files is. And the user does not have knowledge of that they need to edit the minified files, or how, or knowledge about script debug.
So the users changes are not working, and it is not a positive experience for them.

If someone forks a theme and modifies the style.css then this indeed would be an issue. I described this in a previous comment on #49665 where I wondered if the forking scenario was a reason why minification hasn't been done in the past. Nevertheless, there are themes today that already have a build step for their CSS, namely Twenty Nineteen and Twenty Twenty-One. So technically if someone forks those themes they already should not be using the theme file editor to edit the style.css file directly but rather to edit the underlying SASS files and then run npm install && npm run build in the theme. For themes that use SASS and for those which have minified CSS files, there should probably be a big notice comment in the style.css that instructs the users to run npm run build after making a change, and that for themes using SASS that the change should be made to the SASS files directly. For minification, the comment can say something like:

/*
 * NOTICE: 
 * This file is served minified to improve performance. In order to see edits 
 * to this file on the frontend, either enable the `SCRIPT_DEBUG` constant or 
 * run `npm run build` to regenerate `style.min.css`. Alternatively, if you do
 * not want to use minified version, then remove the `style.min.css` version from 
 * being enqueued in `functions.php`.
 */

Every theme would need to include their own package.json which has the npm run build command that which would result in the minified files being generated so that they wouldn't have to rely on core's build process to generate those files.

We discussed this today during the bi-weekly Performance bug scrub.

To facilitate the editing of CSS files in the theme file editor, we could do the CSS minification whenever a *.css file is modified to then update the corresponding *.min.css file. We can show a notice when editing a *.css file when there is a corresponding *.min.css file that the latter will be updated with a minified version of the former automatically.

The three scenarios I see for editing CSS files that would need re-minification are:

1. Distributed theme stylesheets are minified automatically as part of the core build process.
2. Stylesheets edited by a developer on the filesystem are minified via npm run build thanks to a package.json shipped with the theme (as is already done in T19, T20, T21). The file header comment in the CSS file must include instructions for how to re-minify. If a developer does not want to do this, they can enable SCRIPT_DEBUG.
3. A stylesheet modified in the theme file editor is automatically minified to update any corresponding *.min.css file. This minification could be done client-side (in JavaScript) or server-side (in PHP e.g. via ​MyIntervals/PHP-CSS-Parser).

The automatic re-minification process might be able to be employed in the second scenario as well (when a developer edits the file), but this could be out of scope here. For example, if wp_get_environment_type() returns local/development, or if wp_get_development_mode() returns theme/all, then with each page load, core could read the contents of the stylesheet, compute an MD5 hash, and store this in a transient. If at the next page load the MD5 hash changes, then it could re-minify the CSS file. This would depend on there being a PHP-based CSS minifier.

Before doing any of this, I'll work on gathering metrics on what the performance gains are with minified stylesheets (and inlined too).

Once this is is completed, it will unblock these tickets:

• #58519
• #63007

TODO:

• [x] Stylesheet minification for Twenty Twenty-Two and Twenty Twenty-Five (Block themes).
• [x] Stylesheet minification for Twenty Nineteen, Twenty Twenty, and Twenty Twenty-One themes (Classic themes with existing build setup).
• [ ] Stylesheet minification for classic themes without existing build setup.
• [ ] Use Webpack instead of Gruntfile for a more streamlined workflow.
• [ ] Update related GitHub workflow for handling stylesheet minification.
• [ ] Add server-side or client-side minification of stylesheets.

Trac ticket: https://core.trac.wordpress.org/ticket/63012

cc: @westonruter

@b1ink0 To make this easier to review and initially land, how about this initial PR be limited to just the block themes. To that end, could you revert the changes to all themes other than Twenty Twenty-Two and Twenty Twenty-Five? Once this is merged, we can then follow up with restoring the changes you currently have. Once we establish the pattern with the block themes, then it will be easier to then follow up with classic themes.

The highest priority is for the block themes well, since these are the ones which will be eligible to have their theme styles inlined, since they are very small. The classic themes will likely not be eligible for inlining, so the impact of minification will be less.

Results are looking good so far!

# Twenty Twenty-Five

The stylesheet's size is reduced from 3.1 kB down to 0.8 kB, or ~75% smaller.

Before:

After:

# Twenty Twenty-Two

The stylesheet's size is reduced from 6.6 kB down to 2.7 kB, or ~60% smaller.

Can this be marked as ready for review as opposed to being a draft? It would be good to get more contributors who work on bundled themes to also look. It's looking good to me!

• Update related GitHub workflow for handling stylesheet minification.

What is this about?

• Update related GitHub workflow for handling stylesheet minification.

What is this about?

Need to work on the deployment workflow of themes with a build step. There is already logic for handling T19, T20, and TT1, but need to add support for TT2 and TT5. Will do it today.

@poena This is largely ready for review: ​https://github.com/WordPress/wordpress-develop/pull/10081

Note the initial scope has been reduced to just the block themes, TT2 and TT5.

We can follow up with non-block themes either with an additional commit for this ticket, or open a new ticket later.

The stylesheets for block themes are the most important to minify because they are the best candidates for inlining, given they are small. The stylesheets for classic themes are much much larger, so it is unlikely they would be suitable for inlining, so minifying them would not result in the desired ideal of eliminating render-blocking external stylesheets.

Need to work on the deployment workflow of themes with a build step. There is already logic for handling T19, T20, and TT1, but need to add support for TT2 and TT5.

@b1ink0 I've added these in f908cca and b83b6d0

This is looking and testing well for me! The ​PR should have someone on the themes team to look it over as well.