Skip to content

Commit

Permalink
bug symfony#1411 [Live] Fixing edge case parallel rendering bug (weav…
Browse files Browse the repository at this point in the history
…erryan)

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

Discussion
----------

[Live] Fixing edge case parallel rendering bug

| Q             | A
| ------------- | ---
| Bug fix?      | yes
| New feature?  | no
| Issues        | Fix symfony#889, Fix symfony#1333
| License       | MIT

Fixed an edge-case bug where a 2nd request would start before the 1st request finished processing.

Commits
-------

b07df68 [Live] Fixing edge case parallel rendering bug
  • Loading branch information
weaverryan committed Jan 23, 2024
2 parents 7ee0373 + b07df68 commit b07f864
Show file tree
Hide file tree
Showing 3 changed files with 63 additions and 2 deletions.
3 changes: 2 additions & 1 deletion src/LiveComponent/assets/dist/live_controller.js
Original file line number Diff line number Diff line change
Expand Up @@ -1953,7 +1953,6 @@ class Component {
this.valueStore.flushDirtyPropsToPending();
this.isRequestPending = false;
this.backendRequest.promise.then(async (response) => {
this.backendRequest = null;
const backendResponse = new BackendResponse(response);
const html = await backendResponse.getBody();
for (const input of Object.values(this.pendingFiles)) {
Expand All @@ -1967,10 +1966,12 @@ class Component {
if (controls.displayError) {
this.renderError(html);
}
this.backendRequest = null;
thisPromiseResolve(backendResponse);
return response;
}
this.processRerender(html, backendResponse);
this.backendRequest = null;
thisPromiseResolve(backendResponse);
if (this.isRequestPending) {
this.isRequestPending = false;
Expand Down
3 changes: 2 additions & 1 deletion src/LiveComponent/assets/src/Component/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -390,7 +390,6 @@ export default class Component {
this.isRequestPending = false;

this.backendRequest.promise.then(async (response) => {
this.backendRequest = null;
const backendResponse = new BackendResponse(response);
const html = await backendResponse.getBody();

Expand All @@ -410,6 +409,7 @@ export default class Component {
this.renderError(html);
}

this.backendRequest = null;
thisPromiseResolve(backendResponse);

return response;
Expand All @@ -418,6 +418,7 @@ export default class Component {
this.processRerender(html, backendResponse);

// finally resolve this promise
this.backendRequest = null;
thisPromiseResolve(backendResponse);

// do we already have another request pending?
Expand Down
59 changes: 59 additions & 0 deletions src/LiveComponent/assets/test/controller/render.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -364,6 +364,65 @@ describe('LiveController rendering Tests', () => {
await waitFor(() => expect(test.element).toHaveTextContent('Title: "greetings to you"'));
});

it('waits for the rendering process of previous request to finish before starting a new one', async () => {
const test = await createTest({
title: 'greetings',
contents: '',
}, (data: any) => `
<div ${initComponent(data)}>
<input data-model='title' value='${data.title}'>
Title: "${data.title}"
<button data-action='live#$render'>Reload</button>
</div>
`);

let didSecondRenderStart = false;
let secondRenderStartedAt = 0;
test.component.on('render:started', () => {
if (didSecondRenderStart) {
return;
}
didSecondRenderStart = true;

test.component.on('loading.state:started', () => {
secondRenderStartedAt = Date.now();
});

test.expectsAjaxCall();
test.component.render();
});

let firstRenderFinishedAt = 0;
test.component.on('render:finished', () => {
// set the finish time for the first render only
if (firstRenderFinishedAt === 0) {
firstRenderFinishedAt = Date.now();
}

// the sleep guarantees that if the 2nd request was correctly
// delayed, its start time will be at least 10ms after the first
// render finished. Without this, even if the 2nd request is
// correctly delayed, the "first render finish" and "second render
// start" times could be the same, because no time has passed.
const sleep = (milliseconds: number) => {
const startTime = new Date().getTime();
while (new Date().getTime() < startTime + milliseconds);
}
sleep(10);
});

test.expectsAjaxCall();

await test.component.render();

await waitFor(() => expect(didSecondRenderStart).toBe(true));
await waitFor(() => expect(firstRenderFinishedAt).not.toBe(0));
await waitFor(() => expect(secondRenderStartedAt).not.toBe(0));
expect(secondRenderStartedAt).toBeGreaterThan(firstRenderFinishedAt);
});

it('can update svg', async () => {
const test = await createTest({ text: 'SVG' }, (data: any) => `
<div ${initComponent(data)}>
Expand Down

0 comments on commit b07f864

Please sign in to comment.