Add tree shaking for attribute-extant selectors #2080

westonruter · 2019-04-07T22:08:31Z

As noted in #1492 (comment), one additional CSS reduction which can be done for Twenty Nineteen is eliminate the selectors that mention [focus-within] since those polyfill rules will not work in AMP (since they require JS). That eliminates 1.5KB of CSS.

It turns out that the infrastructure for doing attribute-based tree shaking was implemented in #1959 since it now checks if certain attributes are present on the page to know whether or not a given AMP component will be needed (e.g. dock for amp-video-docking). With this in hand, the CSS selector parsing can be extended to also extract the attribute names that are referenced in the selector and then at runtime check if any of those attributes exist in the document.

Note that the attribute selector tree shaking only considers if an attribute is mentioned. It does not check the value of the attribute. In other words, consider this CSS:

<style>
span[hidden] {
    display: none;
}
span[data-pos] {
    color: red;
}
span[data-pos="object"] {
    color: green;
}
span[data-pos^="interjection"] {
    color: green;
}
span[data-punctuation] {
    color: purple;
}
</style>
<span data-pos="noun">John</span>
<span data-pos="verb">kicks</span>
<span data-pos="adverb">quickly</span>

Only the last rule with the selector span[data-punctuation] would be tree-shaken. The others remain because the value is not taken into consideration. The reason is that the value, especially in the case of amp-bind, can be mutated at runtime (see bindings); this is also the case for the [hidden] selector since it and other such boolean attributes can be added/removed by AMP at runtime.

Results

Here are the savings for each core theme, along with other core styles also being included:

theme	before	after	diff	savings
twentyeleven	20293	20012	-281	-1.38%
twentyfifteen	38444	38163	-281	-0.73%
twentyfourteen	23982	23701	-281	-1.17%
twentynineteen	38986	37039	-1947	-4.99%
twentyseventeen	30003	29722	-281	-0.94%
twentysixteen	30625	30304	-321	-1.05%
twentyten	14692	14411	-281	-1.91%
twentythirteen	21617	21336	-281	-1.30%
twentytwelve	20736	20455	-281	-1.36%

As expected, the greatest savings is for Twenty Nineteen, where the focus-within rules are removed.

👉 Interestingly, with this change, the core theme with the most CSS after tree shaking is now Twenty Fifteen. Previously it was Twenty Nineteen. Twenty Sixteen remains in third place, followed by Twenty Seventeen.

With this along with #2079, the amount of CSS for Twenty Nineteen has gone down 4.6KB (-11.07%).

Here is the script (beware that running will clear out all widgets on an install):

#!/bin/bash

set -e
url=$1

if [[ -z "$url" ]]; then
	echo "Error: Must supply the URL to an AMP page as the first argument."
	exit 2
fi

function curl_grep_included_size {
	curl -s "$url" > /tmp/response
	if grep -s 'Total excluded size' /tmp/response >/dev/null; then
		echo '-1'
	else
		cat /tmp/response | grep 'Total included size' | sed 's/.*: //' | sed 's/ bytes.*//' | sed 's/,//'
	fi
}

echo "theme | before | after | diff | savings" > results.txt;
echo "------|--------|-------|------|--------" >> results.txt;

wp transient delete --all
for theme in $(wp theme list --field=name | egrep 'twenty' | grep -v '-'); do
	wp theme activate "$theme"
	wp widget reset --all # For consistency.
	git checkout develop
	before_bytes=$(curl_grep_included_size)
	git checkout add/attribute-selector-tree-shaking
	after_bytes=$(curl_grep_included_size)
	reduction_bytes=$(expr "$after_bytes" - "$before_bytes")
	reduction_percentage=$(php -r "printf( '%.2f%%', -( 1 - \$argv[1] / \$argv[2] ) * 100 );" "$after_bytes" "$before_bytes" )

	echo -e "$theme | $before_bytes | $after_bytes | $reduction_bytes | $reduction_percentage" >> results.txt
done

cat results.txt

Here is how I invoked:

lando ssh -c "./check-tree-shaker-savings.sh https://wordpressdev.lndo.site/2019/04/09/welcome-to-the-gutenberg-editor-4/?amp"

Todo

Add tests.
Determine which attributes should be excluded from tree-shaking (e.g. hidden), considering which attributes that AMP can add/remove.
Test themes to see which selectors are removed now with this change. Check savings for each.

See #1492.

westonruter · 2019-04-07T22:42:32Z

~~The removal of selectors like a[href^=http], a[href^="#"], #content[tabindex="-1"], and [type="button"] are erroneous.~~ (I'm no longer observing this.)
❓ The removal of selectors like .wp-block-quote[style*="text-align:right"] is expected since style attributes are removed, but the selector seems like a hack in the first place.
*~~The [hidden] selector needs to not be removed.~~
✅ Selectors with [focus-within] are being removed.

…th other extractions

westonruter · 2019-04-09T18:18:20Z

(Comment moved to description)

westonruter · 2019-04-09T18:32:07Z

Twenty Nineteen

As expected/desired, the JS-polyfilled [focus-within] selectors are successfully removed:

309,320d308
< .wp-block-quote[style*="text-align:right"], .wp-block-quote[style*="text-align: right"] {
< 	border-left: none;
< 	border-right: 4px solid #000;
< 	padding-left: 0;
< 	padding-right: 1em
< }
< 
< .wp-block-quote[style*="text-align:center"], .wp-block-quote[style*="text-align: center"] {
< 	border: none;
< 	padding-left: 0
< }
< 
959,967d946
< .main-navigation .main-menu .menu-item-has-children:not(.off-canvas)[focus-within] > .sub-menu {
< 	display: block;
< 	left: 0;
< 	margin-top: 0;
< 	opacity: 1;
< 	width: auto;
< 	min-width: 100%
< }
< 
978,993d956
< 	.main-navigation .main-menu .menu-item-has-children:not(.off-canvas)[focus-within] > .sub-menu {
< 		display: block;
< 		margin-top: 0;
< 		opacity: 1;
< 		position: absolute;
< 		left: 0;
< 		right: auto;
< 		top: auto;
< 		bottom: auto;
< 		height: auto;
< 		min-width: -moz-max-content;
< 		min-width: -webkit-max-content;
< 		min-width: max-content;
< 		transform: none
< 	}
< 
1011,1017d973
< .main-navigation .main-menu .menu-item-has-children:not(.off-canvas)[focus-within] > .sub-menu.hidden-links {
< 	left: 0;
< 	width: 100%;
< 	display: table;
< 	position: absolute
< }
< 
1026,1032d981
< 	.main-navigation .main-menu .menu-item-has-children:not(.off-canvas)[focus-within] > .sub-menu.hidden-links {
< 		right: 0;
< 		left: auto;
< 		display: block;
< 		width: max-content
< 	}
< 
1041,1044d989
< .main-navigation .main-menu .menu-item-has-children:not(.off-canvas)[focus-within] > .sub-menu .submenu-expand {
< 	display: none
< }
< 
1049,1057d993
< .main-navigation .main-menu .menu-item-has-children:not(.off-canvas)[focus-within] > .sub-menu .sub-menu {
< 	display: block;
< 	margin-top: inherit;
< 	position: relative;
< 	width: 100%;
< 	left: 0;
< 	opacity: 1
< }
< 
1068,1072d1003
< 	.main-navigation .main-menu .menu-item-has-children:not(.off-canvas)[focus-within] > .sub-menu .sub-menu {
< 		float: none;
< 		max-width: 100%
< 	}
< 
1079,1082d1009
< .main-navigation .main-menu .menu-item-has-children:not(.off-canvas)[focus-within] > .sub-menu .sub-menu {
< 	counter-reset: submenu
< }
< 
1087,1093d1013
< .main-navigation .main-menu .menu-item-has-children:not(.off-canvas)[focus-within] > .sub-menu .sub-menu > li > a::before {
< 	font-family: "NonBreakingSpaceOverride", "Hoefler Text", "Baskerville Old Face", Garamond, "Times New Roman", serif;
< 	font-weight: normal;
< 	content: "– " counters(submenu, "– ", none);
< 	counter-increment: submenu
< }
<

Twenty Sixteen

Note the #content[tabindex="-1"] selector appears to be dead code because the #content element does not have a tabindex in the theme. So that is why it removed.

309,320d308
< .wp-block-quote[style*="text-align:right"], .wp-block-quote[style*="text-align: right"] {
< 	border-left: none;
< 	border-right: 4px solid #000;
< 	padding-left: 0;
< 	padding-right: 1em
< }
< 
< .wp-block-quote[style*="text-align:center"], .wp-block-quote[style*="text-align: center"] {
< 	border: none;
< 	padding-left: 0
< }
< 
912,915d899
< #content[tabindex="-1"]:focus {
< 	outline: 0
< }
<

Other Themes

The other themes being active only have the following CSS removed from block-library/theme.css:

309,320d308
< .wp-block-quote[style*="text-align:right"], .wp-block-quote[style*="text-align: right"] {
< 	border-left: none;
< 	border-right: 4px solid #000;
< 	padding-left: 0;
< 	padding-right: 1em
< }
< 
< .wp-block-quote[style*="text-align:center"], .wp-block-quote[style*="text-align: center"] {
< 	border: none;
< 	padding-left: 0
< }
<

amedina · 2019-04-11T21:49:37Z

includes/sanitizers/class-amp-style-sanitizer.php

@@ -1456,41 +1507,64 @@ function( $selector ) {
 					$selectors   = explode( $between_selectors . ',', $split_stylesheet[ ++$i ] );
 					$declaration = $split_stylesheet[ ++$i ];

+					// @todo The following logic could be made much more robust of PHP-CSS-Parser did parsing of selectors. See <https://github.com/sabberworm/PHP-CSS-Parser/pull/138#issuecomment-418193262>.


nit pick: "more robust if PHP-CSS-Parser did.."

Fixed in ccf80c2

amedina

Tree-shaking is getting more and more precise!

westonruter added this to the v1.1 milestone Apr 7, 2019

googlebot added the cla: yes Signed the Google CLA label Apr 7, 2019

westonruter added 7 commits April 9, 2019 10:33

Add tree shaking for attribute-extant selectors

67b7e4c

Reduce size stored parsed selectors by using constants for array indices

62dbcc1

Fix regex for attribute value, and allow colons in names; fix tests

1091b01

Account for doubled-colons in pseudo classes; align tag extraction wi…

e5fcb63

…th other extractions

Add test for attribute_selectors

506a755

Improve format of parsed cached CSS transient key, putting amp at start

13fcdbf

Prevent tree-shaking for amp-bind-mutable boolean attributes; add tests

7e84717

westonruter force-pushed the add/attribute-selector-tree-shaking branch from 8f224a3 to 7e84717 Compare April 9, 2019 17:54

westonruter marked this pull request as ready for review April 9, 2019 18:36

westonruter requested a review from amedina April 9, 2019 18:49

Move attribute extraction after dynamic selector has been handled

4a78a2d

amedina reviewed Apr 11, 2019

View reviewed changes

Fix typo in command and add link to CssSelector issue

ccf80c2

amedina approved these changes Apr 11, 2019

View reviewed changes

westonruter merged commit 0b62c08 into develop Apr 11, 2019

westonruter deleted the add/attribute-selector-tree-shaking branch April 11, 2019 22:06

westonruter added the changelogged label Apr 12, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add tree shaking for attribute-extant selectors #2080

Add tree shaking for attribute-extant selectors #2080

westonruter commented Apr 7, 2019 •

edited

Loading

westonruter commented Apr 7, 2019 •

edited

Loading

westonruter commented Apr 9, 2019 •

edited

Loading

westonruter commented Apr 9, 2019

amedina Apr 11, 2019

westonruter Apr 11, 2019

amedina left a comment

Add tree shaking for attribute-extant selectors #2080

Add tree shaking for attribute-extant selectors #2080

Conversation

westonruter commented Apr 7, 2019 • edited Loading

Results

Todo

westonruter commented Apr 7, 2019 • edited Loading

westonruter commented Apr 9, 2019 • edited Loading

westonruter commented Apr 9, 2019

Twenty Nineteen

Twenty Sixteen

Other Themes

amedina Apr 11, 2019

Choose a reason for hiding this comment

westonruter Apr 11, 2019

Choose a reason for hiding this comment

amedina left a comment

Choose a reason for hiding this comment

westonruter commented Apr 7, 2019 •

edited

Loading

westonruter commented Apr 7, 2019 •

edited

Loading

westonruter commented Apr 9, 2019 •

edited

Loading