bigskysoftware / bigskysoftware/htmx

Running ready function on readyState === 'interactive'

Open
#2,264 22 comments 1 reaction 0 assignees View on GitHub
Dominant language
JavaScript
Stars
49.4k
Forks
1.7k
Avg merge
3d 22h
Merged PRs (30d)
30

Description

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](https://github.com/magento/magento2) 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.

```js
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](https://developer.mozilla.org/en-US/docs/Web/API/Document/readyState) and the [HTML Spec](https://html.spec.whatwg.org/multipage/dom.html#reporting-document-loading-status), 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.
```js
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](https://github.com/bigskysoftware/htmx/pull/1972), but the code was more complex by watching the `readystatechange` event. It was eventually reverted in [PR 2040](https://github.com/bigskysoftware/htmx/pull/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!

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.