Skip to content

Fix ready state for htmx:load listeners bound on DOMContentLoaded - #2037

Closed
Telroshan wants to merge 1 commit into
bigskysoftware:devfrom
Telroshan:fix-readystate
Closed

Fix ready state for htmx:load listeners bound on DOMContentLoaded#2037
Telroshan wants to merge 1 commit into
bigskysoftware:devfrom
Telroshan:fix-readystate

Conversation

@Telroshan

Copy link
Copy Markdown
Collaborator

Description

So we introduced a commit recently to fix the ready state detection when loading htmx as an async script.
Seems we missed it at that time but we actually introduced a regression for that specific behaviour:

  • htmx.js is included, binds itself to DOMContentLoaded via its internal ready function
  • Client's custom script binds to DOMContentLoaded to wait for htmx to be initialized
  • DOMContentLoaded fires, htmx initializes itselfs thanks to its initial ready listener that has been called back, queues a setTimeout that will fire htmx:load
  • Client's custom script binds to htmx:load event using htmx.onLoad, which itself calls htmx.on
  • Since htmx.on internally also calls that ready function, it now goes through that new logic we introduced in the commit mentioned above. This is where lies the problem:
    • is the document state complete? Not yet. So the callback won't be called right away
    • So ready queues the callback for both DOMContentLoaded and readystatechange.
      • DOMContentLoaded has already been fired at this point, so readystatechange would catch it, but not now since the page has to reach the complete state first (when all assets are downloaded)
      • Meanwhile, the setTimeout that had been queued is executed, and htmx:load is fired on the body
      • When the document reaches the complete state, the htmx.on listener is called back, thus the client's custom script that binds a listener function to htmx:load. However, htmx:load has already been fired at this point, and this listener won't be called since it missed that initial htmx:load event.

Hope the description of the problem is clear, let me know!

As for the fix, this PR introduces again (that boolean variable is a mechanism we had prior to the commit mentioned above) a boolean variable, that is set to true the first time a ready callback is called. When a ready listener is called, we know that no matter what the state of the document is, htmx is now initialized and can bind events right away. Because this variable would be set to true in the steps described above, the client's hx.onLoad call wouldn't be bound to the document events but called right away instead. Thus, its load listener would be called when htmx fires htmx:load

Corresponding issue: #2033

Testing

First, to reproduce the issue:

  • See this JSFiddle with htmx 1.9.8, that works as expected (I just copied the code provided in the discussion linked above)
  • See this JSFiddle with htmx 1.9.9 for which the htmx load listener is never called
  • See this JSFiddle with this PR's fix in it, where it works as it used to in 1.9.8

I don't know how to write a test in the test suite for it, since it really relies on this timing between DOMContentLoaded and load events with the initial htmx:load event, and the test suite only runs on DOMContentLoaded, by the time this test runs, the complete state would very likely already been reached

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 Telroshan added the bug Something isn't working label Nov 23, 2023
@alexpetros

Copy link
Copy Markdown
Collaborator

Having written the first version of it myself, I think that this bit of code is ill-advised. It was important to support modules, because those are a Native JS feature, but trying to cover ever possible permutation of when htmx might enter the DOM is a fool's errand. Because htmx acts on every element with hx-* on it, if the user is going to load this in a non-standard way, it's much better that we provide them with a way to tell us when htmx is loaded, rather than us trying to guess where in the lifecycle they did it.

So what I propose instead is that we revert #1972, and provide an htmx.processAll() element that you can call to run the initialization whenever in the lifecycle works for you. This is a nice API, and prevents us from building an increasingly complex nest of closures to catch edge cases that these handlers weren't designed for.

@alexpetros alexpetros mentioned this pull request Nov 24, 2023
4 tasks
@alexpetros

Copy link
Copy Markdown
Collaborator

Closing this in favor of #2040, after discussion

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

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants