Skip to content

Commit b8f343b

Browse files
lucadobrescuclaude
andcommitted
fix: preserve script order for lazy-rendered charts so the renderer registers before render-facade fires (#1319)
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent 4c379dc commit b8f343b

2 files changed

Lines changed: 86 additions & 5 deletions

File tree

classes/Visualizer/Module/Frontend.php

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -803,14 +803,20 @@ function visualizerTriggerScriptLoader() {
803803

804804
function visualizerLoadScripts() {
805805
document.querySelectorAll("script[data-visualizer-script]").forEach(function(elem) {
806-
jQuery.getScript( elem.getAttribute("data-visualizer-script") )
807-
.done( function( script, textStatus ) {
808-
elem.setAttribute("src", elem.getAttribute("data-visualizer-script"));
809-
elem.removeAttribute("data-visualizer-script");
806+
// Replace the placeholder with a real script tag using async=false:
807+
// scripts download in parallel but execute in insertion order, which
808+
// preserves the WordPress dependency order. Parallel jQuery.getScript
809+
// calls could execute render-facade.js before the chart renderer had
810+
// registered its render event listener, leaving charts blank.
811+
var script = document.createElement("script");
812+
script.src = elem.getAttribute("data-visualizer-script");
813+
script.async = false;
814+
script.onload = function() {
810815
setTimeout( function() {
811816
visualizerRefreshChart();
812817
} );
813-
} );
818+
};
819+
elem.parentNode.replaceChild(script, elem);
814820
});
815821
}
816822

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,75 @@
1+
/**
2+
* WordPress dependencies
3+
*/
4+
const { test, expect } = require( '@wordpress/e2e-test-utils-playwright' );
5+
6+
/**
7+
* Internal dependencies
8+
*/
9+
const { deleteAllCharts, createChartWithAdmin } = require( '../utils/common' );
10+
11+
/**
12+
* Regression tests for https://github.com/Codeinwp/visualizer/issues/1319
13+
*
14+
* Charts with lazy rendering enabled load their scripts only after the first
15+
* user interaction, via `script[data-visualizer-script]` placeholders. The
16+
* loader must preserve the WordPress dependency order: if render-facade.js
17+
* executes before the chart renderer (render-google.js etc.) has registered
18+
* its `visualizer:render:chart:start` listener, the render event is lost and
19+
* the chart stays blank forever.
20+
*/
21+
test.describe( 'Lazy rendered charts (frontend)', () => {
22+
test.beforeEach( async ( { requestUtils } ) => {
23+
await deleteAllCharts( requestUtils );
24+
} );
25+
26+
async function createChartOnPost( admin, page, requestUtils ) {
27+
// New charts have lazy rendering enabled by default.
28+
const chartId = await createChartWithAdmin( admin, page );
29+
30+
const post = await requestUtils.createPost( {
31+
title: 'Lazy render',
32+
content: `[visualizer id="${ chartId }" lazy="no" class=""]`,
33+
status: 'publish',
34+
} );
35+
36+
return { chartId, post };
37+
}
38+
39+
async function triggerLazyLoader( page ) {
40+
// Scripts are swapped to `data-visualizer-script` placeholders.
41+
await expect( page.locator( 'script[data-visualizer-script]' ).first() ).toBeAttached();
42+
43+
// The loader starts on the first user interaction.
44+
await page.evaluate( () => window.dispatchEvent( new Event( 'scroll' ) ) );
45+
}
46+
47+
test( 'chart renders when the renderer script loads after render-facade.js', async ( { admin, page, requestUtils } ) => {
48+
const { chartId, post } = await createChartOnPost( admin, page, requestUtils );
49+
50+
// Delay the chart renderer scripts so render-facade.js would win the
51+
// load race — the deterministic reproduction of issue #1319.
52+
await page.route( /js\/render-(google|chartjs|datatables)\.js/, async ( route ) => {
53+
await new Promise( ( resolve ) => setTimeout( resolve, 2000 ) );
54+
await route.continue();
55+
} );
56+
57+
await page.goto( `/?p=${ post.id }` );
58+
await triggerLazyLoader( page );
59+
60+
const chart = page.locator( `.visualizer-front-${ chartId }` ).first();
61+
await expect( chart ).toHaveClass( /visualizer-chart-loaded/, { timeout: 15000 } );
62+
await expect( chart.locator( 'svg, canvas, table' ).first() ).toBeVisible();
63+
} );
64+
65+
test( 'chart renders without artificial script delay', async ( { admin, page, requestUtils } ) => {
66+
const { chartId, post } = await createChartOnPost( admin, page, requestUtils );
67+
68+
await page.goto( `/?p=${ post.id }` );
69+
await triggerLazyLoader( page );
70+
71+
const chart = page.locator( `.visualizer-front-${ chartId }` ).first();
72+
await expect( chart ).toHaveClass( /visualizer-chart-loaded/, { timeout: 15000 } );
73+
await expect( chart.locator( 'svg, canvas, table' ).first() ).toBeVisible();
74+
} );
75+
} );

0 commit comments

Comments
 (0)