Bundled themes: Stylesheets should be minified

[westonruter]

As commented on #49665:

Since core has CSS minification as part of the build process, shouldn't this be used by themes as well? This would help address things like #47925, where the stylesheet for T19 is 224K but after going through cssmin it is reduced to 196K. It doesn't really make sense for themes to serve the WP theme metadata CSS comment block to browsers. I should think that all .css files in a theme should also have a corresponding .min.css file. Maybe minification has been discouraged in the past to facilitate authors forking a theme and making changes to the CSS, with there being an unexpected result where the minified CSS wouldn't also be updated. Nevertheless, themes like T19, T20, and T21 are already using a build step for the CSS so authors shouldn't be directly modifying the style.css file directly anyway. They should be using the build process to re-generate the CSS instead.

Minifying the CSS will facilitate allowing the stylesheets to be inlined, for example in #63007. Without minification, enqueued CSS will more quickly reach the styles_inline_size_limit.

And as noted by @karmatosed:

I would also support minification for themes if this is correctly documented. I think historically it has been as you noted not done because of confusion where to update. However, in many real world situations this is part of the development process.

And as advised by @sabernhardt:

Classic themes from Twenty Ten to Twenty Twenty call get_stylesheet_uri() for their main stylesheets, so a child theme's style.css would replace its parent stylesheet.

I expect that a patch would need an is_child_theme() check to avoid looking for a minified file in the child theme directory (and/or printing the parent stylesheet inline).

[westonruter]

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.

[poena]

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.

[poena]

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?

[westonruter]

@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.

[poena]

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.

[westonruter]

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

[westonruter]

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.

[westonruter]

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).