-
Notifications
You must be signed in to change notification settings - Fork 382
Description
The AMP plugin settings screen has an option for whether or not to enable tree shaking:
This option is enabled by default:
| 'accept_tree_shaking' => true, |
And all core themes force it to be enabled:
| 'removed_unused_css_rules' => true, |
When tree shaking was first introduced, the idea behind this option was to guard against scenarios where the tree shaker erroneously removed rules that actually applied to the page, specifically for themes that have been crafted to not have more than 50KB of CSS. The reality is that core + themes + plugins will almost always generate more than 50KB, as evidenced by the above defaults. Also, there have been significant improvements to the tree shaker to prevent over-eager removal of rules, including:
- Account for escaped chars in CSS class names #2260: Account for escaped chars in CSS class names
- Account for toggleClass when obtaining used class names for tree shaking #2328: Account for toggleClass when obtaining used class names for tree shaking
- Prevent tree-shaking class names for amp-geo when component on page #2017: Prevent tree-shaking class names for amp-geo when component on page
- Fix tree shaking for dynamically-added class names #1959: Fix tree shaking for dynamically-added class names
So since this option is not really needed anymore, I suggest we remove it entirely and always perform tree shaking. Again, decisions not options.
