Dispose existing instance when a component is re-instantiated on an element (#42509)

Re-creating a component on an element that already has one (e.g. `new
Toast(el)` twice) silently overwrote the instances registry, orphaning the
previous instance with its event listeners and timers still attached. Dispose
the existing instance before registering the new one so it is cleaned up
instead of leaking, while keeping `getInstance()` returning the latest instance.

Also guard `_queueCallback` so a transition completion callback is skipped once
the instance has been disposed, preventing the `this._element is null` error
when `dispose()` is called mid-transition.

Closes #37245
Closes #41473

diff --git a/js/src/base-component.js b/js/src/base-component.js
index 504096a2be3c0da101eb3f1bc7cd8d8707a39398..e5d293a1d0f0d3133044585470f1ba7d487d25f4 100644 (file)
--- a/js/src/base-component.js
+++ b/js/src/base-component.js
@@ -32,6 +32,13 @@ class BaseComponent extends Config {
     this._element = element
     this._config = this._getConfig(config)
 
+    // Dispose any existing instance bound to this element before registering the new one,
+    // so its event listeners and timers are cleaned up instead of leaking
+    const existingInstance = Data.get(this._element, this.constructor.DATA_KEY)
+    if (existingInstance) {
+      existingInstance.dispose()
+    }
+
     Data.set(this._element, this.constructor.DATA_KEY, this)
   }
 
@@ -47,7 +54,14 @@ class BaseComponent extends Config {
 
   // Private
   _queueCallback(callback, element, isAnimated = true) {
-    executeAfterTransition(callback, element, isAnimated)
+    executeAfterTransition(() => {
+      // Don't run the completion callback if the instance was disposed mid-transition
+      if (!this._element) {
+        return
+      }
+
+      callback()
+    }, element, isAnimated)
   }
 
   _getConfig(config) {

diff --git a/js/tests/unit/alert.spec.js b/js/tests/unit/alert.spec.js
index e6c153d1b16e996404a60a39935e27332eb7d86a..02ac0198663db79d6893489815452d742b82aa46 100644 (file)
--- a/js/tests/unit/alert.spec.js
+++ b/js/tests/unit/alert.spec.js
@@ -18,9 +18,9 @@ describe('Alert', () => {
 
     const alertEl = fixtureEl.querySelector('.alert')
     const alertBySelector = new Alert('.alert')
-    const alertByElement = new Alert(alertEl)
-
     expect(alertBySelector._element).toEqual(alertEl)
+
+    const alertByElement = new Alert(alertEl)
     expect(alertByElement._element).toEqual(alertEl)
   })
 

diff --git a/js/tests/unit/base-component.spec.js b/js/tests/unit/base-component.spec.js
index e90b42de185bd7826df0eefc163a30a7529c8b5a..4b67f73b24ae66661e70a9ed3ac0368d7ef16477 100644 (file)
--- a/js/tests/unit/base-component.spec.js
+++ b/js/tests/unit/base-component.spec.js
@@ -93,6 +93,19 @@ describe('Base Component', () => {
         expect(elInstance._element).not.toBeDefined()
         expect(selectorInstance._element).not.toBeDefined()
       })
+
+      it('should dispose an existing instance when re-instantiated on the same element', () => {
+        fixtureEl.innerHTML = '<div id="foo"></div>'
+
+        const el = fixtureEl.querySelector('#foo')
+        const firstInstance = new DummyClass(el)
+        const disposeSpy = spyOn(firstInstance, 'dispose').and.callThrough()
+        const secondInstance = new DummyClass(el)
+
+        expect(disposeSpy).toHaveBeenCalled()
+        expect(DummyClass.getInstance(el)).toEqual(secondInstance)
+        expect(DummyClass.getInstance(el)).not.toEqual(firstInstance)
+      })
     })
 
     describe('dispose', () => {
@@ -114,6 +127,21 @@ describe('Base Component', () => {
 
         expect(spy).toHaveBeenCalledWith(element, DummyClass.EVENT_KEY)
       })
+
+      it('should not run a queued transition callback once the instance is disposed', () => {
+        return new Promise(resolve => {
+          createInstance()
+          const callbackSpy = jasmine.createSpy('callback')
+
+          instance._queueCallback(callbackSpy, element, true)
+          instance.dispose()
+
+          setTimeout(() => {
+            expect(callbackSpy).not.toHaveBeenCalled()
+            resolve()
+          }, 50)
+        })
+      })
     })
 
     describe('getInstance', () => {

diff --git a/js/tests/unit/button.spec.js b/js/tests/unit/button.spec.js
index ed8bb1c84d1b332e9e383ac65be83d429ac2e245..db63e73f37ebb0289cdcc6268c9f86dbc7de5ed2 100644 (file)
--- a/js/tests/unit/button.spec.js
+++ b/js/tests/unit/button.spec.js
@@ -16,9 +16,9 @@ describe('Button', () => {
     fixtureEl.innerHTML = '<button data-bs-toggle="button">Placeholder</button>'
     const buttonEl = fixtureEl.querySelector('[data-bs-toggle="button"]')
     const buttonBySelector = new Button('[data-bs-toggle="button"]')
-    const buttonByElement = new Button(buttonEl)
-
     expect(buttonBySelector._element).toEqual(buttonEl)
+
+    const buttonByElement = new Button(buttonEl)
     expect(buttonByElement._element).toEqual(buttonEl)
   })
 

diff --git a/js/tests/unit/carousel.spec.js b/js/tests/unit/carousel.spec.js
index 5f670e54331b34fad4df6ce255b6a616b064ad6b..d8e575c99a075de37f6dc3ed4d2d6233ad8229df 100644 (file)
--- a/js/tests/unit/carousel.spec.js
+++ b/js/tests/unit/carousel.spec.js
@@ -156,9 +156,9 @@ describe('Carousel', () => {
 
       const carouselEl = fixtureEl.querySelector('#myCarousel')
       const carouselBySelector = new Carousel('#myCarousel')
-      const carouselByElement = new Carousel(carouselEl)
-
       expect(carouselBySelector._element).toEqual(carouselEl)
+
+      const carouselByElement = new Carousel(carouselEl)
       expect(carouselByElement._element).toEqual(carouselEl)
     })
 

diff --git a/js/tests/unit/collapse.spec.js b/js/tests/unit/collapse.spec.js
index 5dc901ae68aa358d1d29a0533f518f6fbd53e585..9c69fccafc2f1793e33726d5975ceb1de58bf6fe 100644 (file)
--- a/js/tests/unit/collapse.spec.js
+++ b/js/tests/unit/collapse.spec.js
@@ -37,9 +37,9 @@ describe('Collapse', () => {
 
       const collapseEl = fixtureEl.querySelector('div.my-collapse')
       const collapseBySelector = new Collapse('div.my-collapse')
-      const collapseByElement = new Collapse(collapseEl)
-
       expect(collapseBySelector._element).toEqual(collapseEl)
+
+      const collapseByElement = new Collapse(collapseEl)
       expect(collapseByElement._element).toEqual(collapseEl)
     })
 

diff --git a/js/tests/unit/datepicker.spec.js b/js/tests/unit/datepicker.spec.js
index c8e56831d1bddc556e076db5ae819a8f8c623964..f8efc45fc1b042af67e4c7aa6c3118eb728c2e61 100644 (file)
--- a/js/tests/unit/datepicker.spec.js
+++ b/js/tests/unit/datepicker.spec.js
@@ -67,9 +67,9 @@ describe('Datepicker', () => {
 
       const inputEl = fixtureEl.querySelector('#datepickerEl')
       const datepickerBySelector = new Datepicker('#datepickerEl')
-      const datepickerByElement = new Datepicker(inputEl)
-
       expect(datepickerBySelector._element).toEqual(inputEl)
+
+      const datepickerByElement = new Datepicker(inputEl)
       expect(datepickerByElement._element).toEqual(inputEl)
     })
 

diff --git a/js/tests/unit/dialog.spec.js b/js/tests/unit/dialog.spec.js
index 92c0f80eb18d6265e98ce95865e2701ecc5230c4..c763abe5d25ce14ac1ca5d8854fb94395df2e557 100644 (file)
--- a/js/tests/unit/dialog.spec.js
+++ b/js/tests/unit/dialog.spec.js
@@ -50,9 +50,9 @@ describe('Dialog', () => {
 
       const dialogEl = fixtureEl.querySelector('.dialog')
       const dialogBySelector = new Dialog('#testDialog')
-      const dialogByElement = new Dialog(dialogEl)
-
       expect(dialogBySelector._element).toEqual(dialogEl)
+
+      const dialogByElement = new Dialog(dialogEl)
       expect(dialogByElement._element).toEqual(dialogEl)
     })
   })

diff --git a/js/tests/unit/menu.spec.js b/js/tests/unit/menu.spec.js
index 108fbd93f04af97d25b3847b4e6f5fd62a7e78ad..a02005ac5cbf3dfef903342fcc381872f5a245bc 100644 (file)
--- a/js/tests/unit/menu.spec.js
+++ b/js/tests/unit/menu.spec.js
@@ -53,9 +53,9 @@ describe('Menu', () => {
 
       const btnMenu = fixtureEl.querySelector('[data-bs-toggle="menu"]')
       const menuBySelector = new Menu('[data-bs-toggle="menu"]')
-      const menuByElement = new Menu(btnMenu)
-
       expect(menuBySelector._element).toEqual(btnMenu)
+
+      const menuByElement = new Menu(btnMenu)
       expect(menuByElement._element).toEqual(btnMenu)
     })
 

diff --git a/js/tests/unit/nav-overflow.spec.js b/js/tests/unit/nav-overflow.spec.js
index 2f8fc6321b62f57787a89f1f85aeef9a70323848..8de3646d37ad9e4d721f04cfebd232f4a29a71d9 100644 (file)
--- a/js/tests/unit/nav-overflow.spec.js
+++ b/js/tests/unit/nav-overflow.spec.js
@@ -43,12 +43,11 @@ describe('NavOverflow', () => {
 
       const navEl = fixtureEl.querySelector('[data-bs-toggle="nav-overflow"]')
       const navBySelector = new NavOverflow('[data-bs-toggle="nav-overflow"]')
-      const navByElement = new NavOverflow(navEl)
-
       expect(navBySelector._element).toEqual(navEl)
+
+      const navByElement = new NavOverflow(navEl)
       expect(navByElement._element).toEqual(navEl)
 
-      navBySelector.dispose()
       navByElement.dispose()
     })
 

diff --git a/js/tests/unit/otp-input.spec.js b/js/tests/unit/otp-input.spec.js
index 7aa4cd619d9724bcd1914cc9078bbc7c0c1ed28b..f94aedbf3d337f25451d57ce178dd8a3f2080cab 100644 (file)
--- a/js/tests/unit/otp-input.spec.js
+++ b/js/tests/unit/otp-input.spec.js
@@ -52,9 +52,9 @@ describe('OtpInput', () => {
 
       const otpEl = fixtureEl.querySelector('.otp')
       const otpBySelector = new OtpInput('.otp')
-      const otpByElement = new OtpInput(otpEl)
-
       expect(otpBySelector._element).toEqual(otpEl)
+
+      const otpByElement = new OtpInput(otpEl)
       expect(otpByElement._element).toEqual(otpEl)
     })
 

diff --git a/js/tests/unit/scrollspy.spec.js b/js/tests/unit/scrollspy.spec.js
index f5e75029377de44b69656b8d4bd4f7a9b64f2943..bf72e0a4a8b2f160603a88673edaae9b264d528f 100644 (file)
--- a/js/tests/unit/scrollspy.spec.js
+++ b/js/tests/unit/scrollspy.spec.js
@@ -100,9 +100,9 @@ describe('ScrollSpy', () => {
 
       const sSpyEl = fixtureEl.querySelector('.content')
       const sSpyBySelector = new ScrollSpy('.content')
-      const sSpyByElement = new ScrollSpy(sSpyEl)
-
       expect(sSpyBySelector._element).toEqual(sSpyEl)
+
+      const sSpyByElement = new ScrollSpy(sSpyEl)
       expect(sSpyByElement._element).toEqual(sSpyEl)
     })
 

diff --git a/js/tests/unit/strength.spec.js b/js/tests/unit/strength.spec.js
index e65033401cb725367b76e7d2cd0c236864c8de98..f899c7a23121dfa8cea308e7c7c7dc089dbb219e 100644 (file)
--- a/js/tests/unit/strength.spec.js
+++ b/js/tests/unit/strength.spec.js
@@ -71,9 +71,9 @@ describe('Strength', () => {
 
       const strengthEl = fixtureEl.querySelector('.strength')
       const strengthBySelector = new Strength('.strength')
-      const strengthByElement = new Strength(strengthEl)
-
       expect(strengthBySelector._element).toEqual(strengthEl)
+
+      const strengthByElement = new Strength(strengthEl)
       expect(strengthByElement._element).toEqual(strengthEl)
     })
 

diff --git a/js/tests/unit/tab.spec.js b/js/tests/unit/tab.spec.js
index fba9674d5fd7c1c8cfc53b311b6cfe9648c9634d..a3cd9b8e4cb4b71e4d695a6387504b56ce287ac1 100644 (file)
--- a/js/tests/unit/tab.spec.js
+++ b/js/tests/unit/tab.spec.js
@@ -33,9 +33,9 @@ describe('Tab', () => {
 
       const tabEl = fixtureEl.querySelector('[href="#home"]')
       const tabBySelector = new Tab('[href="#home"]')
-      const tabByElement = new Tab(tabEl)
-
       expect(tabBySelector._element).toEqual(tabEl)
+
+      const tabByElement = new Tab(tabEl)
       expect(tabByElement._element).toEqual(tabEl)
     })
 

diff --git a/js/tests/unit/toast.spec.js b/js/tests/unit/toast.spec.js
index 7dcf82de89e651e84755154701c5f204b8cd0e39..078b3fd7a60434e65785a7c907545e3c1f2094ee 100644 (file)
--- a/js/tests/unit/toast.spec.js
+++ b/js/tests/unit/toast.spec.js
@@ -32,9 +32,9 @@ describe('Toast', () => {
 
       const toastEl = fixtureEl.querySelector('.toast')
       const toastBySelector = new Toast('.toast')
-      const toastByElement = new Toast(toastEl)
-
       expect(toastBySelector._element).toEqual(toastEl)
+
+      const toastByElement = new Toast(toastEl)
       expect(toastByElement._element).toEqual(toastEl)
     })
 

diff --git a/js/tests/unit/toggler.spec.js b/js/tests/unit/toggler.spec.js
index e7806be3663fd5c25309274aa25861d1fbd630cd..4692ece0c0ce537f8c38459f3795e501bed04287 100644 (file)
--- a/js/tests/unit/toggler.spec.js
+++ b/js/tests/unit/toggler.spec.js
@@ -24,9 +24,9 @@ describe('Toggler', () => {
 
       const togglerEl = fixtureEl.querySelector('[data-bs-toggle="toggler"]')
       const togglerBySelector = new Toggler('[data-bs-toggle="toggler"]')
-      const togglerByElement = new Toggler(togglerEl)
-
       expect(togglerBySelector._element).toEqual(togglerEl)
+
+      const togglerByElement = new Toggler(togglerEl)
       expect(togglerByElement._element).toEqual(togglerEl)
     })
   })

diff --git a/js/tests/unit/tooltip.spec.js b/js/tests/unit/tooltip.spec.js
index a20a80bb5e335a2c27758e8a4157515f07be7226..3f1650b79cab15651aacb0f9bd9fb338997e8f8b 100644 (file)
--- a/js/tests/unit/tooltip.spec.js
+++ b/js/tests/unit/tooltip.spec.js
@@ -62,9 +62,9 @@ describe('Tooltip', () => {
 
       const tooltipEl = fixtureEl.querySelector('#tooltipEl')
       const tooltipBySelector = new Tooltip('#tooltipEl')
-      const tooltipByElement = new Tooltip(tooltipEl)
-
       expect(tooltipBySelector._element).toEqual(tooltipEl)
+
+      const tooltipByElement = new Tooltip(tooltipEl)
       expect(tooltipByElement._element).toEqual(tooltipEl)
     })