Add a way for devs to enable css tree-shaking

[smokeyfro]

Hi guys,

First off let me start by saying how much we appreciate all the effort you guys are putting into the AMP for WP plugin - it's come a long way since the early beta version I first tested. We decided to take the plunge a few months ago to go full AMP and while it's come with it's own set of challenges, we're all pretty stoked with the outcome.

One of the things I've been dying to try out is the new css tree-shaking after seeing the demo in the Google IO preso. I believe the tree-shaking only kicks in after the 50kb limit has been exceded. It would be great if there was a way for devs to enable the tree-shaking by default? Obviously it would need to be tested in a staging environment first, but I'm really curious to how much it shaves off and at present I have no way of testing, since all the pages are below the 50kb limit.

It also opens the door to using front-end frameworks without incurring the extra bloat. Part of the challenge going full AMP was staying below the 50kb limit, which included replacing previously used UIkit components with vanilla css. Not that this is a bad thing, but having the option to still use front-end frameworks, while keeping the CSS at it's minimum would probably help with amp adoption by devs.

TIA,
Chris
Ps, we just launched https://winefolly.com, which is using the plugin in full AMP mode :)

[westonruter]

Thanks a lot, Chris.

I believe the tree-shaking only kicks in after the 50kb limit has been exceded. It would be great if there was a way for devs to enable the tree-shaking by default? Obviously it would need to be tested in a staging environment first, but I'm really curious to how much it shaves off and at present I have no way of testing, since all the pages are below the 50kb limit.

That's right. It only kicks in if the CSS is over 50KB. The reason for conditionally enabling it is that there is a chance that some rules could be deleted from the stylesheet which actually are needed by the page at some point. Specifically when you have dynamic content being added to the page, such as via amp-live-list, it's possible that this could result in new content added to the page that would end up unstyled because the tree shaking would have removed the rules for elements that weren't initially on the page. The way to mitigate this in your CSS is to explicitly include amp-live-list as an ancestor element in the selector. But this depends on the theme author ensuring that they write the CSS in this way. So that is why tree shaking is not enabled by default.

What's more is that when tree shaking is needed, by default it is going to be raised as a validation error and block the response from being served as AMP. What this means is that a user will need to opt-in to tree shaking the first time the error is encountered. There would be a programmatic way to automatically allow tree shaking via the brand new amp_validation_error_sanitized filter (merged yesterday):

https://github.com/Automattic/amp-wp/blob/ab7204af35a8eb68bd8840a2e3531df8ae73c21f/includes/validation/class-amp-validation-manager.php#L435-L454

This filter however won't cause tree shaking to be invoked even when less than 50KB. There is a separate way you use to do that now, actually:

add_filter( 'amp_content_sanitizers', function( $sanitizers ) {
    $sanitizers['AMP_Style_Sanitizer']['remove_unused_rules'] = 'always';
    return $sanitizers;
} );

The default value for the remove_unused_rules arg is “sometimes”:

https://github.com/Automattic/amp-wp/blob/ab7204af35a8eb68bd8840a2e3531df8ae73c21f/includes/sanitizers/class-amp-style-sanitizer.php#L50

By changing it to “always” then this should cause tree shaking to always be performed. Give that a try. For the ampnews native AMP theme, this reduces CSS from 35KB to 13KB on the homepage.

I've been considering that we should add a new AMP admin screen to manage settings regarding theme support, including:

• Forcibly enable amp theme support for the current theme, even if it lacks add_theme_support('amp'). This will be particularly useful as the plugin will support all the core themes in AMP by default, and it would allow
• Decide whether to have paired mode.
• Preemptively opt-in to tree shaking, since it is going to be needed in the vast majority of cases anyway. Options here could be “Ask me” (block AMP without accepting), “As needed” (sometimes), and “Always”.

Ps, we just launched https://winefolly.com, which is using the plugin in full AMP mode :)

Love it 😄

[smokeyfro]

Hi Weston,

Thanks for the detailed response and all the info :) Good to know.

Thierry and I enabled it on the WineFolly dev install and saw about 20-30% reduction. I'm going to do a lot more testing / comparisons between the live and dev sites and will compile my findings in Google Sheet. I'm happy to share the results if you're interested.

Having a place to configure the site specific amp stuff would be great. Happy to help out with testing where ever needed.

Cheers,
Chris

[westonruter]

@devignerforhire FYI: As of v1.2 tree shaking will be done regardless of whether or not the original CSS is >50KB. See #2501.