Move get element functionality to a helper (#33327)

Looking around on js components I found out many checks, different expressed but with same purpose.
Some of them are trying to parse string to element, others, jQuery element to js simple nodeElement etc

With this Pr, I am trying to give a standard way to parse an element

So this pr:

* Creates `getElement` helper that tries to parse an argument to element or null
* Changes `isElement` to make  explicit checks and return Boolean 
* fixes tests deficiencies

8 files changed, 85 insertions, 49 deletions

diff --git a/js/src/base-component.js b/js/src/base-component.js
index eacc8420bc..05b0adcd26 100644
--- a/js/src/base-component.js
+++ b/js/src/base-component.js
@@ -9,6 +9,7 @@ import Data from './dom/data'
 import {
   emulateTransitionEnd,
   execute,
+  getElement,
   getTransitionDurationFromElement
 } from './util/index'
 import EventHandler from './dom/event-handler'
@@ -23,7 +24,7 @@ const VERSION = '5.0.0'
 
 class BaseComponent {
   constructor(element) {
-    element = typeof element === 'string' ? document.querySelector(element) : element
+    element = getElement(element)
 
     if (!element) {
       return
diff --git a/js/src/collapse.js b/js/src/collapse.js
index 1c8f53ebd6..3241a71327 100644
--- a/js/src/collapse.js
+++ b/js/src/collapse.js
@@ -7,9 +7,9 @@
 
 import {
   defineJQueryPlugin,
+  getElement,
   getSelectorFromElement,
   getElementFromSelector,
-  isElement,
   reflow,
   typeCheckConfig
 } from './util/index'
@@ -272,14 +272,7 @@ class Collapse extends BaseComponent {
   _getParent() {
     let { parent } = this._config
 
-    if (isElement(parent)) {
-      // it's a jQuery object
-      if (typeof parent.jquery !== 'undefined' || typeof parent[0] !== 'undefined') {
-        parent = parent[0]
-      }
-    } else {
-      parent = SelectorEngine.findOne(parent)
-    }
+    parent = getElement(parent)
 
     const selector = `${SELECTOR_DATA_TOGGLE}[data-bs-parent="${parent}"]`
 
diff --git a/js/src/dropdown.js b/js/src/dropdown.js
index 8f501d8113..3eb5891f8f 100644
--- a/js/src/dropdown.js
+++ b/js/src/dropdown.js
@@ -9,6 +9,7 @@ import * as Popper from '@popperjs/core'
 
 import {
   defineJQueryPlugin,
+  getElement,
   getElementFromSelector,
   isDisabled,
   isElement,
@@ -166,12 +167,7 @@ class Dropdown extends BaseComponent {
       if (this._config.reference === 'parent') {
         referenceElement = parent
       } else if (isElement(this._config.reference)) {
-        referenceElement = this._config.reference
-
-        // Check if it's jQuery element
-        if (typeof this._config.reference.jquery !== 'undefined') {
-          referenceElement = this._config.reference[0]
-        }
+        referenceElement = getElement(this._config.reference)
       } else if (typeof this._config.reference === 'object') {
         referenceElement = this._config.reference
       }
diff --git a/js/src/tooltip.js b/js/src/tooltip.js
index e440573829..fb46578393 100644
--- a/js/src/tooltip.js
+++ b/js/src/tooltip.js
@@ -10,6 +10,7 @@ import * as Popper from '@popperjs/core'
 import {
   defineJQueryPlugin,
   findShadowRoot,
+  getElement,
   getUID,
   isElement,
   isRTL,
@@ -256,7 +257,7 @@ class Tooltip extends BaseComponent {
     const attachment = this._getAttachment(placement)
     this._addAttachmentClass(attachment)
 
-    const container = this._getContainer()
+    const { container } = this._config
     Data.set(tip, this.constructor.DATA_KEY, this)
 
     if (!this._element.ownerDocument.documentElement.contains(this.tip)) {
@@ -385,10 +386,8 @@ class Tooltip extends BaseComponent {
       return
     }
 
-    if (typeof content === 'object' && isElement(content)) {
-      if (content.jquery) {
-        content = content[0]
-      }
+    if (isElement(content)) {
+      content = getElement(content)
 
       // content is a DOM node or a jQuery
       if (this._config.html) {
@@ -518,18 +517,6 @@ class Tooltip extends BaseComponent {
     this.getTipElement().classList.add(`${CLASS_PREFIX}-${this.updateAttachment(attachment)}`)
   }
 
-  _getContainer() {
-    if (this._config.container === false) {
-      return document.body
-    }
-
-    if (isElement(this._config.container)) {
-      return this._config.container
-    }
-
-    return SelectorEngine.findOne(this._config.container)
-  }
-
   _getAttachment(placement) {
     return AttachmentMap[placement.toUpperCase()]
   }
@@ -664,16 +651,14 @@ class Tooltip extends BaseComponent {
       }
     })
 
-    if (config && typeof config.container === 'object' && config.container.jquery) {
-      config.container = config.container[0]
-    }
-
     config = {
       ...this.constructor.Default,
       ...dataAttributes,
       ...(typeof config === 'object' && config ? config : {})
     }
 
+    config.container = config.container === false ? document.body : getElement(config.container)
+
     if (typeof config.delay === 'number') {
       config.delay = {
         show: config.delay,
diff --git a/js/src/util/index.js b/js/src/util/index.js
index 0dd6b1d454..5ee211c73b 100644
--- a/js/src/util/index.js
+++ b/js/src/util/index.js
@@ -1,3 +1,5 @@
+import SelectorEngine from '../dom/selector-engine'
+
 /**
  * --------------------------------------------------------------------------
  * Bootstrap (v5.0.0): util/index.js
@@ -100,7 +102,29 @@ const triggerTransitionEnd = element => {
   element.dispatchEvent(new Event(TRANSITION_END))
 }
 
-const isElement = obj => (obj[0] || obj).nodeType
+const isElement = obj => {
+  if (!obj || typeof obj !== 'object') {
+    return false
+  }
+
+  if (typeof obj.jquery !== 'undefined') {
+    obj = obj[0]
+  }
+
+  return typeof obj.nodeType !== 'undefined'
+}
+
+const getElement = obj => {
+  if (isElement(obj)) { // it's a jQuery object or a node element
+    return obj.jquery ? obj[0] : obj
+  }
+
+  if (typeof obj === 'string' && obj.length > 0) {
+    return SelectorEngine.findOne(obj)
+  }
+
+  return null
+}
 
 const emulateTransitionEnd = (element, duration) => {
   let called = false
@@ -238,6 +262,7 @@ const execute = callback => {
 }
 
 export {
+  getElement,
   getUID,
   getSelectorFromElement,
   getElementFromSelector,
diff --git a/js/tests/unit/collapse.spec.js b/js/tests/unit/collapse.spec.js
index 590b5795c7..0997ae7c20 100644
--- a/js/tests/unit/collapse.spec.js
+++ b/js/tests/unit/collapse.spec.js
@@ -58,7 +58,8 @@ describe('Collapse', () => {
       const collapseEl = fixtureEl.querySelector('div.collapse')
       const myCollapseEl = fixtureEl.querySelector('.my-collapse')
       const fakejQueryObject = {
-        0: myCollapseEl
+        0: myCollapseEl,
+        jquery: 'foo'
       }
       const collapse = new Collapse(collapseEl, {
         parent: fakejQueryObject
diff --git a/js/tests/unit/dropdown.spec.js b/js/tests/unit/dropdown.spec.js
index 7121857841..5275f1a556 100644
--- a/js/tests/unit/dropdown.spec.js
+++ b/js/tests/unit/dropdown.spec.js
@@ -3,7 +3,7 @@ import EventHandler from '../../src/dom/event-handler'
 import { noop } from '../../src/util'
 
 /** Test helpers */
-import { getFixture, clearFixture, createEvent, jQueryMock } from '../helpers/fixture'
+import { clearFixture, createEvent, getFixture, jQueryMock } from '../helpers/fixture'
 
 describe('Dropdown', () => {
   let fixtureEl
@@ -467,6 +467,7 @@ describe('Dropdown', () => {
 
       const btnDropdown = fixtureEl.querySelector('[data-bs-toggle="dropdown"]')
       const virtualElement = {
+        nodeType: 1,
         getBoundingClientRect() {
           return {
             width: 0,
diff --git a/js/tests/unit/util/index.spec.js b/js/tests/unit/util/index.spec.js
index a7c1c28982..b4010e0e0e 100644
--- a/js/tests/unit/util/index.spec.js
+++ b/js/tests/unit/util/index.spec.js
@@ -1,7 +1,7 @@
 import * as Util from '../../../src/util/index'
 
 /** Test helpers */
-import { getFixture, clearFixture } from '../../helpers/fixture'
+import { clearFixture, getFixture } from '../../helpers/fixture'
 
 describe('Util', () => {
   let fixtureEl
@@ -171,24 +171,58 @@ describe('Util', () => {
   })
 
   describe('isElement', () => {
-    it('should detect if the parameter is an element or not', () => {
-      fixtureEl.innerHTML = '<div></div>'
+    it('should detect if the parameter is an element or not and return Boolean', () => {
+      fixtureEl.innerHTML =
+        [
+          '<div id="foo" class="test"></div>',
+          '<div id="bar" class="test"></div>'
+        ].join('')
 
-      const el = document.querySelector('div')
+      const el = fixtureEl.querySelector('#foo')
 
-      expect(Util.isElement(el)).toEqual(el.nodeType)
-      expect(Util.isElement({})).toEqual(undefined)
+      expect(Util.isElement(el)).toEqual(true)
+      expect(Util.isElement({})).toEqual(false)
+      expect(Util.isElement(fixtureEl.querySelectorAll('.test'))).toEqual(false)
     })
 
     it('should detect jQuery element', () => {
       fixtureEl.innerHTML = '<div></div>'
 
-      const el = document.querySelector('div')
+      const el = fixtureEl.querySelector('div')
       const fakejQuery = {
-        0: el
+        0: el,
+        jquery: 'foo'
+      }
+
+      expect(Util.isElement(fakejQuery)).toEqual(true)
+    })
+  })
+
+  describe('getElement', () => {
+    it('should try to parse element', () => {
+      fixtureEl.innerHTML =
+        [
+          '<div id="foo" class="test"></div>',
+          '<div id="bar" class="test"></div>'
+        ].join('')
+
+      const el = fixtureEl.querySelector('div')
+
+      expect(Util.getElement(el)).toEqual(el)
+      expect(Util.getElement('#foo')).toEqual(el)
+      expect(Util.getElement('#fail')).toBeNull()
+      expect(Util.getElement({})).toBeNull()
+      expect(Util.getElement([])).toBeNull()
+      expect(Util.getElement()).toBeNull()
+      expect(Util.getElement(null)).toBeNull()
+      expect(Util.getElement(fixtureEl.querySelectorAll('.test'))).toBeNull()
+
+      const fakejQueryObject = {
+        0: el,
+        jquery: 'foo'
       }
 
-      expect(Util.isElement(fakejQuery)).toEqual(el.nodeType)
+      expect(Util.getElement(fakejQueryObject)).toEqual(el)
     })
   })