Try using AMP Optimizer if available

[swissspidy]

Summary

Relevant Technical Choices

To-do

---

Fixes #1276

[codecov]

Codecov Report

Merging #1780 into main will increase coverage by 14.45%.
The diff coverage is 14.28%.

@@             Coverage Diff             @@
##             main    #1780       +/-   ##
===========================================
+ Coverage   69.11%   83.56%   +14.45%     
===========================================
  Files         714      770       +56     
  Lines       12498    12997      +499     
===========================================
+ Hits         8638    10861     +2223     
+ Misses       3860     2136     -1724     

Flag	Coverage Δ
#karmatests	67.71% <ø> (ø)
#unittests	66.05% <ø> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
includes/Story_Renderer.php	64.89% <14.28%> (-21.48%)	⬇️
...ts/src/dashboard/app/views/shared/storyListView.js	93.93% <0.00%> (-2.94%)	⬇️
assets/src/activation-notice/index.js	0.00% <0.00%> (ø)
assets/src/date/getDateObjectWithTimezone.js	100.00% <0.00%> (ø)
bin/utils/constants.js	100.00% <0.00%> (ø)
assets/src/date/index.js	100.00% <0.00%> (ø)
...src/activation-notice/app/config/configProvider.js	100.00% <0.00%> (ø)
...ts/src/story-embed-block/block/embedPlaceholder.js	100.00% <0.00%> (ø)
assets/src/dashboard/app/api/useCategoriesApi.js	91.66% <0.00%> (ø)
assets/src/edit-story/utils/useWhyDidYouUpdate.js	0.00% <0.00%> (ø)
... and 311 more

[schlessera]

Let me know if you need any assistance with getting the Optimizer to work.

[westonruter]

The TransformedIdentifier transformer is what is is responsible for flagging a document as being transformed which would then allow style[amp-runtime], which you are including. So that seems OK. But injecting the element in this way seems not ideal.

Flagging a document as being transformed is normally bound up with a document also going through SSR. Hence the AmpRuntimeCss, ServerSideRendering, and TransformedIdentifier form a pair: https://github.com/ampproject/amp-wp/blob/8d5f29f1f22e6623789dca250901cff0a559e590/includes/class-amp-theme-support.php#L2182-L2194

The ServerSideRendering transformer adds the style[amp-runtime] element. However, why is this not the responsibility of the AmpRuntimeCss transformer?

@schlessera Should this logic be moved from ServerSideRendering to AmpRuntimeCss?

// Emit the amp-runtime marker to indicate that we're applying server side rendering in the document.
$ampRuntimeMarker = $document->createElement(Tag::STYLE);
$ampRuntimeMarker->setAttribute(Attribute::AMP_RUNTIME, '');
$document->head->insertBefore($ampRuntimeMarker, $document->head->hasChildNodes() ? $document->head->firstChild : null);

[schlessera]

The ServerSideRendering does all the checks to see if SSR can be done, and then tags the runtime to denote that it should be inlined.

The AmpRuntimeCss is responsible for inlining (or linking) the runtime CSS, but only does so if the SSR gave its okay.

That models the way it is also handled in the amp-toolbox implementation.

If we move the generation of the element into the AmpRuntimeCss transformer, we'd need another element/attribute of some sort to provide the information whether inlining should happen or not.

[westonruter]

Given that stories can't do SSR, should the AmpRuntimeCss be done?

[swissspidy]

Given that stories can't do SSR, should the AmpRuntimeCss be done?

I don't know too much about AMP SSR, so if you can give me some guidance that would be great :-)