amp-wp
amp-wp copied to clipboard
Include AMP content in REST API responses for embedding (AMP in PWA)
In #1013 a script for the Shadow AMP API is registered. To facilitate AMP in PWA there needs to be an easy way to obtain AMP content to embed into a page. One easy way would just be to add an amp
property to go alongside the existing raw
and rendered
properties of the content
field.
The logic used to obtain the content.amp
value would be similar to:
https://github.com/ampproject/amp-wp/blob/e2c5b193a6350282cf9387c7890fc99c97ab0156/includes/utils/class-amp-validation-utils.php#L357-L367
The REST API endpoint is extensible for plugins to add a new property here. For example, the rest_prepare_{$this->post_type}
filter could be used. Nevertheless, the schema should also be updated to advertise this new property.
See also https://github.com/WordPress/gutenberg/issues/7668
A second endpoint for the tree shaken CSS would be great too
An array of scripts would be useful as well:
{
...
content: {
rendered: "....<iframe src="https://youtube....></iframe>",
amp: "...<amp-youtube ...>..."
},
amp: {
styles: "....",
scripts: [
{
"custom-element": "amp-youtube",
src: "https://cdn.ampproject.org/v0/amp-youtube-0.1.js"
}
]
}
}
I'd love to share another point here, just in case it matters during the implementation:
This content.amp
field is not only useful for the Shadow AMP API, but also for Headless WP frameworks used to render full AMP pages, like Next.js 8+ or Frontity's upcoming support.
I've started a PR (https://github.com/ampproject/amp-wp/pull/2827) to work on this.
Nevertheless, the schema should also be updated to advertise this new property.
I don't currently see a way to do that when adding the AMP content under content.amp
. It seems only top-level fields added via register_rest_field
are allowed to extend the schema.
Extracting https://github.com/ampproject/amp-wp/pull/2827#discussion_r304967583 from PR from @luisherranz for discussion here:
Ok, so now this is getting very interesting. We have many options here. I'll try to summarize the options and the decisions we need to make.
Options for AMP output:
-
content.amp
: full document AMP. - Maintain
content.amp
but add new fieldamp
:-
content.amp
: onlythe_content()
converted to AMP -
amp.styles
: tree-shaken CSS -
amp.scripts
: array of used scripts
-
- Put everything in
content.amp
:-
content.amp.body
: onlythe_content()
converted to AMP -
content.amp.styles
: tree-shaken CSS -
content.amp.scripts
: array of used scripts
-
- Optional using the query (see below).
Options for the opt-in query:
- add a separate endpoint and use
_links
to make it embeddable with_embed
. - add a new query, for example
_amp
.- make it boolean (include it or not).
- add option for output format, for example:
-
_amp=full
: full document. -
_amp=body
: onlythe_content
.
The only thing I have strong feelings against is to use _embed
for the opt-in. If this is expensive, I think it should be a separate param. Most headless themes are probably using _embed
on each request, so I don't see why they would need amp
unless they are actively using AMP.
How are you intending to incorporate this into Frontity?
I think both options suit different use cases:
- Do you have a PWA and want to insert the
content
using AMP instead of HTML because it has some nice performance features? Use the Shadow AMP API. - Do you want to generate full AMP pages using a decoupled solution like Frontity or Next? Insert the content and add the scripts and styles to your head.
If we were forced to do only one, I'd rather go with the separate body content, scripts, and styles because as @ataylorme said it's easier to combine them to generate the full document than the other way around.
I guess one interesting question would be: can the Shadow AMP API be used inside an AMP document?
And https://github.com/ampproject/amp-wp/pull/2827#discussion_r304972728 from @swissspidy:
Should we just use
register_rest_field
and then opt-in is done via using the_fields
param?
@swissspidy Is this done elsewhere with the _fields
param? Isn't it normally used only for reducing the number of fields, rather than adding to the number of fields included?
A couple of different use cases for the different options, also extracted from the PR comments:
1. Full AMP document
Do you have a PWA and want to insert the content using AMP instead of HTML because it has some nice performance features? Use the full AMP document and the Shadow AMP API.
2. Body content + styles + scripts
Do you want to generate AMP pages using a decoupled solution like Frontity or Next? Insert the body content in the correct place and add the scripts and styles to your head.
For what I've seen so far there's no easy way to include a full AMP document inside another full AMP document, so using the Full AMP document for the use case 2 doesn't seem possible.
I guess one interesting question would be: can the Shadow AMP API be used inside an AMP document?
The only way to do so would be to use an amp-iframe
at present, but this is not ideal. There should really be some dedicated amp-shadow-doc
element which would allow for this.
- Body content + styles + scripts
I think this is going to be the preferred format for AMP data being included in the post entity of a REST API response JSON. The reason is that AMP Shadow API prefers to be populated with streaming: https://github.com/ampproject/amphtml/blob/master/spec/amp-shadow-doc.md#fetching-and-attaching-shadow-docs
So streaming entails that the response is HTML not JSON.
In a non-shadow context, when the AMP markup is being assembled into the page server-side, then there is no streaming preference. Also, when being assembled into an existing page, this already requires that the fields be broken up into content, style, and scripts. So this makes even more sense to prefer separation in the post entity.
- Full AMP document
Nevertheless, we can also support the Shadow use case by having a separate link in the response which has a URL pointing to the standalone content rendered in a valid AMP document.
For content + styles + scripts, take a look at this: https://github.com/ampproject/amp-wp/pull/2827#issuecomment-513090982
Looks great to me so far.
I've also added the version numbers to the scripts array following the spec and removed the check of content.rendered
now that we are using $post->post_content
.
Is this done elsewhere with the
_fields
param? Isn't it normally used only for reducing the number of fields, rather than adding to the number of fields included?
Yeah that's true. Not really an option then. It would have been an easy way for adding the proper schema though.
Maybe it's ultimately overkill to try to omit the content.amp
by default, as long as the AMP processing is only being done when content
is among the _fields
being included.
It looks like extending the content
schema to include the amp
property was not possible yet so I've opened a core ticket to add a filter in WP_REST_Posts_Controller::get_item_schema()
and submitted a patch myself: https://core.trac.wordpress.org/ticket/47779
Then I've added the filter to our AMP_REST_API
class with the new, extended schema.
The new schema looks like this:
"content": {
"description": "The content for the object.",
"type": "object",
"context": ["view", "edit"],
"properties": {
"raw": {
"description": "Content for the object, as it exists in the database.",
"type": "string",
"context": ["edit"]
},
"rendered": {
"description": "HTML content for the object, transformed for display.",
"type": "string",
"context": ["view", "edit"],
"readonly": true
},
"block_version": {
"description": "Version of the content block format used by the object.",
"type": "integer",
"context": ["edit"],
"readonly": true
},
"protected": {
"description": "Whether the content is protected with a password.",
"type": "boolean",
"context": ["view", "edit", "embed"],
"readonly": true
},
"amp": {
"description": "The AMP content for the object.",
"type": "object",
"context": ["view", "edit"],
"readonly": true,
"properties": {
"markup": {
"description": "HTML content for the object, transformed to be valid AMP.",
"type": "string",
"context": ["view", "edit"],
"readonly": true
},
"styles": {
"description": "An array of tree-shaken CSS styles extracted from the content.",
"type": "array",
"items": {
"type": "string"
},
"context": ["view", "edit"],
"readonly": true
},
"scripts": {
"description": "An object of scripts, extracted from the AMP elements and templates present in the content.",
"type": "object",
"context": ["view", "edit"],
"readonly": true,
"additionalProperties": {
"type": "object",
"context": ["view", "edit"],
"readonly": true,
"properties": {
"src": {
"type": "string",
"description": "The source of the script.",
"context": ["view", "edit"],
"readonly": true
},
"runtime_version": {
"type": "string",
"description": "The runtime version of AMP used by the script.",
"context": ["view", "edit"],
"readonly": true
},
"extension_version": {
"type": "string",
"description": "The version of the script itself.",
"context": ["view", "edit"],
"readonly": true
},
"async": {
"type": "boolean",
"description": "Whether or not the script should be loaded asynchronously.",
"context": ["view", "edit"],
"readonly": true
},
"extension_type": {
"type": "string",
"enum": ["custom-template", "custom-element"],
"description": "Type of the script, either a template or an element.",
"context": ["view", "edit"],
"readonly": true
}
}
}
}
}
}
}
}
Disable AMP content in REST API if website experience is not enabled
@westonruter I'd love to know more about this.
The website
experience needs to be enabled to be able to use the sanitizers or is this something you think will lead to a better user experience?
@luisherranz Yes. When \AMP_Options_Manager::is_website_experience_enabled()
is false
, that means the site is only using AMP Stories but not AMP for the rest of the website.
Such a check can be added to rest_api_init()
, i.e. short-circuit when the website experience is disabled, unless the post type equals \AMP_Story_Post_Type::POST_TYPE_SLUG
.
What happens if someone doesn't want to use AMP in their website (PHP theme) but for some reason wants to use it in another place where the content is fetched via the REST API.
I mean, why adding an extra limitation when we already are making the content.amp
opt-in only though a query param?
Not that I disagree, but I'm trying to understand :)
What happens if someone doesn't want to use AMP in their website (PHP theme) but for some reason wants to use it in another place where the content is fetched via the REST API.
If they want to do that they could uncheck all of the templates:
I mean, why adding an extra limitation when we already are making the
content.amp
opt-in only though a query param?
What if we discontinued that? Per https://github.com/ampproject/amp-wp/issues/1014#issuecomment-514683394
My concern is discoverability of the AMP fields on the REST API responses. If you have to supply a secret query parameter, then this would seem to be a violation of how the REST API was designed (e.g. HAL).
Ok, I understand. The _amp
query is probably not a good idea then.
Still, having to enable the website experience
to get a field in the REST API doesn't look like the most intuitive approach to me.
What about adding a setting for the REST API? Only when that setting is checked we insert the field and extend the schema.
We can even encourage people to use a cache plugin for the REST API, like you do with object cache. Either in the description or in a warning notice.
Something like this:

Usually the website experience is checked, however. If you uncheck the website experience then that entails you don't want your (non-story) content to be available in AMP, so I think it makes sense for that to control whether or not the REST API fields are included.
Also, I think my concern about AMP content being expensive to generate is probably overblown. Perhaps just include but default but then allow omitting via some AMP-specific PHP filter.
It's obviously your take guys, but I still think that if you want REST API support without affecting your site, having to enable website
and disable all the templates is far less intuitive than having a REST API checkbox.
Anyway, let me know your decision and I'll continue the development :)
Apart from that, @westonruter how is your idea for this? Is it a new field?
Add link for where the rendered AMP content can be fetched as an entire valid AMP document (for Shadow AMP document purposes).
It's obviously your take guys, but I still think that if you want REST API support without affecting your site, having to enable
website
and disable all the templates is far less intuitive than having a REST API checkbox.
It's not entirely intuitive, but I think it's intuitive enough, given that this should be a very rare scenario. Let's go ahead and gate the inclusion of content.amp
based on \AMP_Options_Manager::is_website_experience_enabled()
being true
. As part of this, let's remove this note:
https://github.com/ampproject/amp-wp/blob/1b8afd930a6c3512846ecae0ef767e78275d7a57/includes/class-amp-theme-support.php#L831
Add link for where the rendered AMP content can be fetched as an entire valid AMP document (for Shadow AMP document purposes).
This was the route I was going for a standalone plugin I was working on before, so I'll work on extracting that code to incorporate into your PR.
@kadamwhite re: https://github.com/ampproject/amp-wp/pull/2827#issuecomment-517396233
Would love your thoughts on the best way for a client to opt-in to include AMP content in the REST API response. See notes above about a special _amp
query param, the schema, and HAL.
@westonruter Sorry, missed the ping. I would think the most conceptually accurate way to handle this would be to introduce a new context=amp
. It's possible that we would still need to introduce the ability to filter the schema, but I think that Context is the right method to use to signal that we will be exposing and returning alternative data structures, as this is already the mechanism we use to signal the presence or lack of the raw
content (for example).
I would continue, that if context=amp
, rather than returning a (mostly duplicative) content.rendered
alongside the new content.amp
, I would consider it more appropriate to return the AMP content as content.rendered
.
Additional AMP properties like CSS would still need to be added on whenever they don't belong as part of the content proper, but unless there's a situation where both content.rendered
and content.amp
would be required at once, it feels like overkill to include both.