Skip to content

Commit b07f864

Browse files
committed
bug #1411 [Live] Fixing edge case parallel rendering bug (weaverryan)
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 #889, Fix #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
2 parents 7ee0373 + b07df68 commit b07f864

File tree

3 files changed

+63
-2
lines changed

3 files changed

+63
-2
lines changed

src/LiveComponent/assets/dist/live_controller.js

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1953,7 +1953,6 @@ class Component {
19531953
this.valueStore.flushDirtyPropsToPending();
19541954
this.isRequestPending = false;
19551955
this.backendRequest.promise.then(async (response) => {
1956-
this.backendRequest = null;
19571956
const backendResponse = new BackendResponse(response);
19581957
const html = await backendResponse.getBody();
19591958
for (const input of Object.values(this.pendingFiles)) {
@@ -1967,10 +1966,12 @@ class Component {
19671966
if (controls.displayError) {
19681967
this.renderError(html);
19691968
}
1969+
this.backendRequest = null;
19701970
thisPromiseResolve(backendResponse);
19711971
return response;
19721972
}
19731973
this.processRerender(html, backendResponse);
1974+
this.backendRequest = null;
19741975
thisPromiseResolve(backendResponse);
19751976
if (this.isRequestPending) {
19761977
this.isRequestPending = false;

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -390,7 +390,6 @@ export default class Component {
390390
this.isRequestPending = false;
391391

392392
this.backendRequest.promise.then(async (response) => {
393-
this.backendRequest = null;
394393
const backendResponse = new BackendResponse(response);
395394
const html = await backendResponse.getBody();
396395

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

412+
this.backendRequest = null;
413413
thisPromiseResolve(backendResponse);
414414

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

420420
// finally resolve this promise
421+
this.backendRequest = null;
421422
thisPromiseResolve(backendResponse);
422423

423424
// do we already have another request pending?

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

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -364,6 +364,65 @@ describe('LiveController rendering Tests', () => {
364364
await waitFor(() => expect(test.element).toHaveTextContent('Title: "greetings to you"'));
365365
});
366366

367+
it('waits for the rendering process of previous request to finish before starting a new one', async () => {
368+
const test = await createTest({
369+
title: 'greetings',
370+
contents: '',
371+
}, (data: any) => `
372+
<div ${initComponent(data)}>
373+
<input data-model='title' value='${data.title}'>
374+
375+
Title: "${data.title}"
376+
377+
<button data-action='live#$render'>Reload</button>
378+
</div>
379+
`);
380+
381+
let didSecondRenderStart = false;
382+
let secondRenderStartedAt = 0;
383+
test.component.on('render:started', () => {
384+
if (didSecondRenderStart) {
385+
return;
386+
}
387+
didSecondRenderStart = true;
388+
389+
test.component.on('loading.state:started', () => {
390+
secondRenderStartedAt = Date.now();
391+
});
392+
393+
test.expectsAjaxCall();
394+
test.component.render();
395+
});
396+
397+
let firstRenderFinishedAt = 0;
398+
test.component.on('render:finished', () => {
399+
// set the finish time for the first render only
400+
if (firstRenderFinishedAt === 0) {
401+
firstRenderFinishedAt = Date.now();
402+
}
403+
404+
// the sleep guarantees that if the 2nd request was correctly
405+
// delayed, its start time will be at least 10ms after the first
406+
// render finished. Without this, even if the 2nd request is
407+
// correctly delayed, the "first render finish" and "second render
408+
// start" times could be the same, because no time has passed.
409+
const sleep = (milliseconds: number) => {
410+
const startTime = new Date().getTime();
411+
while (new Date().getTime() < startTime + milliseconds);
412+
}
413+
sleep(10);
414+
});
415+
416+
test.expectsAjaxCall();
417+
418+
await test.component.render();
419+
420+
await waitFor(() => expect(didSecondRenderStart).toBe(true));
421+
await waitFor(() => expect(firstRenderFinishedAt).not.toBe(0));
422+
await waitFor(() => expect(secondRenderStartedAt).not.toBe(0));
423+
expect(secondRenderStartedAt).toBeGreaterThan(firstRenderFinishedAt);
424+
});
425+
367426
it('can update svg', async () => {
368427
const test = await createTest({ text: 'SVG' }, (data: any) => `
369428
<div ${initComponent(data)}>

0 commit comments

Comments
 (0)