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

querySelectorAllExt returns unexpected null which impacts findAttributeTargets #3055

Closed
san4d opened this issue Dec 2, 2024 · 3 comments
Closed

Comments

@san4d
Copy link

san4d commented Dec 2, 2024

Summary

querySelectorAllExt can return [null] which violates the documented return type of findAttributeTargets: (Node|Window)[]. This has upstream effects on methods like disableElements, which iterate through a collection of elements returned by findAttributeTargets, expecting each to be not null.

  function findAttributeTargets(elt, attrName) {
    const attrTarget = getClosestAttributeValue(elt, attrName)
    if (attrTarget) {
      if (attrTarget === 'this') {
        return [findThisElement(elt, attrName)]
      } else {
        const result = querySelectorAllExt(elt, attrTarget) // <-- This can return [null]
        if (result.length === 0) {
          logError('The selector "' + attrTarget + '" on ' + attrName + ' returned no matches!')
          return [DUMMY_ELT]
        } else {
          return result
        }
      }
    }
  }

Context

In my application, this happens when the elt passed in to findAttributeTargets has no children and the attrTarget would yield no results. The attrName in my context is hx-disabled-elt.

Workaround

In my setup, the inherited hx-disabled-elt was causing the failed lookup. I explicitly set the hx-disabled-elt to a query selector that would return results to unblock myself.

@MichaelWest22
Copy link
Contributor

I think this was found here #2913 which i wrote a quick fix for but this fix has now been combined with an existing in progress enhancment PR #2902 which should resolve this when this is merged and released.

@san4d
Copy link
Author

san4d commented Dec 3, 2024

Thanks, that looks similar. I'll close this for now, follow 2902, and retest once that's merged.

@san4d
Copy link
Author

san4d commented Dec 3, 2024

Closing in favor of #2902.

@san4d san4d closed this as completed Dec 3, 2024
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

No branches or pull requests

2 participants