refactor: use a Map instead of an Object in dom/data (#32180)

Co-authored-by: XhmikosR <xhmikosr@gmail.com>
Co-authored-by: Rohit Sharma <rohit2sharma95@gmail.com>

15 files changed, 126 insertions, 161 deletions

diff --git a/.bundlewatch.config.json b/.bundlewatch.config.json
index bd9ce671d4..a68eeb85be 100644
--- a/.bundlewatch.config.json
+++ b/.bundlewatch.config.json
@@ -38,7 +38,7 @@
     },
     {
       "path": "./dist/js/bootstrap.bundle.min.js",
-      "maxSize": "21.75 kB"
+      "maxSize": "22 kB"
     },
     {
       "path": "./dist/js/bootstrap.esm.js",
diff --git a/js/src/alert.js b/js/src/alert.js
index 8fc3f12a8d..d10e6c8da3 100644
--- a/js/src/alert.js
+++ b/js/src/alert.js
@@ -98,7 +98,7 @@ class Alert extends BaseComponent {
 
   static jQueryInterface(config) {
     return this.each(function () {
-      let data = Data.getData(this, DATA_KEY)
+      let data = Data.get(this, DATA_KEY)
 
       if (!data) {
         data = new Alert(this)
diff --git a/js/src/base-component.js b/js/src/base-component.js
index 989a641561..2a9e29c2aa 100644
--- a/js/src/base-component.js
+++ b/js/src/base-component.js
@@ -24,18 +24,18 @@ class BaseComponent {
     }
 
     this._element = element
-    Data.setData(this._element, this.constructor.DATA_KEY, this)
+    Data.set(this._element, this.constructor.DATA_KEY, this)
   }
 
   dispose() {
-    Data.removeData(this._element, this.constructor.DATA_KEY)
+    Data.remove(this._element, this.constructor.DATA_KEY)
     this._element = null
   }
 
   /** Static */
 
   static getInstance(element) {
-    return Data.getData(element, this.DATA_KEY)
+    return Data.get(element, this.DATA_KEY)
   }
 
   static get VERSION() {
diff --git a/js/src/button.js b/js/src/button.js
index 4ec48ca083..7a9449f075 100644
--- a/js/src/button.js
+++ b/js/src/button.js
@@ -51,7 +51,7 @@ class Button extends BaseComponent {
 
   static jQueryInterface(config) {
     return this.each(function () {
-      let data = Data.getData(this, DATA_KEY)
+      let data = Data.get(this, DATA_KEY)
 
       if (!data) {
         data = new Button(this)
@@ -75,7 +75,7 @@ EventHandler.on(document, EVENT_CLICK_DATA_API, SELECTOR_DATA_TOGGLE, event => {
 
   const button = event.target.closest(SELECTOR_DATA_TOGGLE)
 
-  let data = Data.getData(button, DATA_KEY)
+  let data = Data.get(button, DATA_KEY)
   if (!data) {
     data = new Button(button)
   }
diff --git a/js/src/carousel.js b/js/src/carousel.js
index 75f8a4da79..a825aaef48 100644
--- a/js/src/carousel.js
+++ b/js/src/carousel.js
@@ -527,7 +527,7 @@ class Carousel extends BaseComponent {
   // Static
 
   static carouselInterface(element, config) {
-    let data = Data.getData(element, DATA_KEY)
+    let data = Data.get(element, DATA_KEY)
     let _config = {
       ...Default,
       ...Manipulator.getDataAttributes(element)
@@ -586,7 +586,7 @@ class Carousel extends BaseComponent {
     Carousel.carouselInterface(target, config)
 
     if (slideIndex) {
-      Data.getData(target, DATA_KEY).to(slideIndex)
+      Data.get(target, DATA_KEY).to(slideIndex)
     }
 
     event.preventDefault()
@@ -605,7 +605,7 @@ EventHandler.on(window, EVENT_LOAD_DATA_API, () => {
   const carousels = SelectorEngine.find(SELECTOR_DATA_RIDE)
 
   for (let i = 0, len = carousels.length; i < len; i++) {
-    Carousel.carouselInterface(carousels[i], Data.getData(carousels[i], DATA_KEY))
+    Carousel.carouselInterface(carousels[i], Data.get(carousels[i], DATA_KEY))
   }
 })
 
diff --git a/js/src/collapse.js b/js/src/collapse.js
index f861667659..036ffcf248 100644
--- a/js/src/collapse.js
+++ b/js/src/collapse.js
@@ -147,7 +147,7 @@ class Collapse extends BaseComponent {
     const container = SelectorEngine.findOne(this._selector)
     if (actives) {
       const tempActiveData = actives.find(elem => container !== elem)
-      activesData = tempActiveData ? Data.getData(tempActiveData, DATA_KEY) : null
+      activesData = tempActiveData ? Data.get(tempActiveData, DATA_KEY) : null
 
       if (activesData && activesData._isTransitioning) {
         return
@@ -166,7 +166,7 @@ class Collapse extends BaseComponent {
         }
 
         if (!activesData) {
-          Data.setData(elemActive, DATA_KEY, null)
+          Data.set(elemActive, DATA_KEY, null)
         }
       })
     }
@@ -332,7 +332,7 @@ class Collapse extends BaseComponent {
   // Static
 
   static collapseInterface(element, config) {
-    let data = Data.getData(element, DATA_KEY)
+    let data = Data.get(element, DATA_KEY)
     const _config = {
       ...Default,
       ...Manipulator.getDataAttributes(element),
@@ -380,7 +380,7 @@ EventHandler.on(document, EVENT_CLICK_DATA_API, SELECTOR_DATA_TOGGLE, function (
   const selectorElements = SelectorEngine.find(selector)
 
   selectorElements.forEach(element => {
-    const data = Data.getData(element, DATA_KEY)
+    const data = Data.get(element, DATA_KEY)
     let config
     if (data) {
       // update parent attribute
diff --git a/js/src/dom/data.js b/js/src/dom/data.js
index c93a8dc7c5..1d283d68b8 100644
--- a/js/src/dom/data.js
+++ b/js/src/dom/data.js
@@ -11,57 +11,47 @@
  * ------------------------------------------------------------------------
  */
 
-const mapData = (() => {
-  const storeData = {}
-  let id = 1
-  return {
-    set(element, key, data) {
-      if (typeof element.bsKey === 'undefined') {
-        element.bsKey = {
-          key,
-          id
-        }
-        id++
-      }
+const elementMap = new Map()
 
-      storeData[element.bsKey.id] = data
-    },
-    get(element, key) {
-      if (!element || typeof element.bsKey === 'undefined') {
-        return null
-      }
-
-      const keyProperties = element.bsKey
-      if (keyProperties.key === key) {
-        return storeData[keyProperties.id]
-      }
+export default {
+  set(element, key, instance) {
+    if (!elementMap.has(element)) {
+      elementMap.set(element, new Map())
+    }
 
-      return null
-    },
-    delete(element, key) {
-      if (typeof element.bsKey === 'undefined') {
-        return
-      }
+    const instanceMap = elementMap.get(element)
 
-      const keyProperties = element.bsKey
-      if (keyProperties.key === key) {
-        delete storeData[keyProperties.id]
-        delete element.bsKey
-      }
+    // make it clear we only want one instance per element
+    // can be removed later when multiple key/instances are fine to be used
+    if (!instanceMap.has(key) && instanceMap.size !== 0) {
+      // eslint-disable-next-line no-console
+      console.error(`Bootstrap doesn't allow more than one instance per element. Bound instance: ${Array.from(instanceMap.keys())[0]}.`)
+      return
     }
-  }
-})()
 
-const Data = {
-  setData(instance, key, data) {
-    mapData.set(instance, key, data)
+    instanceMap.set(key, instance)
   },
-  getData(instance, key) {
-    return mapData.get(instance, key)
+
+  get(element, key) {
+    if (elementMap.has(element)) {
+      return elementMap.get(element).get(key) || null
+    }
+
+    return null
   },
-  removeData(instance, key) {
-    mapData.delete(instance, key)
+
+  remove(element, key) {
+    if (!elementMap.has(element)) {
+      return
+    }
+
+    const instanceMap = elementMap.get(element)
+
+    instanceMap.delete(key)
+
+    // free up element references if there are no instances left for an element
+    if (instanceMap.size === 0) {
+      elementMap.delete(element)
+    }
   }
 }
-
-export default Data
diff --git a/js/src/dropdown.js b/js/src/dropdown.js
index 590c748012..fea0b1919b 100644
--- a/js/src/dropdown.js
+++ b/js/src/dropdown.js
@@ -357,7 +357,7 @@ class Dropdown extends BaseComponent {
   // Static
 
   static dropdownInterface(element, config) {
-    let data = Data.getData(element, DATA_KEY)
+    let data = Data.get(element, DATA_KEY)
     const _config = typeof config === 'object' ? config : null
 
     if (!data) {
@@ -387,7 +387,7 @@ class Dropdown extends BaseComponent {
     const toggles = SelectorEngine.find(SELECTOR_DATA_TOGGLE)
 
     for (let i = 0, len = toggles.length; i < len; i++) {
-      const context = Data.getData(toggles[i], DATA_KEY)
+      const context = Data.get(toggles[i], DATA_KEY)
       const relatedTarget = {
         relatedTarget: toggles[i]
       }
diff --git a/js/src/modal.js b/js/src/modal.js
index 5afb9791b4..332d636d0f 100644
--- a/js/src/modal.js
+++ b/js/src/modal.js
@@ -508,7 +508,7 @@ class Modal extends BaseComponent {
 
   static jQueryInterface(config, relatedTarget) {
     return this.each(function () {
-      let data = Data.getData(this, DATA_KEY)
+      let data = Data.get(this, DATA_KEY)
       const _config = {
         ...Default,
         ...Manipulator.getDataAttributes(this),
@@ -556,7 +556,7 @@ EventHandler.on(document, EVENT_CLICK_DATA_API, SELECTOR_DATA_TOGGLE, function (
     })
   })
 
-  let data = Data.getData(target, DATA_KEY)
+  let data = Data.get(target, DATA_KEY)
   if (!data) {
     const config = {
       ...Manipulator.getDataAttributes(target),
diff --git a/js/src/popover.js b/js/src/popover.js
index 0677dafa09..5535447547 100644
--- a/js/src/popover.js
+++ b/js/src/popover.js
@@ -136,7 +136,7 @@ class Popover extends Tooltip {
 
   static jQueryInterface(config) {
     return this.each(function () {
-      let data = Data.getData(this, DATA_KEY)
+      let data = Data.get(this, DATA_KEY)
       const _config = typeof config === 'object' ? config : null
 
       if (!data && /dispose|hide/.test(config)) {
@@ -145,7 +145,7 @@ class Popover extends Tooltip {
 
       if (!data) {
         data = new Popover(this, _config)
-        Data.setData(this, DATA_KEY, data)
+        Data.set(this, DATA_KEY, data)
       }
 
       if (typeof config === 'string') {
diff --git a/js/src/scrollspy.js b/js/src/scrollspy.js
index 0c51eab0fe..c7472439be 100644
--- a/js/src/scrollspy.js
+++ b/js/src/scrollspy.js
@@ -278,7 +278,7 @@ class ScrollSpy extends BaseComponent {
 
   static jQueryInterface(config) {
     return this.each(function () {
-      let data = Data.getData(this, DATA_KEY)
+      let data = Data.get(this, DATA_KEY)
       const _config = typeof config === 'object' && config
 
       if (!data) {
diff --git a/js/src/tab.js b/js/src/tab.js
index e60ecddb56..95968f4f8b 100644
--- a/js/src/tab.js
+++ b/js/src/tab.js
@@ -182,7 +182,7 @@ class Tab extends BaseComponent {
 
   static jQueryInterface(config) {
     return this.each(function () {
-      const data = Data.getData(this, DATA_KEY) || new Tab(this)
+      const data = Data.get(this, DATA_KEY) || new Tab(this)
 
       if (typeof config === 'string') {
         if (typeof data[config] === 'undefined') {
@@ -204,7 +204,7 @@ class Tab extends BaseComponent {
 EventHandler.on(document, EVENT_CLICK_DATA_API, SELECTOR_DATA_TOGGLE, function (event) {
   event.preventDefault()
 
-  const data = Data.getData(this, DATA_KEY) || new Tab(this)
+  const data = Data.get(this, DATA_KEY) || new Tab(this)
   data.show()
 })
 
diff --git a/js/src/toast.js b/js/src/toast.js
index 2f451aab76..ea91163d84 100644
--- a/js/src/toast.js
+++ b/js/src/toast.js
@@ -189,7 +189,7 @@ class Toast extends BaseComponent {
 
   static jQueryInterface(config) {
     return this.each(function () {
-      let data = Data.getData(this, DATA_KEY)
+      let data = Data.get(this, DATA_KEY)
       const _config = typeof config === 'object' && config
 
       if (!data) {
diff --git a/js/src/tooltip.js b/js/src/tooltip.js
index d35b5e0ab1..6f33245f8e 100644
--- a/js/src/tooltip.js
+++ b/js/src/tooltip.js
@@ -275,7 +275,7 @@ class Tooltip extends BaseComponent {
     this._addAttachmentClass(attachment)
 
     const container = this._getContainer()
-    Data.setData(tip, this.constructor.DATA_KEY, this)
+    Data.set(tip, this.constructor.DATA_KEY, this)
 
     if (!this._element.ownerDocument.documentElement.contains(this.tip)) {
       container.appendChild(tip)
@@ -465,11 +465,11 @@ class Tooltip extends BaseComponent {
 
   _initializeOnDelegatedTarget(event, context) {
     const dataKey = this.constructor.DATA_KEY
-    context = context || Data.getData(event.delegateTarget, dataKey)
+    context = context || Data.get(event.delegateTarget, dataKey)
 
     if (!context) {
       context = new this.constructor(event.delegateTarget, this._getDelegateConfig())
-      Data.setData(event.delegateTarget, dataKey, context)
+      Data.set(event.delegateTarget, dataKey, context)
     }
 
     return context
@@ -761,7 +761,7 @@ class Tooltip extends BaseComponent {
 
   static jQueryInterface(config) {
     return this.each(function () {
-      let data = Data.getData(this, DATA_KEY)
+      let data = Data.get(this, DATA_KEY)
       const _config = typeof config === 'object' && config
 
       if (!data && /dispose|hide/.test(config)) {
diff --git a/js/tests/unit/dom/data.spec.js b/js/tests/unit/dom/data.spec.js
index c80f32db0f..a00d3b7341 100644
--- a/js/tests/unit/dom/data.spec.js
+++ b/js/tests/unit/dom/data.spec.js
@@ -4,128 +4,103 @@ import Data from '../../../src/dom/data'
 import { getFixture, clearFixture } from '../../helpers/fixture'
 
 describe('Data', () => {
+  const TEST_KEY = 'bs.test'
+  const UNKNOWN_KEY = 'bs.unknown'
+  const TEST_DATA = {
+    test: 'bsData'
+  }
+
   let fixtureEl
+  let div
 
   beforeAll(() => {
     fixtureEl = getFixture()
   })
 
+  beforeEach(() => {
+    fixtureEl.innerHTML = '<div></div>'
+    div = fixtureEl.querySelector('div')
+  })
+
   afterEach(() => {
+    Data.remove(div, TEST_KEY)
     clearFixture()
   })
 
-  describe('setData', () => {
-    it('should set data in an element by adding a bsKey attribute', () => {
-      fixtureEl.innerHTML = '<div></div>'
-
-      const div = fixtureEl.querySelector('div')
-      const data = {
-        test: 'bsData'
-      }
-
-      Data.setData(div, 'test', data)
-      expect(div.bsKey).toBeDefined()
-    })
+  it('should return null for unknown elements', () => {
+    const data = { ...TEST_DATA }
 
-    it('should change data if something is already stored', () => {
-      fixtureEl.innerHTML = '<div></div>'
+    Data.set(div, TEST_KEY, data)
 
-      const div = fixtureEl.querySelector('div')
-      const data = {
-        test: 'bsData'
-      }
-
-      Data.setData(div, 'test', data)
-
-      data.test = 'bsData2'
-      Data.setData(div, 'test', data)
-
-      expect(div.bsKey).toBeDefined()
-    })
+    expect(Data.get(null)).toBeNull()
+    expect(Data.get(undefined)).toBeNull()
+    expect(Data.get(document.createElement('div'), TEST_KEY)).toBeNull()
   })
 
-  describe('getData', () => {
-    it('should return stored data', () => {
-      fixtureEl.innerHTML = '<div></div>'
-
-      const div = fixtureEl.querySelector('div')
-      const data = {
-        test: 'bsData'
-      }
+  it('should return null for unknown keys', () => {
+    const data = { ...TEST_DATA }
 
-      Data.setData(div, 'test', data)
-      expect(Data.getData(div, 'test')).toEqual(data)
-    })
+    Data.set(div, TEST_KEY, data)
 
-    it('should return null on undefined element', () => {
-      expect(Data.getData(null)).toEqual(null)
-      expect(Data.getData(undefined)).toEqual(null)
-    })
-
-    it('should return null when an element have nothing stored', () => {
-      fixtureEl.innerHTML = '<div></div>'
-
-      const div = fixtureEl.querySelector('div')
-
-      expect(Data.getData(div, 'test')).toEqual(null)
-    })
-
-    it('should return null when an element have nothing stored with the provided key', () => {
-      fixtureEl.innerHTML = '<div></div>'
+    expect(Data.get(div, null)).toBeNull()
+    expect(Data.get(div, undefined)).toBeNull()
+    expect(Data.get(div, UNKNOWN_KEY)).toBeNull()
+  })
 
-      const div = fixtureEl.querySelector('div')
-      const data = {
-        test: 'bsData'
-      }
+  it('should store data for an element with a given key and return it', () => {
+    const data = { ...TEST_DATA }
 
-      Data.setData(div, 'test', data)
+    Data.set(div, TEST_KEY, data)
 
-      expect(Data.getData(div, 'test2')).toEqual(null)
-    })
+    expect(Data.get(div, TEST_KEY)).toBe(data)
   })
 
-  describe('removeData', () => {
-    it('should do nothing when an element have nothing stored', () => {
-      fixtureEl.innerHTML = '<div></div>'
+  it('should overwrite data if something is already stored', () => {
+    const data = { ...TEST_DATA }
+    const copy = { ...data }
 
-      const div = fixtureEl.querySelector('div')
+    Data.set(div, TEST_KEY, data)
+    Data.set(div, TEST_KEY, copy)
 
-      Data.removeData(div, 'test')
-      expect().nothing()
-    })
+    expect(Data.get(div, TEST_KEY)).not.toBe(data)
+    expect(Data.get(div, TEST_KEY)).toBe(copy)
+  })
 
-    it('should should do nothing if it\'s not a valid key provided', () => {
-      fixtureEl.innerHTML = '<div></div>'
+  it('should do nothing when an element have nothing stored', () => {
+    Data.remove(div, TEST_KEY)
 
-      const div = fixtureEl.querySelector('div')
-      const data = {
-        test: 'bsData'
-      }
+    expect().nothing()
+  })
 
-      Data.setData(div, 'test', data)
+  it('should remove nothing for an unknown key', () => {
+    const data = { ...TEST_DATA }
 
-      expect(div.bsKey).toBeDefined()
+    Data.set(div, TEST_KEY, data)
+    Data.remove(div, UNKNOWN_KEY)
 
-      Data.removeData(div, 'test2')
+    expect(Data.get(div, TEST_KEY)).toBe(data)
+  })
 
-      expect(div.bsKey).toBeDefined()
-    })
+  it('should remove data for a given key', () => {
+    const data = { ...TEST_DATA }
 
-    it('should remove data if something is stored', () => {
-      fixtureEl.innerHTML = '<div></div>'
+    Data.set(div, TEST_KEY, data)
+    Data.remove(div, TEST_KEY)
 
-      const div = fixtureEl.querySelector('div')
-      const data = {
-        test: 'bsData'
-      }
+    expect(Data.get(div, TEST_KEY)).toBeNull()
+  })
 
-      Data.setData(div, 'test', data)
+  it('should console.error a message if called with multiple keys', () => {
+    /* eslint-disable no-console */
+    console.error = jasmine.createSpy('console.error')
 
-      expect(div.bsKey).toBeDefined()
+    const data = { ...TEST_DATA }
+    const copy = { ...data }
 
-      Data.removeData(div, 'test')
+    Data.set(div, TEST_KEY, data)
+    Data.set(div, UNKNOWN_KEY, copy)
 
-      expect(div.bsKey).toBeUndefined()
-    })
+    expect(console.error).toHaveBeenCalled()
+    expect(Data.get(div, UNKNOWN_KEY)).toBe(null)
   })
 })