Skip to content
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

bugfix: fix htmx integration when innerHTML swap strategy is used #65

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

marcobeierer
Copy link

The htmx integration was broken when morph:innerHTML swap strategy was used. Namely the morphed content did not get processed by htmx.

The reason for this was, that the return type of the morph function was inconsistent. When outerHTML swap strategy gets used, idiomorph returns an array, but with innerHTML a HTMLCollection was returned, see https://github.com/bigskysoftware/idiomorph/blob/main/src/idiomorph.js#L204

htmx checks if the returned value of a swap by an extension is an array. This check fails for HTMLCollections and thus the content doesn't get processed. See https://github.com/bigskysoftware/htmx/blob/master/src/htmx.js#L1788

I decided to fix the issue by converting the HTMLCollection to an array directly in the morphNormalizedContent function to make the return type consistent (see https://github.com/marcobeierer/idiomorph/blob/main/src/idiomorph.js#L204).

However, this could be a breaking change if someone just uses innerHTML strategy and relies on the return type. An alternative would be to just fix the htmx integration and convert the return value of handleSwap at https://github.com/bigskysoftware/idiomorph/blob/main/src/idiomorph-htmx.js#L20

@MichaelWest22
Copy link

Yeah this looks like a good catch to me. found this commit which explains how the htmx handleSwap extension function should now work: bigskysoftware/htmx@0b9448e

If an extension uses internal htmx swap functions via the internalApi for example to swap in content then it will work fine and initialize hx-attributes on newly added content but if you do not use these functions then new content will not always work with htmx unless you return an array of elements/nodes to be processed. returning just true does nothing and false will fall back to default swap style. I don't think this is all well documented that I have found anyway.

turning the HTMLCollection into a Node[] does seem like a good option as it simplifies idiomorphs return types but the JSDocs lines in the source would also need to be updated as well. This is kind of an api contract change then so someone has to make a call if this is a breaking change. Just changing it in the extension code only would be 100% safe though so may be better to change the PR to just this instead and offer to change it the proper way and update the JSDocs at the same time.

Would be great to have a couple of inner and outer html tests with new htmx attributes swapped in and tested in htmx-integration.js as well.

@marcobeierer
Copy link
Author

@MichaelWest22
Thanks a lot for your detailed feedback.

I will update the JSDocs return types in this PR and create a new PR for the non-breaking implementation. Will also try to implement some tests, just couldn't get them running yesterday, but also tried just for a few minutes...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants