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.