bug #588 [Live] Show a clear error multiple roots + resolve Component promise after render (weaverryan)

This PR was squashed before being merged into the 2.x branch.

Discussion
----------

[Live] Show a clear error multiple roots + resolve Component promise after render

| Q             | A
| ------------- | ---
| Bug fix?      | yes
| New feature?  | no
| Tickets       | Fix #501
| License       | MIT

Hi!

A) Ignore "comment" or "text" nodes if they are at the "root" level
B) Throw a clear error if the component has multiple elements (no re-render)
C) Resolve the Component promise after re-rendering. This is how this was intended, it allows you to, for example, `await component.render()` and your code will continue only after the Ajax call and re-render have finished.

Commits
-------

6faa252 [Live] Show a clear error multiple roots + resolve Component promise after render

Author: weaverryan

src/LiveComponent/assets/src/Component/index.ts

@@ -300,7 +300,6 @@ export default class Component {
 
         this.backendRequest.promise.then(async (response) => {
             const backendResponse = new BackendResponse(response);
-            thisPromiseResolve(backendResponse);
             const html = await backendResponse.getBody();
 
             // if the response does not contain a component, render as an error
@@ -313,13 +312,17 @@ export default class Component {
                     this.renderError(html);
                 }
 
+                thisPromiseResolve(backendResponse);
+
                 return response;
             }
 
             this.processRerender(html, backendResponse);
-
             this.backendRequest = null;
 
+            // finally resolve this promise
+            thisPromiseResolve(backendResponse);
+
             // do we already have another request pending?
             if (this.isRequestPending) {
                 this.isRequestPending = false;
@@ -364,7 +367,14 @@ export default class Component {
             modifiedModelValues[modelName] = this.valueStore.get(modelName);
         });
 
-        const newElement = htmlToElement(html);
+        let newElement;
+        try {
+            newElement = htmlToElement(html);
+        } catch (error) {
+            console.error('There was a problem with the component HTML returned:');
+
+            throw error;
+        }
         // normalize new element into non-loading state before diff
         this.hooks.triggerHook('loading.state:finished', newElement);
 

src/LiveComponent/assets/src/dom_utils.ts

@@ -220,14 +220,20 @@ export function htmlToElement(html: string): HTMLElement {
     html = html.trim();
     template.innerHTML = html;
 
-    const child = template.content.firstChild;
+    if (template.content.childElementCount > 1) {
+        throw new Error(
+            `Component HTML contains ${template.content.childElementCount} elements, but only 1 root element is allowed.`
+        );
+    }
+
+    const child = template.content.firstElementChild;
     if (!child) {
         throw new Error('Child not found');
     }
 
     // enforcing this for type simplicity: in practice, this is only use for HTMLElements
     if (!(child instanceof HTMLElement)) {
-        throw new Error(`Created element is not an Element from HTML: ${html.trim()}`);
+        throw new Error(`Created element is not an HTMLElement: ${html.trim()}`);
     }
 
     return child;

src/LiveComponent/assets/test/controller/render.test.ts

@@ -365,9 +365,8 @@ describe('LiveController rendering Tests', () => {
 
         await waitFor(() => expect(test.element).toHaveTextContent('123'));
     });
-
     it('can update html containing svg', async () => {
-        const test = await createTest({ text: 'Hello' }, (data: any) => `
+        const test = await createTest({text: 'Hello'}, (data: any) => `
             <div ${initComponent(data)}>
                 ${data.text}
                 <svg xmlns="http://www.w3.org/2000/svg" width="300" height="200">
@@ -389,4 +388,24 @@ describe('LiveController rendering Tests', () => {
 
         await waitFor(() => expect(test.element).toHaveTextContent('123'));
     });
+
+    it('can understand comment in the response', async () => {
+        const test = await createTest({ season: 'summer' }, (data: any) => `
+            <!-- messy comment -->
+            <div ${initComponent(data)}>
+                The season is: ${data.season}
+            </div>
+        `);
+
+        test.expectsAjaxCall('get')
+            .expectSentData(test.initialData)
+            .serverWillChangeData((data) => {
+                data.season = 'autumn';
+            })
+            .init();
+
+        await test.component.render();
+        // verify the component *did* render ok
+        expect(test.element).toHaveTextContent('The season is: autumn');
+    });
 });