Move getElementFromSelector & getSelectorFromElement to SelectorEngine (#36027)

* Move `getElementFromSelector` & getSelectorFromElement` inside selector-engine.js, in order to use SelectorEngine methods, avoiding raw querySelector usage

* add `getMultipleElementsFromSelector` helper

Co-authored-by: Julien Déramond <juderamond@gmail.com>

js/src/carousel.js

@@ -7,7 +7,6 @@
 
 import {
   defineJQueryPlugin,
-  getElementFromSelector,
   getNextActiveElement,
   isRTL,
   isVisible,
@@ -431,7 +430,7 @@ class Carousel extends BaseComponent {
  */
 
 EventHandler.on(document, EVENT_CLICK_DATA_API, SELECTOR_DATA_SLIDE, function (event) {
-  const target = getElementFromSelector(this)
+  const target = SelectorEngine.getElementFromSelector(this)
 
   if (!target || !target.classList.contains(CLASS_NAME_CAROUSEL)) {
     return

js/src/collapse.js

@@ -8,8 +8,6 @@
 import {
   defineJQueryPlugin,
   getElement,
-  getElementFromSelector,
-  getSelectorFromElement,
   reflow
 } from './util/index.js'
 import EventHandler from './dom/event-handler.js'
@@ -68,7 +66,7 @@ class Collapse extends BaseComponent {
     const toggleList = SelectorEngine.find(SELECTOR_DATA_TOGGLE)
 
     for (const elem of toggleList) {
-      const selector = getSelectorFromElement(elem)
+      const selector = SelectorEngine.getSelectorFromElement(elem)
       const filterElement = SelectorEngine.find(selector)
         .filter(foundElement => foundElement === this._element)
 
@@ -185,7 +183,7 @@ class Collapse extends BaseComponent {
     this._element.classList.remove(CLASS_NAME_COLLAPSE, CLASS_NAME_SHOW)
 
     for (const trigger of this._triggerArray) {
-      const element = getElementFromSelector(trigger)
+      const element = SelectorEngine.getElementFromSelector(trigger)
 
       if (element && !this._isShown(element)) {
         this._addAriaAndCollapsedClass([trigger], false)
@@ -229,7 +227,7 @@ class Collapse extends BaseComponent {
     const children = this._getFirstLevelChildren(SELECTOR_DATA_TOGGLE)
 
     for (const element of children) {
-      const selected = getElementFromSelector(element)
+      const selected = SelectorEngine.getElementFromSelector(element)
 
       if (selected) {
         this._addAriaAndCollapsedClass([element], this._isShown(selected))
@@ -285,10 +283,7 @@ EventHandler.on(document, EVENT_CLICK_DATA_API, SELECTOR_DATA_TOGGLE, function (
     event.preventDefault()
   }
 
-  const selector = getSelectorFromElement(this)
-  const selectorElements = SelectorEngine.find(selector)
-
-  for (const element of selectorElements) {
+  for (const element of SelectorEngine.getMultipleElementsFromSelector(this)) {
     Collapse.getOrCreateInstance(element, { toggle: false }).toggle()
   }
 })

js/src/dom/selector-engine.js

@@ -77,6 +77,53 @@ const SelectorEngine = {
     ].map(selector => `${selector}:not([tabindex^="-"])`).join(',')
 
     return this.find(focusables, element).filter(el => !isDisabled(el) && isVisible(el))
+  },
+
+  getSelector(element) {
+    let selector = element.getAttribute('data-bs-target')
+
+    if (!selector || selector === '#') {
+      let hrefAttribute = element.getAttribute('href')
+
+      // The only valid content that could double as a selector are IDs or classes,
+      // so everything starting with `#` or `.`. If a "real" URL is used as the selector,
+      // `document.querySelector` will rightfully complain it is invalid.
+      // See https://github.com/twbs/bootstrap/issues/32273
+      if (!hrefAttribute || (!hrefAttribute.includes('#') && !hrefAttribute.startsWith('.'))) {
+        return null
+      }
+
+      // Just in case some CMS puts out a full URL with the anchor appended
+      if (hrefAttribute.includes('#') && !hrefAttribute.startsWith('#')) {
+        hrefAttribute = `#${hrefAttribute.split('#')[1]}`
+      }
+
+      selector = hrefAttribute && hrefAttribute !== '#' ? hrefAttribute.trim() : null
+    }
+
+    return selector
+  },
+
+  getSelectorFromElement(element) {
+    const selector = SelectorEngine.getSelector(element)
+
+    if (selector) {
+      return SelectorEngine.findOne(selector) ? selector : null
+    }
+
+    return null
+  },
+
+  getElementFromSelector(element) {
+    const selector = SelectorEngine.getSelector(element)
+
+    return selector ? SelectorEngine.findOne(selector) : null
+  },
+
+  getMultipleElementsFromSelector(element) {
+    const selector = SelectorEngine.getSelector(element)
+
+    return selector ? SelectorEngine.find(selector) : []
   }
 }
 

js/src/modal.js

@@ -5,7 +5,7 @@
  * --------------------------------------------------------------------------
  */
 
-import { defineJQueryPlugin, getElementFromSelector, isRTL, isVisible, reflow } from './util/index.js'
+import { defineJQueryPlugin, isRTL, isVisible, reflow } from './util/index.js'
 import EventHandler from './dom/event-handler.js'
 import SelectorEngine from './dom/selector-engine.js'
 import ScrollBarHelper from './util/scrollbar.js'
@@ -336,7 +336,7 @@ class Modal extends BaseComponent {
  */
 
 EventHandler.on(document, EVENT_CLICK_DATA_API, SELECTOR_DATA_TOGGLE, function (event) {
-  const target = getElementFromSelector(this)
+  const target = SelectorEngine.getElementFromSelector(this)
 
   if (['A', 'AREA'].includes(this.tagName)) {
     event.preventDefault()

js/src/offcanvas.js

@@ -7,7 +7,6 @@
 
 import {
   defineJQueryPlugin,
-  getElementFromSelector,
   isDisabled,
   isVisible
 } from './util/index.js'
@@ -231,7 +230,7 @@ class Offcanvas extends BaseComponent {
  */
 
 EventHandler.on(document, EVENT_CLICK_DATA_API, SELECTOR_DATA_TOGGLE, function (event) {
-  const target = getElementFromSelector(this)
+  const target = SelectorEngine.getElementFromSelector(this)
 
   if (['A', 'AREA'].includes(this.tagName)) {
     event.preventDefault()

js/src/tab.js

@@ -5,7 +5,7 @@
  * --------------------------------------------------------------------------
  */
 
-import { defineJQueryPlugin, getElementFromSelector, getNextActiveElement, isDisabled } from './util/index.js'
+import { defineJQueryPlugin, getNextActiveElement, isDisabled } from './util/index.js'
 import EventHandler from './dom/event-handler.js'
 import SelectorEngine from './dom/selector-engine.js'
 import BaseComponent from './base-component.js'
@@ -106,7 +106,7 @@ class Tab extends BaseComponent {
 
     element.classList.add(CLASS_NAME_ACTIVE)
 
-    this._activate(getElementFromSelector(element)) // Search and activate/show the proper section
+    this._activate(SelectorEngine.getElementFromSelector(element)) // Search and activate/show the proper section
 
     const complete = () => {
       if (element.getAttribute('role') !== 'tab') {
@@ -133,7 +133,7 @@ class Tab extends BaseComponent {
     element.classList.remove(CLASS_NAME_ACTIVE)
     element.blur()
 
-    this._deactivate(getElementFromSelector(element)) // Search and deactivate the shown section too
+    this._deactivate(SelectorEngine.getElementFromSelector(element)) // Search and deactivate the shown section too
 
     const complete = () => {
       if (element.getAttribute('role') !== 'tab') {
@@ -203,7 +203,7 @@ class Tab extends BaseComponent {
   }
 
   _setInitialAttributesOnTargetPanel(child) {
-    const target = getElementFromSelector(child)
+    const target = SelectorEngine.getElementFromSelector(child)
 
     if (!target) {
       return

js/src/util/component-functions.js

@@ -6,7 +6,8 @@
  */
 
 import EventHandler from '../dom/event-handler.js'
-import { getElementFromSelector, isDisabled } from './index.js'
+import { isDisabled } from './index.js'
+import SelectorEngine from '../dom/selector-engine.js'
 
 const enableDismissTrigger = (component, method = 'hide') => {
   const clickEvent = `click.dismiss${component.EVENT_KEY}`
@@ -21,7 +22,7 @@ const enableDismissTrigger = (component, method = 'hide') => {
       return
     }
 
-    const target = getElementFromSelector(this) || this.closest(`.${name}`)
+    const target = SelectorEngine.getElementFromSelector(this) || this.closest(`.${name}`)
     const instance = component.getOrCreateInstance(target)
 
     // Method argument is left, for Alert and only, as it doesn't implement the 'hide' method

js/src/util/index.js

@@ -30,47 +30,6 @@ const getUID = prefix => {
   return prefix
 }
 
-const getSelector = element => {
-  let selector = element.getAttribute('data-bs-target')
-
-  if (!selector || selector === '#') {
-    let hrefAttribute = element.getAttribute('href')
-
-    // The only valid content that could double as a selector are IDs or classes,
-    // so everything starting with `#` or `.`. If a "real" URL is used as the selector,
-    // `document.querySelector` will rightfully complain it is invalid.
-    // See https://github.com/twbs/bootstrap/issues/32273
-    if (!hrefAttribute || (!hrefAttribute.includes('#') && !hrefAttribute.startsWith('.'))) {
-      return null
-    }
-
-    // Just in case some CMS puts out a full URL with the anchor appended
-    if (hrefAttribute.includes('#') && !hrefAttribute.startsWith('#')) {
-      hrefAttribute = `#${hrefAttribute.split('#')[1]}`
-    }
-
-    selector = hrefAttribute && hrefAttribute !== '#' ? hrefAttribute.trim() : null
-  }
-
-  return selector
-}
-
-const getSelectorFromElement = element => {
-  const selector = getSelector(element)
-
-  if (selector) {
-    return document.querySelector(selector) ? selector : null
-  }
-
-  return null
-}
-
-const getElementFromSelector = element => {
-  const selector = getSelector(element)
-
-  return selector ? document.querySelector(selector) : null
-}
-
 const getTransitionDurationFromElement = element => {
   if (!element) {
     return 0
@@ -316,10 +275,8 @@ export {
   executeAfterTransition,
   findShadowRoot,
   getElement,
-  getElementFromSelector,
   getjQuery,
   getNextActiveElement,
-  getSelectorFromElement,
   getTransitionDurationFromElement,
   getUID,
   isDisabled,

js/tests/unit/dom/selector-engine.spec.js

@@ -1,5 +1,5 @@
 import SelectorEngine from '../../../src/dom/selector-engine'
-import { getFixture, clearFixture } from '../../helpers/fixture'
+import { clearFixture, getFixture } from '../../helpers/fixture'
 
 describe('SelectorEngine', () => {
   let fixtureEl
@@ -232,5 +232,159 @@ describe('SelectorEngine', () => {
       expect(SelectorEngine.focusableChildren(fixtureEl)).toEqual(expectedElements)
     })
   })
-})
 
+  describe('getSelectorFromElement', () => {
+    it('should get selector from data-bs-target', () => {
+      fixtureEl.innerHTML = [
+        '<div id="test" data-bs-target=".target"></div>',
+        '<div class="target"></div>'
+      ].join('')
+
+      const testEl = fixtureEl.querySelector('#test')
+
+      expect(SelectorEngine.getSelectorFromElement(testEl)).toEqual('.target')
+    })
+
+    it('should get selector from href if no data-bs-target set', () => {
+      fixtureEl.innerHTML = [
+        '<a id="test" href=".target"></a>',
+        '<div class="target"></div>'
+      ].join('')
+
+      const testEl = fixtureEl.querySelector('#test')
+
+      expect(SelectorEngine.getSelectorFromElement(testEl)).toEqual('.target')
+    })
+
+    it('should get selector from href if data-bs-target equal to #', () => {
+      fixtureEl.innerHTML = [
+        '<a id="test" data-bs-target="#" href=".target"></a>',
+        '<div class="target"></div>'
+      ].join('')
+
+      const testEl = fixtureEl.querySelector('#test')
+
+      expect(SelectorEngine.getSelectorFromElement(testEl)).toEqual('.target')
+    })
+
+    it('should return null if a selector from a href is a url without an anchor', () => {
+      fixtureEl.innerHTML = [
+        '<a id="test" data-bs-target="#" href="foo/bar.html"></a>',
+        '<div class="target"></div>'
+      ].join('')
+
+      const testEl = fixtureEl.querySelector('#test')
+
+      expect(SelectorEngine.getSelectorFromElement(testEl)).toBeNull()
+    })
+
+    it('should return the anchor if a selector from a href is a url', () => {
+      fixtureEl.innerHTML = [
+        '<a id="test" data-bs-target="#" href="foo/bar.html#target"></a>',
+        '<div id="target"></div>'
+      ].join('')
+
+      const testEl = fixtureEl.querySelector('#test')
+
+      expect(SelectorEngine.getSelectorFromElement(testEl)).toEqual('#target')
+    })
+
+    it('should return null if selector not found', () => {
+      fixtureEl.innerHTML = '<a id="test" href=".target"></a>'
+
+      const testEl = fixtureEl.querySelector('#test')
+
+      expect(SelectorEngine.getSelectorFromElement(testEl)).toBeNull()
+    })
+
+    it('should return null if no selector', () => {
+      fixtureEl.innerHTML = '<div></div>'
+
+      const testEl = fixtureEl.querySelector('div')
+
+      expect(SelectorEngine.getSelectorFromElement(testEl)).toBeNull()
+    })
+  })
+
+  describe('getElementFromSelector', () => {
+    it('should get element from data-bs-target', () => {
+      fixtureEl.innerHTML = [
+        '<div id="test" data-bs-target=".target"></div>',
+        '<div class="target"></div>'
+      ].join('')
+
+      const testEl = fixtureEl.querySelector('#test')
+
+      expect(SelectorEngine.getElementFromSelector(testEl)).toEqual(fixtureEl.querySelector('.target'))
+    })
+
+    it('should get element from href if no data-bs-target set', () => {
+      fixtureEl.innerHTML = [
+        '<a id="test" href=".target"></a>',
+        '<div class="target"></div>'
+      ].join('')
+
+      const testEl = fixtureEl.querySelector('#test')
+
+      expect(SelectorEngine.getElementFromSelector(testEl)).toEqual(fixtureEl.querySelector('.target'))
+    })
+
+    it('should return null if element not found', () => {
+      fixtureEl.innerHTML = '<a id="test" href=".target"></a>'
+
+      const testEl = fixtureEl.querySelector('#test')
+
+      expect(SelectorEngine.getElementFromSelector(testEl)).toBeNull()
+    })
+
+    it('should return null if no selector', () => {
+      fixtureEl.innerHTML = '<div></div>'
+
+      const testEl = fixtureEl.querySelector('div')
+
+      expect(SelectorEngine.getElementFromSelector(testEl)).toBeNull()
+    })
+  })
+
+  describe('getMultipleElementsFromSelector', () => {
+    it('should get elements from data-bs-target', () => {
+      fixtureEl.innerHTML = [
+        '<div id="test" data-bs-target=".target"></div>',
+        '<div class="target"></div>',
+        '<div class="target"></div>'
+      ].join('')
+
+      const testEl = fixtureEl.querySelector('#test')
+
+      expect(SelectorEngine.getMultipleElementsFromSelector(testEl)).toEqual(Array.from(fixtureEl.querySelectorAll('.target')))
+    })
+
+    it('should get elements in array, from href if no data-bs-target set', () => {
+      fixtureEl.innerHTML = [
+        '<a id="test" href=".target"></a>',
+        '<div class="target"></div>',
+        '<div class="target"></div>'
+      ].join('')
+
+      const testEl = fixtureEl.querySelector('#test')
+
+      expect(SelectorEngine.getMultipleElementsFromSelector(testEl)).toEqual(Array.from(fixtureEl.querySelectorAll('.target')))
+    })
+
+    it('should return empty array if elements not found', () => {
+      fixtureEl.innerHTML = '<a id="test" href=".target"></a>'
+
+      const testEl = fixtureEl.querySelector('#test')
+
+      expect(SelectorEngine.getMultipleElementsFromSelector(testEl)).toHaveSize(0)
+    })
+
+    it('should return empty array if no selector', () => {
+      fixtureEl.innerHTML = '<div></div>'
+
+      const testEl = fixtureEl.querySelector('div')
+
+      expect(SelectorEngine.getMultipleElementsFromSelector(testEl)).toHaveSize(0)
+    })
+  })
+})

js/tests/unit/util/index.spec.js

@@ -22,119 +22,6 @@ describe('Util', () => {
     })
   })
 
-  describe('getSelectorFromElement', () => {
-    it('should get selector from data-bs-target', () => {
-      fixtureEl.innerHTML = [
-        '<div id="test" data-bs-target=".target"></div>',
-        '<div class="target"></div>'
-      ].join('')
-
-      const testEl = fixtureEl.querySelector('#test')
-
-      expect(Util.getSelectorFromElement(testEl)).toEqual('.target')
-    })
-
-    it('should get selector from href if no data-bs-target set', () => {
-      fixtureEl.innerHTML = [
-        '<a id="test" href=".target"></a>',
-        '<div class="target"></div>'
-      ].join('')
-
-      const testEl = fixtureEl.querySelector('#test')
-
-      expect(Util.getSelectorFromElement(testEl)).toEqual('.target')
-    })
-
-    it('should get selector from href if data-bs-target equal to #', () => {
-      fixtureEl.innerHTML = [
-        '<a id="test" data-bs-target="#" href=".target"></a>',
-        '<div class="target"></div>'
-      ].join('')
-
-      const testEl = fixtureEl.querySelector('#test')
-
-      expect(Util.getSelectorFromElement(testEl)).toEqual('.target')
-    })
-
-    it('should return null if a selector from a href is a url without an anchor', () => {
-      fixtureEl.innerHTML = [
-        '<a id="test" data-bs-target="#" href="foo/bar.html"></a>',
-        '<div class="target"></div>'
-      ].join('')
-
-      const testEl = fixtureEl.querySelector('#test')
-
-      expect(Util.getSelectorFromElement(testEl)).toBeNull()
-    })
-
-    it('should return the anchor if a selector from a href is a url', () => {
-      fixtureEl.innerHTML = [
-        '<a id="test" data-bs-target="#" href="foo/bar.html#target"></a>',
-        '<div id="target"></div>'
-      ].join('')
-
-      const testEl = fixtureEl.querySelector('#test')
-
-      expect(Util.getSelectorFromElement(testEl)).toEqual('#target')
-    })
-
-    it('should return null if selector not found', () => {
-      fixtureEl.innerHTML = '<a id="test" href=".target"></a>'
-
-      const testEl = fixtureEl.querySelector('#test')
-
-      expect(Util.getSelectorFromElement(testEl)).toBeNull()
-    })
-
-    it('should return null if no selector', () => {
-      fixtureEl.innerHTML = '<div></div>'
-
-      const testEl = fixtureEl.querySelector('div')
-
-      expect(Util.getSelectorFromElement(testEl)).toBeNull()
-    })
-  })
-
-  describe('getElementFromSelector', () => {
-    it('should get element from data-bs-target', () => {
-      fixtureEl.innerHTML = [
-        '<div id="test" data-bs-target=".target"></div>',
-        '<div class="target"></div>'
-      ].join('')
-
-      const testEl = fixtureEl.querySelector('#test')
-
-      expect(Util.getElementFromSelector(testEl)).toEqual(fixtureEl.querySelector('.target'))
-    })
-
-    it('should get element from href if no data-bs-target set', () => {
-      fixtureEl.innerHTML = [
-        '<a id="test" href=".target"></a>',
-        '<div class="target"></div>'
-      ].join('')
-
-      const testEl = fixtureEl.querySelector('#test')
-
-      expect(Util.getElementFromSelector(testEl)).toEqual(fixtureEl.querySelector('.target'))
-    })
-
-    it('should return null if element not found', () => {
-      fixtureEl.innerHTML = '<a id="test" href=".target"></a>'
-
-      const testEl = fixtureEl.querySelector('#test')
-
-      expect(Util.getElementFromSelector(testEl)).toBeNull()
-    })
-
-    it('should return null if no selector', () => {
-      fixtureEl.innerHTML = '<div></div>'
-
-      const testEl = fixtureEl.querySelector('div')
-
-      expect(Util.getElementFromSelector(testEl)).toBeNull()
-    })
-  })
-
   describe('getTransitionDurationFromElement', () => {
     it('should get transition from element', () => {
       fixtureEl.innerHTML = '<div style="transition: all 300ms ease-out;"></div>'