Skip to content

Running ready function on readyState === 'interactive' #2264

@andrew-ebsco

Description

@andrew-ebsco

Hello htmx team!

I have been working on a confusing bug that I was able to fix, but want to present it to the team before making a pull request. I have started using htmx in Magento for some custom features. Magento uses RequireJS to load AMD modules and htmx runs well in the set up. However, RequireJS loads all modules with async="" on all script tags. The bug I ran into is that the htmx initialization function was not running sometimes. Since the document body was not being processed, it required a page refresh to get the htmx behavior to execute. After some debugging, I found the issue was in how the ready function checks for a complete DOM.

var isReady = false
getDocument().addEventListener('DOMContentLoaded', function() {
    isReady = true
})

/**
 * Execute a function now if DOMContentLoaded has fired, otherwise listen for it.
 *
 * This function uses isReady because there is no realiable way to ask the browswer whether
 * the DOMContentLoaded event has already been fired; there's a gap between DOMContentLoaded
 * firing and readystate=complete.
 */
function ready(fn) {
    // Checking readyState here is a failsafe in case the htmx script tag entered the DOM by
    // some means other than the initial page load.
    if (isReady || getDocument().readyState === 'complete') {
        fn();
    } else {
        getDocument().addEventListener('DOMContentLoaded', fn);
    }
}

In the above code, there is a scenario where fn is never executed. Sometimes the htmx code was being executed after the DOMContentLoaded event had fired, but before document.readyState === 'complete'. After doing some research, I found that DOMContentLoaded can fire when document.readyState === 'interactive'. According to both MDN and the HTML Spec, the DOMContentLoaded event and a readyState of interactive are equivalent.

As a result, my proposal is to change the ready function to check for both a readyState of interactive as well as complete.

var isReady = false
getDocument().addEventListener('DOMContentLoaded', function() {
    isReady = true
})

/**
 * Execute a function now if DOMContentLoaded has fired, otherwise listen for it.
 *
 * This function uses isReady because there is no realiable way to ask the browswer whether
 * the DOMContentLoaded event has already been fired; there's a gap between DOMContentLoaded
 * firing and readystate=complete.
 */
function ready(fn) {
    // Checking readyState here is a failsafe in case the htmx script tag entered the DOM by
    // some means other than the initial page load.
    if (isReady || getDocument().readyState === 'interactive' || getDocument().readyState === 'complete') {
        fn();
    } else {
        getDocument().addEventListener('DOMContentLoaded', fn);
    }
}

The change allows fn to be immediately executed if DOMContentLoaded has already fired, but the readyState equals interactive. Since DOMContentLoaded and readyState === 'interactive' are functionally equivalent, this small change should fix the async bug I have encountered without introducing a regression. I have run the test suite locally and all tests passed without issue.

It looks like there was an attempt to fix loading bugs in PR 1972, but the code was more complex by watching the readystatechange event. It was eventually reverted in PR 2040 in favor of creating a new initialize function. That initialization function has not been created yet and as a result the async bug persists.

I believe my proposal is a much simpler fix, is inline with the HTML spec and maintains htmx's easy to install behavior. Of course, there are edge cases I don't know about. If this change would cause problems, I would like to hear about them. However, if the htmx team likes the fix, I can submit a pull request.

Thanks!

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions