jQuery.cleanData traversal issue

[PSPvZ]

Description

We use a jQuery.event.special remove handler to write components that clean themselves up when they are removed from the DOM. We ran into an issue where certain components did not execute their cleanup. I've managed to trace it back to the fact that one of the components removed its own element from the DOM. This causes jQuery.cleanData to skip the next element in its list.

From what I can tell jQuery.cleanData iterates over a live list of elements. If one of the already traversed elements is removed from the DOM, all elements in the list behind it will move one index down. This causes the iteration to skip one element. For example, if it's cleaning up [0:A, 1:B, 2:C] and the cleanup of A removes itself, the list becomes [0:B, 1:C]; iteration continues at index 1, so B is skipped.

Link to test case

either https://jsfiddle.net/qy2tx8za/
or https://jsbin.com/yadekowafa/edit?html,css,js,console,output

There are three elements that have the remove handler. When removed, they should log the contents of their data-ext attribute. Elements with the removeself class will remove themselves from the DOM. The first element does this, causing cleanData to skip the second one, but still cleanup the third.

[timmywil]

We're going to look into switch the selector list to one that isn't live, i.e. with querySelectorAll

[AnuragMishraa]

One way to solve this problem is to create a copy of the live list of elements before iterating over it, so that changes to the DOM during cleanup don't affect the original list.
This way, even if an element removes itself from the DOM during cleanup, it won't affect the iteration of the original list.

Another solution is to reverse the order of iteration, so that you start with the last element in the list and work your way backwards. This way, if an element removes itself from the DOM, it won't affect the cleanup of any earlier elements.

Can you please assign me this issue, I am Interested to work on it.

[PSPvZ]

If you choose the "reverse order" solution, be mindful that in the [0:A, 1:B, 2:C] case, C might for some reason remove element A or B (I can't think of why anyone would do this, but it technically could happen). This would lead to C being cleaned up twice, once at position 2 and once at position 1. Even worse would be if C removed both A and B, because position 1 would end up out-of-bounds.

[AnuragMishraa]

Yes, that's a good point to keep in mind if you choose the "reverse order" solution. In general, it's always a good idea to be mindful of the potential side effects of DOM manipulation when iterating over a live list of elements, especially if you're modifying the list while iterating over it.

Creating a copy of the live list before iterating over it can be a more reliable solution, because it ensures that the iteration is based on a static snapshot of the list at the time of creation. This can help prevent unexpected behavior caused by changes to the DOM during cleanup.

[AnuragMishraa]

Alternatively, you could also consider using the MutationObserver API to detect changes to the DOM and trigger cleanup routines as needed. This can be a more robust solution for complex scenarios where multiple components are interacting with the DOM and may be manipulating it in different ways.

[mgol]

MutationObserver API would catch all the mutations, not just jQuery-managed ones.

The solution we agreed on during the meeting, as Timmy mentioned, is to get rid of getElementsByTagName from the getAll internal function and just leave querySelectorAll in there. qSA doesn't return a live collection as opposed to gEBTN so changes to the DOM will not influence it.

@AnuragMishraa would you be willing to implement such a change, together with adding a test for the issue reported here?

[srushti-learner]

@mgol Hello I am Srushti. Can I take up this issue I have understood the problem as well as the solution proposed by you above. I am new to open-source contributions, and I am looking for a good mentorship from this community. I do have experience in working with JavaScript, jQuery and git.

[mgol]

@srushti-learner sure, feel free to try! Let us know if you have questions.

[minami-akira]

Hi @mgol
Can you assign this issue to me?
I'm ready to start working on this issue

[mgol]

@minami-akira we don’t need to set a formal assignee, feel free to work on it & to submit a PR when ready.

Please also read https://contribute.jquery.org/commits-and-pull-requests/ for guidelines.