Compiled block styles contain a lot of unnecessary comment blocks

[aristath]

All block styles contain ~75 lines of comments which are not needed. For example you can take a look at ​https://github.com/WordPress/WordPress/blob/6.0.1/wp-includes/blocks/paragraph/style.css
These comments get added to all non-minified stylesheets and they don't make any sense.

Removing these when possible will reduce the overall package size, make debugging easier since there will be significantly less noise when viewing the source, and improve performance when SCRIPT_DEBUG is true, since the loaded assets will be smaller.

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

This appears to modify some files in the wp-includes/blocks directory, causing ​some tests to fail 🤔

Ensure version-controlled files are not modified or deleted during building and cleaning
diff --git a/src/wp-includes/blocks/site-logo/style-rtl.css b/src/wp-includes/blocks/site-logo/style-rtl.css
index 9062fc..c4546 100644
--- a/src/wp-includes/blocks/site-logo/style-rtl.css
+++ b/src/wp-includes/blocks/site-logo/style-rtl.css

This appears to modify some files in the wp-includes/blocks directory, causing ​some tests to fail 🤔

Ensure version-controlled files are not modified or deleted during building and cleaning
diff --git a/src/wp-includes/blocks/site-logo/style-rtl.css b/src/wp-includes/blocks/site-logo/style-rtl.css
index 9062fc..c4546 100644
--- a/src/wp-includes/blocks/site-logo/style-rtl.css
+++ b/src/wp-includes/blocks/site-logo/style-rtl.css

Yes, that is expected. The patch modifies the compilation of block stylesheets to remove comments from them. Looking at the failing tests, the modified stylesheets are what they should be 👍

If I understand correctly, it might be possible to make the same change in Gutenberg, so that when these CSS files are backported to core as part of package updates, they would no longer be flagged as modified by this PR, and then it can be merged 🤔

Gutenberg PR: ​https://github.com/WordPress/gutenberg/pull/43177

[adamsilverstein]

If I understand correctly, it might be possible to make the same change in Gutenberg, so that when these CSS files are backported to core as part of package updates, they would no longer be flagged as modified by this PR, and then it can be merged 🤔

Hi @aristath & @SergeyBiryukov - can you clarify, are the changes here needed in core, in Gutenberg (or both?)? I see some linting (?) failures on the Gutenberg PR.

Also, should we go ahead and punt this to 6.2 or is the change safe enough to include in 6.1?

[aristath]

Hey Adam!

The reason we added the Gutenberg PR, is because the tests here are failing, and we don't know if it's because of the Gutenberg files being different or not.
The failing test is Ensure version-controlled files are not modified or deleted during building and cleaning, which would suggest that this is the case.

The Gutenberg PR is not necessary per se, it's only there to resolve the failing test in the Core PR.
Ideally, Gutenberg would remain as-is and Core would apply the changes in this PR.
The change is 100% safe for 6.1 - provided we find a way to fix the failing test.

[audrasjb]

With WP 6.1 RC 1 scheduled today (Oct 11, 2022), there is not much time left to address this ticket. Let's move it to the next minor milestone, 6.1.1.

Ps: if you are a committer and if you were about to send a patch and if you feel it is realistic to commit it in the next one or two hours, please feel free to move this ticket back to milestone 6.1.

[cbravobernal]

Can we take a look at this ticket so we can land it in 6.1? Changes required in the editor will be backported on RC3.

Thank you very much!

[SergeyBiryukov]

I suppose this can be merged after the package updates for RC3, so that the affected files in wp-includes/blocks/ are no longer flagged as modified.

[hellofromTonya]

After discussing with other Core Committers (see ​here in Slack and ​here in the GB comment), consensus is:

It's too late in the 6.1 cycle with RC3 happening in a few minutes and final release next week. Moving this ticket to 6.2 cycle.

[flixos90]

It looks like there has been no updates here since this was moved to the 6.2 milestone. @aristath Is this something we can realistically complete before beta, or should it be moved to the following release?

I see this is filed as a bug, but that's rather stretching it. While this is certainly worth optimizing, I don't think it qualifies as a bug - nothing is broken, as far as I understand. Would you mind changing it to be an enhancement?

[aristath]

It looks like there has been no updates here since this was moved to the 6.2 milestone. @aristath Is this something we can realistically complete before beta, or should it be moved to the following release?

I believe we can safely apply this change. Nothing breaks, and the styles simply get optimized.

I see this is filed as a bug, but that's rather stretching it. While this is certainly worth optimizing, I don't think it qualifies as a bug - nothing is broken, as far as I understand. Would you mind changing it to be an enhancement?

Fair point. Done

[flixos90]

@aristath Sounds good, thanks. I've just reviewed the PR. With another approval, I'd be confident to commit this.

Looks good to me 👍

It doesn't change any behavior for production, so we can proceed with it 👍🏻

[flixos90]

In 55193:

Editor: Remove unnecessary CSS comments from compiled styles.

Props aristath, adamsilverstein, SergeyBiryukov, gziolo.
Fixes #56388.

Committed in https://core.trac.wordpress.org/changeset/55193