fix for hx-swab-oob within web components

[workjonathan]

Description

I added a rootNode parameter to functions pertaining to oob swaps. This is so that oob swaps can respect shadowRoots.

Testing

I added a test in test/attributes/hx-swap-oob.js. It creates a web component and two divs with the same id, one inside the shadow root and one outside. The response targets the div and asserts that the one in the web component was swapped and the one outside was not. Note that I ignored the linter and used a template literal. There is no way to unregister a web component, so every test involving one must have a unique web component name, and building html with string concatenation is much less readable.

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

[workjonathan]

I added type annotations for the rootNode parameter.

I also realize now this probably does not correctly handle the special global and host targets mentioned in the web component example. Should I look into that for this PR?

[Telroshan]

I also realize now this probably does not correctly handle the special global and host targets mentioned in the web component example. Should I look into that for this PR?

I don't know much about web components, but since those are mentioned in the docs, I would say: please do look into that!

Also, you'll want to rebase against dev to get the latest commits that fixed the lint & CI issues (which were making your PR fail the CI even though it was not your fault at all 😄 )

[workjonathan]

I did in fact look into the global target and realized the implementation was a bit off. The correct rootNode is the contextElement argument to swap. I modified some tests in core/api.js because this value is no longer optional and as far as I can tell, will always be provided. Another option is to instead fall back to document if contextElement is undefined in swap. So that's been fixed and some relevant tests are added.

As for the host target, I don't think it's currently implemented on the dev branch and I don't want to implement it because I'm not 100% sure what it's supposed to do. I am not sure whether it's supposed to refer to the shadow root node or whether it should be used to search the parent's DOM in a shadow root within shadow root situation. But I think searching htmx.js for host should find something relevant to this and it doesn't.

[MichaelWest22]

Yeah quick testing shows host does not currently work so maybe it needs to be removed from the documentation?

Also testing of the related 'root' selector that was added with web components support and this seems not be be working or usable any more. This one seems not to be documented but it looks like it may have worked initially but then broke with JSDoc changes in #2336 because getTarget() was wrapped in asElement() and shadow roots are NOT elements! Not sure if a swap targeting the shadow root even made sense when it is not a normal element? When testing with overrides to let the shadow root be the target it just produced errors for me trying to set settling classes on the shadow root when classes not supported here.

      } else if (selector === 'root') {
        return [getRootNode(elt, !!global)]
      } else if (selector === 'host') {
        return [getRootNode(elt, !!global).host]
      } else if (selector.indexOf('global ') === 0) {
        return querySelectorAllExt(elt, selector.slice(7), true)

Adding these two lines in I think is all that would be needed to support 'host' targeting if this was a useful feature. It seemed to work well targeting host with a outerHTML swap or a beforebegin/afterend but inner swaps do not work it seems with web component hosts. Otherwise documentation should be updated instead to remove reference to host.

Note that the workaround to not having 'host' targeting working would be to just add some ID to the web component and then hx-target="global #someID"

[Telroshan]

I don't think it's currently implemented on the dev branch

If it's not implemented at all, then I'd say you should indeed not care about it for this PR which is about oob swap, just to keep things tight and ease the reviewer's reading.

broke with JSDoc changes in #2336

Woops, that was me... apologies for that!

Help is very very welcome when it comes to web components as we don't use those ourselves, if you feel like diving in and opening PRs to handle these cases/features, it'd be great! We also probably need more web component tests as my breaking-PR revealed 😅 .

Again, to ease 1cg's job that reviews all core PRs before merging them, I'd strongly suggest, if you feel like working on those, opening separate PRs as much as you can, to have a minimal amount of code to review at a given time.

[workjonathan]

I won't implement the host target for this pull request. As far as I can tell, these commits fulfill the objectives of the pull request. However, if it is preferred to revert changes to the api.js tests and fall back to document when no contextElement is available, I can do that instead.

[Telroshan]

Looking at the code, I see that contextElement is an optional param in the swapOptions, itself also being optional.
We have a fallback for the swapOptions in the swap method that sets it to an empty object if undefined, thus without any contextElement set, and as the swap method is exposed in the public API (thus unfortunately we can't affirm that it'll always be set), I'm afraid requiring a value for it now could be a breaking change.
So I would indeed go for the document fallback so that the current behavior is preserved and still works!