JavaScript Instances registry, faulty re-set new instance

[GeoSot]

Prerequisites

• I have searched for duplicate or closed issues
• I have validated any HTML to avoid common problems
• I have read the contributing guidelines

Describe the issue

Bootstrap's Instances registry, by design, doesn't support multiple instances per element. However, in the rare case we are trying to re-initiate the same component for second time, it wrongly overrides the already existing instance

I found this flaw on #37195, where two tooltips were created using the same element

and is caused on this method

bootstrap/js/src/dom/data.js

Lines 15 to 31 in da0fe8c

	set(element, key, instance) {
	if (!elementMap.has(element)) {
	elementMap.set(element, new Map())
	}
	const instanceMap = elementMap.get(element)
	// 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
	}
	instanceMap.set(key, instance)
	},

Steps to reproduce

• open debugger with given breakpoint on

  bootstrap/js/src/dom/data.js

  Line 30 in da0fe8c

  instanceMap.set(key, instance)

• initialize an instance of any available js component, using constructor ex: new Toast(myelement)
• Debugger with stop on the breakpoint
• repeat the first step again -> new Toast(myelement)
• Debugger should not stop in the same point as the instance already exists

Reduced test cases

https://codepen.io/GeosSV/pen/gOzzWYV?editors=1011

What operating system(s) are you seeing the problem on?

Windows, macOS, Android, iOS, Linux

What browser(s) are you seeing the problem on?

Chrome, Safari, Firefox, Microsoft Edge, Opera

What version of Bootstrap are you using?

v5.2.2

[GeoSot]

@XhmikosR can you remember something on the following test, or explain it?
I am curious for what reason, we allow overwriting an element instance (as it is not disposed first)

bootstrap/js/tests/unit/dom/data.spec.js

Lines 56 to 66 in 5975ca6

	it('should overwrite data if something is already stored', () => {
	const data = { ...TEST_DATA }
	const copy = { ...data }
	Data.set(div, TEST_KEY, data)
	Data.set(div, TEST_KEY, copy)
	// Using `toBe` since spread creates a shallow copy
	expect(Data.get(div, TEST_KEY)).not.toBe(data)
	expect(Data.get(div, TEST_KEY)).toBe(copy)
	})

[tobiasberge]

Hi everyone, 👋

is this by chance also related to this topic? #35710

If yes I would postpone a workaround re-initializing Popover and Tooltip every time manually after dynamic content changes and use the "selector" option instead.

[WinterSilence]

also, old versions using jQuery, not throws error if selector not exists - need add "silent mode" to keep BC

[GeoSot]

Hey @tobiasberge, it is a bit different, and the tooltip after 5.1.3 (I am not sure of the version) re-initializes its self every time

[tobiasberge]

@GeoSot Thanks for your answer, good to know. We wanted to update to 5.2.x anyway. 👍