patrick-steele-idem / morphdom

Fast and lightweight DOM diffing/patching (no virtual DOM needed)
MIT License
3.21k stars 129 forks source link

Does not eliminate duplicate nodes with same id when deleted #171

Open neurodynamic opened 4 years ago

neurodynamic commented 4 years ago

The library seems to get confused by elements with the same id attribute. I realize having more than one element with the same id is bad practice, but it can easily happen temporarily in the process of editing HTML, and very strange behavior results.

Example: if I create several p elements with an id of hi, running morphdom after each is created, and then I delete some of those same p elements from the HTML that is being passed to morphdom, the corresponding elements are not removed, even if all of the p tags are removed.

This same example behaves exactly as would normally be expected if the p tags do not have id attributes.

Example gif: morphdom_bug

snewcomer commented 4 years ago

@neurodynamic I linked a PR that adds a test for us to review. You are right. It doesn't remove another element with the same id. I'm dig into the traversal mechanisms and see if it is possible to get this test passing.

snewcomer commented 4 years ago

In this PR, I added a few tests to show various behaviours with duplicate ids. There is one test that shows the duplicate id is kept. I know the reason why. In order to allow duplicate ids, we would need to reverse the way we store ids ({ 'id': element }) and take advantage of a WeakMap data structure. I'll have to test out some of these ideas because likely this would result in a major version bump.

https://github.com/patrick-steele-idem/morphdom/pull/173