Discontinue use of amp endpoint in favor of query var when amp theme support is present

[westonruter]

👉 This issue was reverted in #1235

It is currently very difficult to add dynamic switching between AMP and non-AMP responses in plugins because is_amp_endpoint() has to be called after the parse_query action. This means littering is_amp_endpoint() calls into the functions that run during the generation of the page, including enqueueing scripts, generating shortcodes, and rendering widgets. The use of the endpoint is also troublesome with flushing the rewrite rules (e.g. #801). Also, the /amp/ endpoint is only eligible for being used on non-hierarchical endpoints. As noted in a support topic:

The use of the ?amp query var is required for pages and other such hierarchical post types in order to prevent ambiguity from happening. Consider you have a website about guitars and you publish pages about your favorite accessories, including which amp you like the most. So you publish a page with the URL path /guitars/favorites/amp/. If the /amp/ endpoint were used for pages, then WordPress would not know whether you are trying to get the AMP version of the Favorites page or the non-AMP version of the page about your favorite amp.

Here is the logic behind when a query var is chosen instead of an endpoint slug:

https://github.com/Automattic/amp-wp/blob/4744f2def2769f497fdb13fd4f1e08620c1a35d2/includes/amp-helper-functions.php#L37-L38

So you can see that ?amp is used when pretty permalinks aren’t enabled (of course), when a hierarchical post type is used, or when there are other query vars present.

We confirmed with the Google search team that using ?amp is perfectly valid from their side of things.

This is bound up with #945, deprecating the AMP_QUERY_VAR constant and amp_query_var filter.

When all is said and done, isset( $_GET['amp'] ) is all that would be needed to check if the AMP version is being requested. Note that if amp is not available for a given URL, then a redirect to the non-AMP version of the page will still be done.

[westonruter]

Aside, this is how to prevent using the amp endpoint today:

// Force using query var instead of endpoint.
add_filter( 'amp_pre_get_permalink', function( $pre, $post_id ) {
	return add_query_arg( amp_get_slug(), '1', get_permalink( $post_id ) );
}, 10, 2 );

[mor10]

tracking for future update of WP Rig.

[westonruter]

👉 Important follow-up to the removal of the parse_query restriction for calling is_amp_endpoint(): #934 (comment)

In short, if a site is to have some URLs be native AMP but other URLs be non-native AMP, then this will require waiting until the parse_query action has triggered so that we can use conditional functions like is_singular() and also get the queried object for the URL to see whether or not AMP has been disabled by the user.

[westonruter]

For example, in WP Rig this would require a change to lazy loading, either to wait to call wprig_lazyload_images() until parse_query action or to instead check is_amp_endpoint() inside of each of the hook callbacks it registers. /cc @mor10