ampproject / amp-toolbox

A collection of AMP tools making it easier to publish and host AMP pages.
Apache License 2.0
449 stars 243 forks source link

Add SSR support for fluid layout #1232

Closed westonruter closed 3 years ago

westonruter commented 3 years ago

The use of the fluid layout currently blocks server-side rendering since it is not among the SUPPORTED_LAYOUTS:

https://github.com/ampproject/amp-toolbox/blob/3ec7bd40f34adbc8be66f951edfa6d91d5794c01/packages/optimizer/lib/transformers/ApplyLayout.js#L30-L40

The fluid layout is not ideal because it inherently involves a layout shift since its dimensions get determined at runtime, and it will not get rendered when it is initially in the viewport (per docs). Nevertheless, when this layout is required it would be preferable for it to not prevent other parts of the page from benefiting from SSR.

Implementation provided by @jridgewell.

Merging this PR is blocked by the AMP validator being updated, specifically to modify validateSsrLayout() as follows:

      const validInternalClasses = Object.create(null);
      validInternalClasses[getLayoutClass(layout)] = 0;
      if (isLayoutSizeDefined(layout)) {
        // i-amphtml-layout-size-defined
        validInternalClasses[getLayoutSizeDefinedClass()] = 0;
      }
+     if ('fluid' === layout) {
+       validInternalClasses['i-amphtml-layout-awaiting-size'] = 0;
+     }

cf. b/184254585