🐛 [amp-render] fix root-element stripping from amp-render with amp-bind

extensions/amp-mustache/0.1/amp-mustache.js

@@ -78,7 +78,8 @@ export class AmpMustache extends BaseTemplate {
       const container = this.element.ownerDocument.createElement('div');
       container.appendChild(content);
       return container./*OK*/ innerHTML;
-    } else if (this.element.tagName == 'SCRIPT') {
+    }
+    if (this.element.tagName == 'SCRIPT') {
       return this.element.textContent;
     }
 

extensions/amp-render/1.0/amp-render.js

@@ -26,6 +26,7 @@ import {computedStyle, setStyles} from '#core/dom/style';
 import {dev, user, userAssert} from '../../../src/log';
 import {dict} from '#core/types/object';
 import {getSourceOrigin, isAmpScriptUri} from '../../../src/url';
+import {toArray} from '#core/types/array';
 
 /** @const {string} */
 const TAG = 'amp-render';
@@ -56,7 +57,7 @@ const isAmpStateSrc = (src) => src && src.startsWith(AMP_STATE_URI_SCHEME);
  * @param {string} src
  * @return {Promise<!JsonObject>}
  */
-const getAmpStateJson = (element, src) => {
+function getAmpStateJson(element, src) {
   return Services.bindForDocOrNull(element)
     .then((bind) => {
       userAssert(bind, '"amp-state:" URLs require amp-bind to be installed.');
@@ -77,7 +78,7 @@ const getAmpStateJson = (element, src) => {
       );
       return json;
     });
-};
+}
 
 /**
  * @param {string} bindingValue
@@ -97,6 +98,37 @@ function getUpdateValue(bindingValue, isFirstMutation) {
   return false;
 }
 
+/**
+ * Returns the non-empty node count in a template (defined as a `template`
+ * or `script` element). This is required to establish if the template content has
+ * a single wrapper element and if so we need to include it while rendering the
+ * template. For more info, see https://github.com/ampproject/amphtml/issues/35401.
+ * TODO(dmanek): Observe rewrapping at a lower level in BaseTemplate.
+ * @param {!Document} doc
+ * @param {?Element} template
+ * @return {number} count of non-empty child nodes
+ */
+function getTemplateNonEmptyNodeCount(doc, template) {
+  let childNodes = [];
+  if (template.tagName === 'SCRIPT') {
+    const div = doc.createElement('div');
+    div./*OK*/ innerHTML = template./*OK*/ innerHTML;
+    childNodes = div.childNodes;
+  } else if (template.tagName == 'TEMPLATE') {
+    childNodes = template.content.childNodes;
+  }
+  return toArray(childNodes).reduce(
+    (count, node) =>
+      count +
+      Number(
+        node.nodeType === Node.TEXT_NODE
+          ? node.textContent.trim().length > 0
+          : node.nodeType !== Node.COMMENT_NODE
+      ),
+    0
+  );
+}
+
 export class AmpRender extends BaseElement {
   /** @param {!AmpElement} element */
   constructor(element) {
@@ -367,6 +399,10 @@ export class AmpRender extends BaseElement {
               if (!bind) {
                 return this.renderTemplateAsString_(data);
               }
+              const nonEmptyNodeCount = getTemplateNonEmptyNodeCount(
+                this.element.ownerDocument,
+                template
+              );
               return templates
                 .renderTemplate(dev().assertElement(template), data)
                 .then((element) => {
@@ -380,7 +416,17 @@ export class AmpRender extends BaseElement {
                         bind.signals().get('FIRST_MUTATE') === null
                       ),
                     })
-                    .then(() => dict({'__html': element./* OK */ innerHTML}));
+                    .then(() =>
+                      dict({
+                        // We should use innerHTML when the template lacks a wrapper
+                        // element, outerHTML otherwise in order to include the wrapper
+                        // element itself.
+                        '__html':
+                          nonEmptyNodeCount === 1
+                            ? element./* OK */ outerHTML
+                            : element./* OK */ innerHTML,
+                      })
+                    );
                 });
             });
           },

extensions/amp-render/1.0/test/test-amp-render.js

@@ -964,5 +964,47 @@ describes.realWin(
       await getRenderedData();
       expect(fakeMutator.forceChangeSize).to.be.calledOnce;
     });
+
+    it('should preserve the wrapper element for template tag', async () => {
+      env.sandbox.stub(BatchedJsonModule, 'batchFetchJsonFor').resolves({
+        'menu': '<li>Item 2</li><li>Item 3</li>',
+      });
+
+      // prettier-ignore
+      element = html`
+        <amp-render
+          binding="never"
+          src="https://example.com/data.json"
+          width="auto"
+          height="300"
+          layout="fixed-height">
+          <template type="amp-mustache"><ul><li>Item 1</li>{{{menu}}}</ul></template></amp-render>`;
+      doc.body.appendChild(element);
+
+      const wrapper = await waitRendered();
+      expect(wrapper.innerHTML).to.equal(
+        '<ul><li>Item 1</li><li>Item 2</li><li>Item 3</li></ul>'
+      );
+    });
+
+    it('should preserve the wrapper element for script template tag', async () => {
+      env.sandbox.stub(BatchedJsonModule, 'batchFetchJsonFor').resolves({
+        'menu': '<p>Hello</p>',
+      });
+
+      // prettier-ignore
+      element = html`
+        <amp-render
+          binding="never"
+          src="https://example.com/data.json"
+          width="auto"
+          height="300"
+          layout="fixed-height">
+          <script type="text/plain" template="amp-mustache">{{{menu}}}</script></amp-render>`;
+      doc.body.appendChild(element);
+
+      const wrapper = await waitRendered();
+      expect(wrapper.innerHTML).to.equal('<p>Hello</p>');
+    });
   }
 );