JavaScript DOMContentLiteSpeedLoaded event firing too soon?

[frzsombor]

I have the "Load JS Deferred" option set to "Delayed" and the last INLINE script on my page is something like this:

<script type="litespeed/javascript">
(function(){
    debugger; //  FIRST BREAK: Check if function runs
    document.addEventListener('DOMContentLiteSpeedLoaded', function () {
        debugger; // SECOND BREAK: Check if event triggered
    });
})();
</script>

I also have this (non-delayed) JS for testing in the page source:

<script data-no-optimize="1">
  document.addEventListener('DOMContentLiteSpeedLoaded', function () {
    console.log('DOMContentLiteSpeedLoaded fired');
  });
</script>

I found that by the time "FIRST BREAK" breaks into the debugger, "DOMContentLiteSpeedLoaded fired" is already logged on the console, and "SECOND BREAK" never happens. Which means the js_delay.js file here fires the "DOMContentLiteSpeedLoaded" event BEFORE the last delayed inline JS is actually finishes executing.

Now just my thoughts (correct me anywhere I'm wrong):

The litespeed_load_one() function works as expected for EXTERNAL js files, because it resolves on the script's load event - and for external scripts, "load" fires when the script is downloaded and the synchronous code finished executing.

But the function has issues with INLINE scripts. Because in that case the function resolves here as soon as the inline script is injected in the DOM, but because of how litespeed_inline2src() works, this does NOT mean that the inline script finished sync execution.

Looking at this test code, it works as expected:

console.log('before');
const script = document.createElement('script');
script.textContent = `console.log('inline script')`;
randomElem.after(script);
console.log('after');

This works well, because when using after() to add an inline script in the DOM, the sync execution only continues (with logging "after") when the inserted script finished sync execution.

However - and I think the issue lies here - litespeed_inline2src() uses a special way to create a Blob URL or data URL, therefore the script will be treated as an external script - therefore async by default. Then litespeed_load_one() injects the script tag which will load async, but after insertion the function will incorrectly resolve immediately here, because script "is_inline".

Because of this, we can't depend on listening for "DOMContentLiteSpeedLoaded" in inline scripts because if the script is one of the last ones on the page, LSCache JS might fire this event before the custom script adds the listener.

EDIT1:
What makes the situation even more difficult is that it looks like the same thing is happening even if I remove if (is_inline) resolve(); here. It looks like for DOM-inserted external scripts are always async, their execution is queued and the browser is allowed to fire "load", then schedule execution as a separate task. So basically for me it looks like here "load" only means "resource is ready & execution has been queued" and NOT a hard execution barrier.

EDIT2 (SOLUTION):
After "EDIT1" I also changed e2.src = litespeed_inline2src(e.textContent); to e2.textContent = e.textContent; here and finally this DID solve the issue! With this, now all code runs in the expected order! Event listener added correctly, before LSC fires "DOMContentLiteSpeedLoaded".

Now I only have the remaining questions:
Why litespeed_inline2src() was even introduced and used?
And what issues can this native solution cause when used instead of litespeed_inline2src()?

[timotei-litespeed]

@frzsombor thank you for the debugging time invested.
litespeed_inline2src is used to change JS code not to be run until a action is done on site(mouse mouve,click,tap)
The blob link is created to save inline JS in your browser so later can be used to load cached data.
I made some tests and seems to be working. Can you create a report from LSC and share me the ID generated(just the ID).
Maybe there are some settings that can help make the JS work

[frzsombor]

@timotei-litespeed ZQDANXLR
Please check Developer Tools on the home page.
Delayed JS will run on interaction.

[timotei-litespeed]

Can you change the event to DOMContentLoaded ?
There is the plugin DoLogin Security that will create login link. Use the field Passwordless Link from report tab. Please generate the link and send a new Report. Just share the ID of report.

[frzsombor]

New ID: IGPVUCUU (with login link)
I've added logging for "DOMContentLoaded" and "window.load".
Edit custom scripts: /wp-admin/post.php?post=266&action=edit

[timotei-litespeed]

@frzsombor OH!
DOMContentLoaded happens once. We hook there to stop running all JS.
We also change all DOMContentLoaded to DOMContentLiteSpeedLoaded, to make sure we run all user scripts after interaction happen.

Your addEventListener will run but will not execute the script added because is run AFTER DOMContentLiteSpeedLoaded is triggered.

To be sure it run you will need to run data-no-optimize="1"

[frzsombor]

We also change all DOMContentLoaded to DOMContentLiteSpeedLoaded, to make sure we run all user scripts after interaction happen.

I know, the issue is still, that js_delay.js can trigger "DOMContentLiteSpeedLoaded" before the last inline script finishes running, because of the reasons I found and described above and as you can still see on the reported test page.

Your addEventListener will run but will not execute the script added because is run AFTER DOMContentLiteSpeedLoaded is triggered.

Well, and that IS the issue :) And the reason is that how litespeed_inline2src() makes the inline scripts async. I see that there used to be a solution using .textContent at here, but for some reason it is now commented and litespeed_inline2src() is used instead. Which causes this race condition issue.

To be sure it run you will need to run data-no-optimize="1"

To use data-no-optimize just because an inline script at the end of a page has a listener for "DOMContentLoaded" (that will be replaced with "DOMContentLiteSpeedLoaded" by LSC and whose callback will fail to run because of this issue) is rather a workaround then a solution in my eyes. There can be inline scripts relying on "DOMContentLoaded" at the end of a page, that a user wants to delay.

[timotei-litespeed]

@frzsombor I see your point. We need some time to talk the changes and we will come back with an answer

[timotei-litespeed]

@frzsombor please allow multiple use for the login link. If new url is created, send a new report
Thank you

[frzsombor]

@timotei-litespeed My bad, it should work now.

[timotei-litespeed]

@frzsombor what do you mean?

[frzsombor]

@timotei-litespeed I mean the login link in IGPVUCUU

[timotei-litespeed]

@frzsombor added some changes to your site, also added the code on a DEV code(not for live code!) to this link: here
The link can be used in LSC => Toolbox => Beta Test => paste the code to input and click update.
Maybe test the code on other sites too

[timotei-litespeed]

@frzsombor friendly bump
Just curious about the result

[frzsombor]

@timotei-litespeed I dug deeper into the subject again.

First, a correction: My statement, where I wrote external scripts "execution is queued and the browser is allowed to fire 'load', then schedule execution as a separate task" was incorrect. According to the specification, this is NOT true: the load event on a script element fires only after the script has been fully executed.

And because of this, I can verify that your fix works correctly, and it forces the expected, strict "Waterfall" execution. You can also remove the is_inline variables here and here, because they are not used anymore. Also adding revokeObjectURL was a good practice to free up memory. I tested whether it negatively affects debugging with a delayed code that runs after the revoke, but everything still works without issues.

Looks good to me.

ps.: I would move changing the litespeed_ui_events to a different commit as those are unrelated.

[timotei-litespeed]

New code with variable removed: https://github.com/timotei-litespeed/lscache_wp/tree/JS_Delay-fix_blob_trigger

Thank you for the debug, test and help. I will talk with the developers about including this change in next version(at the moment is 7.8 upcoming).
I will keep you up to date.

PS: I left the events added because they are a fix for other issue we have. I know it's out of the scope of issue you have but is JS delay related.