Skip to content

Track DOM content loaded for ready() function#1688

Merged
1cg merged 1 commit intodevfrom
track-dom-content-loaded
Aug 16, 2023
Merged

Track DOM content loaded for ready() function#1688
1cg merged 1 commit intodevfrom
track-dom-content-loaded

Conversation

@alexpetros
Copy link
Collaborator

@alexpetros alexpetros commented Aug 9, 2023

Description

Updated version of #1659 and #1681 based on conversation with @xhaggi.

Basically there's a gap between the DOMContentLoaded event firing and the document readystate updating to complete. If you look at some stackoverflow answers you can see that people think these two things should be the same, but they are not.

Fortunately, it's not hard for us to track with a closure. DOMContentLoaded will only run after all the deferred scripts have been executed, so as long as we listen for it on the initial execution of htmx, we should be safe. For good measure, I ORed it with getDocument().readystate === 'complete', in case someone is doing something insane like hotswapping the htmx script tag into the DOM based on some user action (don't do that).

Sort of an "ugh, JavaScript" moment but I'd argue it's really more of an "ugh, DOM APIs" moment.

DOMContentLoaded event: https://developer.mozilla.org/en-US/docs/Web/API/Document/DOMContentLoaded_event
Document readystate: https://developer.mozilla.org/en-US/docs/Web/API/Document/readyState

@alexpetros alexpetros added bug Something isn't working ready for review Issues that are ready to be considered for merging labels Aug 9, 2023
@alexpetros alexpetros force-pushed the track-dom-content-loaded branch from cc7e9a9 to bc75192 Compare August 10, 2023 15:38
@alexpetros alexpetros force-pushed the track-dom-content-loaded branch from bc75192 to cf9337c Compare August 10, 2023 15:48
@xhaggi
Copy link
Contributor

xhaggi commented Aug 10, 2023

Looks good 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working ready for review Issues that are ready to be considered for merging

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants