loadScript, loadCSS doesn't wait for script loading / parsing when invoked multiple times in parallel

yugandhar02 commented 1 month ago

Expected Behaviour

When loadScript or loadCSS is called multiple times in parallel, the subsequent invocations after first call doesn't wait for script loading or parsing. It returns resolved promise

Actual Behaviour

loadScript or loadCSS should return pending promise if loading or parsing is still in progress

Steps to Reproduce

call loadScript or loadCSS multiple times in same page with same src url

Sample Code that illustrates the problem

async function renderVideo(block) {
  await Promise.all([
    loadCSS(VIDEO_JS_CSS),
    loadScript(VIDEO_JS_SCRIPT),
  ]);

  // render
}

async function decorateCard(card) {
  await renderVideo(card);
}

await Promise.all([
  decorateCard(card1),
  decorateCard(card2),
  decorateCard(card3),
]

Test page where it fails: https://issue-399-demo--franklin-assets-selector--hlxsites.hlx.page/_drafts/yug/demo

Test page with fix applied: https://issue-399-fix--franklin-assets-selector--hlxsites.hlx.page/_drafts/yug/demo

yugandhar02 commented 1 month ago

@rofe @sdmcraft The current assumption is that script will be loaded only once in block and works in most of the cases. In case developer is using some utility function which internally loads some script, he/she will have to ensure that function is not called multiple times in a row Can you please suggest if this issue is worth addressing ?

rofe commented 1 month ago

@yugandhar02 as far as I can see, you can await both functions. They resolve their promise once the element is loaded:

In any case, this code is in aem.js which is being maintained in https://github.com/adobe/aem-lib.

yugandhar02 commented 1 month ago

@rofe

you can await both functions. They resolve their promise once the element is loaded

that's right. but when the function is invoked again with same parameter while script is still getting dowloaded or parsed, it will return resolved promise. the issue can be reproduced with this test page https://issue-399-demo--franklin-assets-selector--hlxsites.hlx.page/_drafts/yug/demo

adobe / aem-boilerplate