bigskysoftware / htmx

</> htmx - high power tools for HTML
https://htmx.org
Other
38.56k stars 1.31k forks source link

hx-disabled-elt causes unhandled exception when targets not found #2913

Open dwasyl opened 2 months ago

dwasyl commented 2 months ago

Hi there,

Following along with the example of settings for hx-disabled-elt, and taking advantage of the inheritance, when the elements specified in the hx-disabled-elt are not found htmx fails with Uncaught TypeError: Cannot read properties of null (reading 'htmx-internal-data').

After a fair bit of debugging, the issue comes from a setup like this:

<div class="parent-container" hx-disabled-elt="find input[type='text'], find button">
  <form hx-post="<SUBMIT URL>">
     <input type="text" name="name1">
     <button type="submit">Submit</button>
    </form>
    <a hx-get="<URL>" href="#" hx-target="#some-div">Do something</a>
</div>

In the above example, the elements in the form are disabled and work great, when when the second link it pressed, because there are no child elements of input or button for find to work on, the uncaught exception is generated.

When the elements specified in a hx-disabled-elt are not found (or not children exist) the failure should be more graceful, especially since as an inheritable property it may be used in a number of different circumstances.

This is occurring with htmx 2.0.2.

MichaelWest22 commented 2 months ago

yeah interesting bug. I've tracked it down to the querySelectorAllExt() function which for the closest find next and previous css selector lookups it does not handle miss lookups. What is happening is these do a querySelector() function that returns one element or null if not found but this is then converted to an array by wrapping in [] because the querySelectorAllExt() function can also be used for multiple item lookups and [null] as the result causes the problems.

also note that hx-disabled-elt="find input[type='text'], find button" is not working properly as this attribute does not support multiple lookups so everything after the comma is ignored I think. But it still breaks with that error with just the one find.

You can work around this issue in your case by not using find and instead finding the components in another way. for example you could add a custom class to all elements you want to disable like "hx-disable" and then use ".hx-disable" as the selector which allows it to find multiple avoiding the , issue and the exception issue.

dwasyl commented 2 months ago

yeah interesting bug. I've tracked it down to the querySelectorAllExt() function which for the closest find next and previous css selector lookups it does not handle miss lookups. What is happening is these do a querySelector() function that returns one element or null if not found but this is then converted to an array by wrapping in [] because the querySelectorAllExt() function can also be used for multiple item lookups and [null] as the result causes the problems.

Makes sense, after a lot of debugging I gathered it was something along those lines returning null but I wasn't sure where.

also note that hx-disabled-elt="find input[type='text'], find button" is not working properly as this attribute does not support multiple lookups so everything after the comma is ignored I think. But it still breaks with that error with just the one find.

Interestingly, this works just fine now. The order seems to be important (button first failed, but inputs then button is fine).

You can work around this issue in your case by not using find and instead finding the components in another way. for example you could add a custom class to all elements you want to disable like "hx-disable" and then use ".hx-disable" as the selector which allows it to find multiple avoiding the , issue and the exception issue.

This would be the solution but there are a number of forms on the same page and so it gets to be a lot to sort. What would be much easier is some type of find or all children for inputs. The find or closest is great but when multiple items requiring css selectors is more of a pain than the keywords.

MichaelWest22 commented 2 months ago

Added a couple of PR's to address your issue and also fix the problem I found in the multi find selector that came from the documentation.

dwasyl commented 2 months ago

Awesome, I took a look, seems like a straightforward fix! Oddly enough to note though, the example from the docs, that shouldn't work based on your PR works fine. My inputs and buttons are disabled when here's an inflight request.