From: Mark Otto Date: Thu, 18 Jun 2026 02:25:44 +0000 (-0700) Subject: Dispose existing instance when a component is re-instantiated on an element (#42509) X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=1f6bdf8c6410951a7ded5c24700610d33bd069e5;p=thirdparty%2Fbootstrap.git 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 504096a2be..e5d293a1d0 100644 --- 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 e6c153d1b1..02ac019866 100644 --- 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 e90b42de18..4b67f73b24 100644 --- 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 = '
' + + 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 ed8bb1c84d..db63e73f37 100644 --- a/js/tests/unit/button.spec.js +++ b/js/tests/unit/button.spec.js @@ -16,9 +16,9 @@ describe('Button', () => { fixtureEl.innerHTML = '' 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 5f670e5433..d8e575c99a 100644 --- 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 5dc901ae68..9c69fccafc 100644 --- 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 c8e56831d1..f8efc45fc1 100644 --- 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 92c0f80eb1..c763abe5d2 100644 --- 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 108fbd93f0..a02005ac5c 100644 --- 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 2f8fc6321b..8de3646d37 100644 --- 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 7aa4cd619d..f94aedbf3d 100644 --- 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 f5e7502937..bf72e0a4a8 100644 --- 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 e65033401c..f899c7a231 100644 --- 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 fba9674d5f..a3cd9b8e4c 100644 --- 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 7dcf82de89..078b3fd7a6 100644 --- 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 e7806be366..4692ece0c0 100644 --- 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 a20a80bb5e..3f1650b79c 100644 --- 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) })