Move AMP initialization logic into after_setup_theme, conditioned on amp_is_enabled filter#826
Conversation
|
@ThierryA In 856bb27 I fixed a defect across the AMP plugin whereby if you added a |
ThierryA
left a comment
There was a problem hiding this comment.
@westonruter how about reverting 36881a4 since it is handle (and modified) in #825?
| load_plugin_textdomain( 'amp', false, plugin_basename( AMP__DIR__ ) . '/languages' ); | ||
|
|
||
| amp_define_query_var(); | ||
| amp_after_setup_theme(); |
There was a problem hiding this comment.
Any specific reason to call amp_after_setup_theme() here since the amp_init() is invoked in a init action which is declared in the amp_after_setup_theme()?
There was a problem hiding this comment.
Good catch. Yeah, this should have already been called so it should be removed entirely.
| amp_render_post( get_queried_object() ); | ||
| exit; | ||
| $post = get_queried_object(); | ||
| if ( $post instanceof WP_Post ) { |
There was a problem hiding this comment.
Should it be exciting even if the page can't be rendered?
There was a problem hiding this comment.
Yes, it is very exciting. 😉
In practice this function only ever gets called if the queried object is already guaranteed to be a post, so this is more for the sake of static analysis. But yeah, it could be better to not exit here.
There was a problem hiding this comment.
Haha, very exciting indeed 😂
There was a problem hiding this comment.
I think we can just leave this as-is right now. We'll need to revisit how this works when we start rendering non-posts in AMP anyway.
856bb27 to
0627442
Compare
Move AMP initialization logic to
after_setup_themeto only run ifamp_is_enabledfilter applies astrue.Amends #813.
See #709.