htmx.ajax swaps body's innerHTML when the target id doesn't exist
Opened this issue · 7 comments
I have an app where tabs hide certain div-ids when not loaded. When the div with the target id is visible, htmx.ajax() POST works as expected swapping the innerHTML as directed. But when the div with the id is no longer visible, the function replaces the entire innerHTML of the body with the response. Is this what's supposed to happen?
I'm using htmx 2.0.2
This is happening because the htmx.ajax() ajax helper function is calling the the issueAjaxRequest function with the default body as the element and passing in a targetOverride set to the resolved target you passed in. If the targetOveride passed in is null because the resolved target is not found then it falls back to checking the body element for its target. And if you happen to have boosted=true on the body then it makes the body target the body causing your issue.
I think this is an edge case bug and not the intended behaviour.
to resolve it in your situation i would do:
if(htmx.find(target)) {
htmx.ajax('GET', url, target)
}
I didn't think of that. Thank you for that suggestion.
Actually looking at it the boosted=true is not required as if target is invalid it falls back to checking body target and unless you set a hx-target on the body it will always return itself as the target boost or no boost. I don't know if this is really intended and may just be what it falls back to. Can fix the issue with dev...MichaelWest22:htmx:ajax-no-target which gets it to htmx:targetError on bad requests but this could be a breaking change if someone was depending on this broken fallback behaviour?
this could be a breaking change if someone was depending on this broken fallback behaviour
It could but honestly it looks really convoluted to me, why would you want the server's reponse that you intended to swap at a very specific place (since you passed in a selector/target element), to instead replace the whole body's content?
Imho it should reflect the behavior of hx-target
, aka that if no target is found, the request doesn't happen and a htmx:targetError
is fired. And as you did in your commit, only do that if a target override was explicitly set.
I like your suggested change @MichaelWest22 , feel free to open a bugfix PR.
Just a few questions/suggestions though
- Couldn't this be greatly simplified by removing
asElement(getTarget(resolveTarget(context.source)))) || DUMMY_ELT
altogether ? I would expect htmx to precisely fallback to thesource
element'shx-target
value, I don't think this is needed, but let me know! - You also might want to replace the falsy checks by explicit checks against undefined (or null). If you pass in an element reference directly that is null (for ex
htmx.ajax('GET', '/test', { source: someElement, target: document.getElementById('whatever') }
wherewhatever
doesn't exist in the DOM), would you also consider this needs to fail like the other cases? Right now, I think your falsy checks will still let it fallback to the body. If you also think this should fail, I would suggest checking explicitly againstnull
vsundefined
(letundefined
fallback to body as the param wasn't set at all, butnull
would mean it's a getElementById/querySelector that didn't resolve to the expected element)
I like your suggested change @MichaelWest22 , feel free to open a bugfix PR. Just a few questions/suggestions though
- Couldn't this be greatly simplified by removing
asElement(getTarget(resolveTarget(context.source)))) || DUMMY_ELT
altogether ? I would expect htmx to precisely fallback to thesource
element'shx-target
value, I don't think this is needed, but let me know!
Yeah i've simplified this a bit now but it has to be a little complex sorry to handle all the situations I was testing it with.
The logic is now:
targetOverride: resolveTarget(context.target) || ((!context.target && resolveTarget(context.source)) ? null : DUMMY_ELT)
If resolveTarget(context.target) is not null|undefiend use this as the target
Else if target is not supplied AND source resolves to a target then set target to null so that the valid source can be used for target checking only
Else set target to DUMMY_ELT to trigger the target error.
- You also might want to replace the falsy checks by explicit checks against undefined (or null). If you pass in an element reference directly that is null (for ex
htmx.ajax('GET', '/test', { source: someElement, target: document.getElementById('whatever') }
wherewhatever
doesn't exist in the DOM), would you also consider this needs to fail like the other cases? Right now, I think your falsy checks will still let it fallback to the body. If you also think this should fail, I would suggest checking explicitly againstnull
vsundefined
(letundefined
fallback to body as the param wasn't set at all, butnull
would mean it's a getElementById/querySelector that didn't resolve to the expected element)
I've updated my logic to make it a little more obvious what it is checking and I think its working well. But please review my updated logic to double check. Added tests already for most of the situations you mention.
rewrote the logic into an expanded if statement so its easier to understand and debug and added inline comment
@ohmyohmyohmy fix now in v2.0.3 so please retest and close if it is now resolved