JavaScript Instances registry, faulty re-set new instance

GeoSot commented 1 year ago

Prerequisites

[X] I have searched for duplicate or closed issues
[X] I have validated any HTML to avoid common problems
[X] 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 https://github.com/twbs/bootstrap/blob/da0fe8c8b2e969bcf620a9ef1c1ca4f2b8554ec6/js/src/dom/data.js#L15-L31

Steps to reproduce

open debugger with given breakpoint on https://github.com/twbs/bootstrap/blob/da0fe8c8b2e969bcf620a9ef1c1ca4f2b8554ec6/js/src/dom/data.js#L30
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 commented 1 year ago

@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)

https://github.com/twbs/bootstrap/blob/5975ca65c552184889df25b8cd1c62e3a58ad621/js/tests/unit/dom/data.spec.js#L56-L66

tobiasberge commented 1 year ago

Hi everyone, 👋

is this by chance also related to this topic? https://github.com/twbs/bootstrap/issues/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 commented 1 year ago

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

GeoSot commented 1 year ago

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 commented 1 year ago

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

twbs / bootstrap