Checking readyState is interactive before listening for DOMContentLoaded

[andrew-ebsco]

Description

This is to fix issue #2264. Sometimes DOMContentLoaded can fire before readyState === 'complete'.

Corresponding issue:

Testing

The unit test suite passed.

Checklist

• I have read the contribution guidelines
• I have targeted this PR against the correct branch (master for website changes, dev for
  source changes)
• This is either a bugfix, a documentation update, or a new feature that has been explicitly
  approved via an issue
• I ran the test suite locally (npm run test) and verified that it succeeded

[Telroshan]

Shouldn't we simply check readyState !== 'loading' in this case?

[alexpetros]

We've tried that—I know this looks a little clunky but I'd rather both checks be included.

[Telroshan]

We've tried that

Sorry but this confuses me @alexpetros , are you suggesting this might result in a different behavior than !== 'loading'?
As it's a 3 values property, this should be the exact same behavior, except that this adds more hardcoded strings not to be minified

[alexpetros]

Ah, I didn't read your suggestion closely enough (I missed the !==, that's my bad). Yes @andrew-ebsco can you make that change?

[Telroshan]

@alexpetros just to emphasize on this, this would take us back to august 2023, aka the first time we started dealing with these ready state / load event issues.
See #1672 which was the first one about this matter ; we precisely had a !== 'loading' check, exactly what we would go back to with this PR.
See my comment in #2264 with the history of PRs and issues related to this topic.

I'm not sure that's really what we should go for here, as we're getting back to a previous state that we already know is going to cause issues, as those very issues have been reported in the past (again, cf the issues and PRs listed in my comment, linked above)

[alexpetros]

@alexpetros just to emphasize on this, this would take us back to august 2023, aka the first time we started dealing with these ready state / load event issues.

I don't think is true, because what we have now that we didn't august 2023 is a variable that captures whether DOMContentLoaded fired.

[Telroshan]

Hmm I'm not sure this variable is useful anymore if we roll back to a !== 'loading' check @alexpetros

When you added that ready variable in #1688, it was precisely because we were only checking === 'complete', thus we could miss the timeframe between interactive and complete state, where we would bind to DOMContentLoaded while the document was already interactive, thus the listener would never be called.

If we roll back to a !== 'loading' check, I don't think there's any gap between the capture you linked above, and the first ready call of the lib.
Between this

htmx/src/htmx.js

Lines 4887 to 4890 in 51807d0

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

And this

htmx/src/htmx.js

Lines 4938 to 4939 in 51807d0

	// initialize the document
	ready(function() {

No async work is performed, so the DOMContentLoaded listeners would be bound one after the other, in the same frame.

So either the DOMContentLoaded event hasn't been fired yet, and both the capture and ready will bind a listener to it, making the capture useless here
Or, the event has already fired when htmx loads, so ready executes right away (as the state is interactive and no more loading), making the capture also useless here.

As per my comment in #2264 I really think we should take some time to dive in this topic again @alexpetros with @1cg as well, to see if A) we still agree with our past selves about the init function, and B) try to setup reproducible cases to test the different loading scenarios