-
Notifications
You must be signed in to change notification settings - Fork 41
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
hx-*
attributes ignored when swapping in different type of element
#124
Comments
Amazingly helpful reproduction, thank you! I'll take a look at this today. |
@MichaelWest22 Would you mind taking a look at this? The htmx->idiomorph extension isn't getting triggered the second time, but I don't yet know why. I tried digging into the htmx source to find the |
Ok had a look at debugging this. Found the cause of the issue is the duct tape parent seems to be causing our issue here. edit ignore this following bit I was totatally wrong: Actually I was totally wrong above and it is mostly fine as it is not newContent but old content in the duct tape parent. But the actual issue is that the duck tape parent is getting created at the start with the original oldNode as its only children and then the morph happens and often this node is now orphaned if it gets replaced with a new real node. the duck tape parent does not update to the real new children after the morph has happened as it is not a real parent node that updates. In the example provided the morph is changing a div node into a button node which can not be morphed in place as they do not softmatch so there is actually no morphing possible in this situation and it just adds the new button node and deletes the div. situations where an id'ed div moves down levels will also cause problems. in the end the node being returned to htmx is now the orphaned removed node and htmx never gets passed back the button so it does not call init routines on it for hx parameters I think we could maybe remove the duck tape parent to resolve this. Also we could streamline the morphOuterHTML solution like this: function morphOuterHTML(ctx, oldNode, newNode) {
const oldParent = normalizeParent(oldNode);
// store start and end points so we can find inbetween nodes to return later
// as we can avoid returning before or after siblings
const beforeStartPoint = oldNode.previousSibling;
const endPoint = oldNode.nextSibling;
morphChildren(
ctx,
oldParent,
newNode,
// these two optional params are the secret sauce
oldNode, // start point for iteration
endPoint, // end point for iteration
);
const nodes = []
// return array from the first node added to before the last node
let cursor = beforeStartPoint?.nextSibling || oldParent.firstChild;
while (cursor && cursor != endPoint) {
nodes.push(cursor);
cursor = cursor.nextSibling;
}
return nodes;
} But I found this does not solve the limitations of the duck tape as there is just no way I can find for it to store updated node children list after the morph. The only way I can think to keep the duck tape would be to get morphChildren to create a running array of all its processed nodes and return these out which can then be just returned back easily. But this means we need to store and calculate this children nodes array in all morphChildren nested calls which is a perf hit. |
https://codepen.io/MichaelWest22/pen/LEYbdNz example code pen with duck tape removed seems to work fine using the code from this branch: While I think this change would be fine and do everything we need it to it does allow the mutation of the source new content in some situations as seen in one of the tests that I had to update. I don't think this is a real world concern but it is possible to use cloning to solve this unlikely edge case if required. But this did need some rework of the id-set generation code as the with the optimization to not clone and reparent the new content if it is a single node with no siblings can cause another test to fail which I added to detect if new content's parent id has a node. |
main...MichaelWest22:realParent another example of a possible way to fix it if we had to keep duck tape old parent. |
@MichaelWest22 Thank you very much for taking a look at this! Honestly, I'd really, really love to remove that ugly duck-type-parent hack, if we can do it without reparenting (losing state, maybe loss of perf?) or cloning (probably loss of perf for larger DOM trees). Or if it turns out the perf hit is negligable. Related to #103, I think we can explicitly commit to not supporting hidden state in I think there are clearly still many unknowns regarding all of that above, so right now I'm favoring your last solution in the |
@MichaelWest22 I just had a brainwave. I think we can resolve all these recent issues by changing how the duck-type-parentNode works. Rather than holding onto the node so that it can return in an array as its That said, I still really really do want to remove the duck-type-parentNode altogether. But at least for the purposes of a v0.7.3, I think this should at least resolve the issues we've been seeing. I'll open a separate PR. |
@timomihaljov-yle Idiomorph v0.7.3 should fix this. Reopen if not! |
Works great! Thanks so much! |
When using
hx-swap="morph"
to swap in a different type of element (e.g. a<div>
to replace a<form>
), thehx-*
attributes of the new element are ignored and the element becomes inert.The following examples demonstrate the problem:
The text was updated successfully, but these errors were encountered: