use get selector from element only when needed

8 files changed, 82 insertions, 49 deletions

diff --git a/js/src/alert/alert.js b/js/src/alert/alert.js
index 99164bd19..328aa16ae 100644
--- a/js/src/alert/alert.js
+++ b/js/src/alert/alert.js
@@ -9,7 +9,7 @@ import {
   jQuery as $,
   TRANSITION_END,
   emulateTransitionEnd,
-  getSelectorFromElement,
+  getElementFromSelector,
   getTransitionDurationFromElement
 } from '../util/index'
 import Data from '../dom/data'
@@ -90,12 +90,7 @@ class Alert {
   // Private
 
   _getRootElement(element) {
-    const selector = getSelectorFromElement(element)
-    let parent = false
-
-    if (selector) {
-      parent = SelectorEngine.findOne(selector)
-    }
+    let parent = getElementFromSelector(element)
 
     if (!parent) {
       parent = SelectorEngine.closest(element, `.${ClassName.ALERT}`)
diff --git a/js/src/carousel/carousel.js b/js/src/carousel/carousel.js
index 0e1bad14a..af4229f07 100644
--- a/js/src/carousel/carousel.js
+++ b/js/src/carousel/carousel.js
@@ -9,7 +9,7 @@ import {
   jQuery as $,
   TRANSITION_END,
   emulateTransitionEnd,
-  getSelectorFromElement,
+  getElementFromSelector,
   getTransitionDurationFromElement,
   isVisible,
   makeArray,
@@ -569,13 +569,7 @@ class Carousel {
   }
 
   static _dataApiClickHandler(event) {
-    const selector = getSelectorFromElement(this)
-
-    if (!selector) {
-      return
-    }
-
-    const target = SelectorEngine.findOne(selector)
+    const target = getElementFromSelector(this)
 
     if (!target || !target.classList.contains(ClassName.CAROUSEL)) {
       return
diff --git a/js/src/collapse/collapse.js b/js/src/collapse/collapse.js
index 671dc3b6c..c1d9aa2f1 100644
--- a/js/src/collapse/collapse.js
+++ b/js/src/collapse/collapse.js
@@ -10,6 +10,7 @@ import {
   TRANSITION_END,
   emulateTransitionEnd,
   getSelectorFromElement,
+  getElementFromSelector,
   getTransitionDurationFromElement,
   isElement,
   makeArray,
@@ -244,15 +245,11 @@ class Collapse {
     if (triggerArrayLength > 0) {
       for (let i = 0; i < triggerArrayLength; i++) {
         const trigger = this._triggerArray[i]
-        const selector = getSelectorFromElement(trigger)
+        const elem = getElementFromSelector(trigger)
 
-        if (selector !== null) {
-          const elem = SelectorEngine.findOne(selector)
-
-          if (!elem.classList.contains(ClassName.SHOW)) {
-            trigger.classList.add(ClassName.COLLAPSED)
-            trigger.setAttribute('aria-expanded', false)
-          }
+        if (elem && !elem.classList.contains(ClassName.SHOW)) {
+          trigger.classList.add(ClassName.COLLAPSED)
+          trigger.setAttribute('aria-expanded', false)
         }
       }
     }
@@ -320,8 +317,7 @@ class Collapse {
 
     makeArray(SelectorEngine.find(selector, parent))
       .forEach(element => {
-        const selector = getSelectorFromElement(element)
-        const selected = selector ? SelectorEngine.findOne(selector) : null
+        const selected = getElementFromSelector(element)
 
         this._addAriaAndCollapsedClass(
           selected,
diff --git a/js/src/dropdown/dropdown.js b/js/src/dropdown/dropdown.js
index 8607afe7f..f30c3924b 100644
--- a/js/src/dropdown/dropdown.js
+++ b/js/src/dropdown/dropdown.js
@@ -7,7 +7,7 @@
 
 import {
   jQuery as $,
-  getSelectorFromElement,
+  getElementFromSelector,
   isElement,
   makeArray,
   noop,
@@ -442,14 +442,7 @@ class Dropdown {
   }
 
   static _getParentFromElement(element) {
-    let parent
-    const selector = getSelectorFromElement(element)
-
-    if (selector) {
-      parent = SelectorEngine.findOne(selector)
-    }
-
-    return parent || element.parentNode
+    return getElementFromSelector(element) || element.parentNode
   }
 
   static _dataApiKeydownHandler(event) {
diff --git a/js/src/modal/modal.js b/js/src/modal/modal.js
index 4c430a01f..a0e460f22 100644
--- a/js/src/modal/modal.js
+++ b/js/src/modal/modal.js
@@ -9,7 +9,7 @@ import {
   jQuery as $,
   TRANSITION_END,
   emulateTransitionEnd,
-  getSelectorFromElement,
+  getElementFromSelector,
   getTransitionDurationFromElement,
   isVisible,
   makeArray,
@@ -549,8 +549,7 @@ class Modal {
  */
 
 EventHandler.on(document, Event.CLICK_DATA_API, Selector.DATA_TOGGLE, function (event) {
-  const selector = getSelectorFromElement(this)
-  const target = SelectorEngine.findOne(selector)
+  const target = getElementFromSelector(this)
 
   if (this.tagName === 'A' || this.tagName === 'AREA') {
     event.preventDefault()
diff --git a/js/src/tab/tab.js b/js/src/tab/tab.js
index e882ff1d2..c9f4869ef 100644
--- a/js/src/tab/tab.js
+++ b/js/src/tab/tab.js
@@ -9,7 +9,7 @@ import {
   jQuery as $,
   TRANSITION_END,
   emulateTransitionEnd,
-  getSelectorFromElement,
+  getElementFromSelector,
   getTransitionDurationFromElement,
   makeArray,
   reflow
@@ -85,10 +85,9 @@ class Tab {
       return
     }
 
-    let target
     let previous
+    const target = getElementFromSelector(this._element)
     const listElement = SelectorEngine.closest(this._element, Selector.NAV_LIST_GROUP)
-    const selector = getSelectorFromElement(this._element)
 
     if (listElement) {
       const itemSelector = listElement.nodeName === 'UL' || listElement.nodeName === 'OL' ? Selector.ACTIVE_UL : Selector.ACTIVE
@@ -113,10 +112,6 @@ class Tab {
       return
     }
 
-    if (selector) {
-      target = SelectorEngine.findOne(selector)
-    }
-
     this._activate(
       this._element,
       listElement
diff --git a/js/src/util/index.js b/js/src/util/index.js
index 5788c8749..537b391dc 100644
--- a/js/src/util/index.js
+++ b/js/src/util/index.js
@@ -28,20 +28,32 @@ const getUID = prefix => {
   return prefix
 }
 
-const getSelectorFromElement = element => {
+const getSelector = element => {
   let selector = element.getAttribute('data-target')
 
   if (!selector || selector === '#') {
     const hrefAttr = element.getAttribute('href')
 
-    selector = hrefAttr && hrefAttr !== '#' ? hrefAttr.trim() : ''
+    selector = hrefAttr && hrefAttr !== '#' ? hrefAttr.trim() : null
   }
 
-  try {
+  return selector
+}
+
+const getSelectorFromElement = element => {
+  const selector = getSelector(element)
+
+  if (selector) {
     return document.querySelector(selector) ? selector : null
-  } catch (error) {
-    return null
   }
+
+  return null
+}
+
+const getElementFromSelector = element => {
+  const selector = getSelector(element)
+
+  return selector ? document.querySelector(selector) : null
 }
 
 const getTransitionDurationFromElement = element => {
@@ -169,6 +181,7 @@ export {
   TRANSITION_END,
   getUID,
   getSelectorFromElement,
+  getElementFromSelector,
   getTransitionDurationFromElement,
   triggerTransitionEnd,
   isElement,
diff --git a/js/src/util/index.spec.js b/js/src/util/index.spec.js
index bf450ee36..b667c270e 100644
--- a/js/src/util/index.spec.js
+++ b/js/src/util/index.spec.js
@@ -64,6 +64,54 @@ describe('Util', () => {
 
       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-target', () => {
+      fixtureEl.innerHTML = [
+        '<div id="test" data-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-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', () => {