Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Dec 5, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 13 additions & 3 deletions src/LiveComponent/assets/src/Component/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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;
Expand Down Expand Up @@ -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);

Expand Down
10 changes: 8 additions & 2 deletions src/LiveComponent/assets/src/dom_utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
23 changes: 21 additions & 2 deletions src/LiveComponent/assets/test/controller/render.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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">
Expand All @@ -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');
});
});