Consolidate init and skip optimizing in AMP mode#57
Conversation
| exit; | ||
| } | ||
|
|
||
| require_once __DIR__ . '/settings.php'; |
There was a problem hiding this comment.
The settings don't need to be conditionally loaded. Let's load them unconditionally.
|
|
||
| return !! get_option( 'page_optimize-js', page_optimize_js_default() ); | ||
| $should_concat = !! get_option( 'page_optimize-js', page_optimize_js_default() ); | ||
| return apply_filters( 'page_optimize_should_concat_js', $should_concat ); |
There was a problem hiding this comment.
While we're revisiting conditional loading, we might as well support a first-class should-concat filter for JS.
|
|
||
| return !! get_option( 'page_optimize-css', page_optimize_css_default() ); | ||
| $should_concat = !! get_option( 'page_optimize-css', page_optimize_css_default() ); | ||
| return apply_filters( 'page_optimize_should_concat_css', $should_concat ); |
There was a problem hiding this comment.
While we're revisiting conditional loading, we might as well support a first-class should-concat filter for CSS.
|
|
||
| // Cases when we don't want to concat | ||
| function page_optimize_bail() { | ||
| function page_optimize_should_optimize() { |
There was a problem hiding this comment.
Since I wanted to publish a related filter for this function, I was looking for a better, non-slang word for "bail", but none satisfied me. I ended up thinking it was better to remove the negation and make this a positive "should optimize" along with a filter of the same name.
| } | ||
| } | ||
|
|
||
| add_action( 'init', 'page_optimize_css_concat_init' ); |
There was a problem hiding this comment.
Instead of having different initialization timing with plugins_loaded in the main module and init in the concat modules, let's simplify and have everything driven in a single place by a single action. This makes loading logic readable in one place and thus easier to understand.
|
|
||
| if ( $concat_js || $concat_css ) { | ||
| // Disable Jetpack photon-cdn for static JS/CSS if we're doing any concat | ||
| add_filter( 'jetpack_force_disable_site_accelerator', '__return_true' ); |
There was a problem hiding this comment.
If page-optimize isn't doing anything, we should probably let Jetpack site accelerator do its thing. 😅
|
I did some local testing, and everything is looking good so far. But I'll write out a test plan to explicitly touch on all the things changed here. |
1 participant
This PR consolidates conditional init logic with the goal of cleanly skipping optimization when in AMP mode. It also adds filters for overriding JS or CSS concat and for skipping optimization completely.
I'd rather break some of this up into steps, but for the sake of time since this is a lower priority item, I am keeping it on PR and will be explicitly testing all the changes. I think these changes are safe enough that we will not regret this compromise.