Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enable generation of Optimized (aka Transformed) AMP pages #958

Closed
westonruter opened this issue Feb 14, 2018 · 9 comments · Fixed by #4019
Closed

Enable generation of Optimized (aka Transformed) AMP pages #958

westonruter opened this issue Feb 14, 2018 · 9 comments · Fixed by #4019
Labels
Enhancement New feature or improvement of an existing one P0 High priority QA passed Has passed QA and is done
Milestone

Comments

@westonruter
Copy link
Member

westonruter commented Feb 14, 2018

We can speed up the serving of AMP pages by pre-rendering them and serving them in the same way they are served from an AMP cache. In short, the processes performed by the AMP Optimizer should be ported into the plugin. Something important to note here that when pre-rendering is performed, there will somewhat counter-intuitively need to be a paired mode even when a site is AMP Canonical. The reason is:

Warning: optimized AMPs will no longer be valid AMPHTML. Don't use this to optimize AMPs meant for being used by platforms such as Google Search.

So the pre-rendered AMP version of a page would would be canonical URL, and there would be an amphtml link to the same exact content without the pre-rendering logic applied. This is similar to the existing paired mode works, except here the canonical version is generated from the AMP version via pre-rendering. So it's important to note that this pre-rendering should not be offered on sites that are not using Canonical AMP, as it would not make sense to have a pre-rendered AMP version alongside the valid AMP and the non-AMP content.

As for where the pre-rendering should be performed, it should be done right after the call to AMP_Content_Sanitizer::sanitize_document() in AMP_Theme_Support::prepare_response():

https://github.com/Automattic/amp-wp/blob/1ab37c654d8c36cb1e17f1c997a6a2f00002273c/includes/class-amp-theme-support.php#L813-L815

For example:

$assets = AMP_Content_Sanitizer::sanitize_document( $dom, self::$sanitizer_classes, $args );
self::ensure_required_markup( $dom );
if ( ! is_amp_endpoint() ) {
    do_the_function_that_runs_amp_prerendering( $dom ); // 👈
}

Since is_amp_endpoint() currently always returns true if amp_is_canonical() then this will need to be amended to support the case for pre-rendering. In other words, even on a canonical-site there will be the need for separate AMP endpoint URLs. So the if ( amp_is_canonical() ) { return true; } conditional may need to be removed here. Related to #934.

👉 For more on server-side rendering, see Faster AMP on the origin: AMP + SSR = ⚡.

👉 For implementation notes, refer to AMP SSR Toolkit Implementation Guide.

@amedina amedina changed the title Add AMP pre-rendering to speed up Canonical AMP sites Enable generation of Optimized AMP pages Aug 21, 2018
@westonruter westonruter added this to the v1.1 milestone Dec 5, 2018
@westonruter westonruter changed the title Enable generation of Optimized AMP pages Enable generation of Transformed AMP pages Feb 20, 2019
@westonruter
Copy link
Member Author

In AMP the SSR is also termed “transformed”. The validator has just landed with some recognition of this which we're not explicitly skipping from the generated spec: 51d81ab.

We'll potentially need to revert that and add some awareness of a transformed mode. Or potentially we'd still not care about the transformed rules since we'd apply them in a sanitizer after the whitelist runs.

@westonruter westonruter removed this from the v1.1 milestone Mar 6, 2019
@westonruter
Copy link
Member Author

See also https://twitter.com/westonruter/status/1120061001443893248

We need to make sure the Transformed AMP sanitizer, along with the other sanitizers, are turned into a separate package which can be used standalone.

@westonruter
Copy link
Member Author

westonruter commented Sep 16, 2019

Depends on layout=intrinsic being server-side renderable. See ampproject/amphtml#17686, ampproject/amphtml#24119, ampproject/amp-toolbox#443, ampproject/amp-toolbox#264.

@westonruter
Copy link
Member Author

An important blocker for SSR (transformed intrinsic layout support) has been lifted: ampproject/amphtml#24119. For pending reference implementation of SSR intrinsic layout, see ampproject/amp-toolbox#443.

@csossi
Copy link

csossi commented Mar 10, 2020

Any QA instructions to be added here, @schlessera ?

@schlessera
Copy link
Collaborator

schlessera commented Mar 10, 2020

Testing instructions:

  1. Make sure the Optimizer is enabled (this is the case on the test site)
  2. Go to the AMP-version of a page
  3. Look at the generated markup of the page via View Source...
  4. Verify: (Optimizer active) The html tag has the attribute transformed="self;v=1" (at the very beginning of the document)
  5. Verify: (Serverside rendering active) There's a <style amp-runtime="" i-amphtml-version="XXX"> tag that contains the entire AMP runtime stylesheet
  6. Verify: Generated page is still valid AMP (aside for dev mode when logged in)

/cc @csossi

@westonruter
Copy link
Member Author

Also verify that the result is Valid AMP (aside for dev mode when logged in).

@schlessera
Copy link
Collaborator

Good point, updated the instructions.

@csossi
Copy link

csossi commented Mar 16, 2020

Verified in qa:
image

@csossi csossi added the QA passed Has passed QA and is done label Mar 16, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement New feature or improvement of an existing one P0 High priority QA passed Has passed QA and is done
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants