arvgta / ajaxify

Ajaxify - The Ajax Plugin
https://4nf.org/
274 stars 124 forks source link

Appropriate handling of DOMContentLoaded event #216

Closed arvgta closed 2 years ago

arvgta commented 2 years ago

When using Ajaxify, the DOMContentLoaded event is not fired every time around i.e. only on initial load.

Lots of sites that translate e.g. the old jQuery ready() event to native JavaScript use the DOMContentLoaded event instead. In order to assist such porting, we have introduced the following piece of code...

... which is suspected to be buggy:

$.intevents = () => {
    let iFn = function (a, b, c = false) { 
        if ((this === document || this === window) && a=="DOMContentLoaded") 
            setTimeout(b); // if "DOMContentLoaded" - execute function, 
        else this.ael(a,b,c); //else - call original event listener
    };

    EventTarget.prototype.ael = EventTarget.prototype.addEventListener; // store original method
    EventTarget.prototype.addEventListener = iFn; // start intercepting event listener addition
}

At the very least the setTimeout(b) bit seems dodgy, because the b handler is being called at the time of registering the handler, which can cause timing problems, that have been reported on some sites.

The above logic is enabled by default (intevents: true)


A different and possibly cleaner approach might be to programmatically trigger DOMContentLoaded, in case the user wants that?

Something like this:

document.dispatchEvent(new Event('DOMContentLoaded'))


Now that I made the transition, the next problem is how to avoid duplicate attaching of any event handlers?

If necessary, I would like to ensure that the handlers are only triggered once by using the modern { once: true } technique:

arvgta commented 2 years ago

Just for the sake of completeness - this approach has been discarded in the course of some testing...

arvgta commented 2 years ago

Our partner AllMediaLab has reported, that there are still issues with DOMContentLoaded handling - thus reopening

arvgta commented 2 years ago

Our partner AllMediaLab has reported, that this is not an issue with DOMContentLoaded handling after all - thus closing again