jQuery.cleanData is used for removed elements to clean up after them. Since in jQuery 3 we attach the data directly to elements, it doesn't seem necessary to delete user data associated with them, does it? Once you lose reference to the node, the data is garbage collected as well.

This is an observable difference, though, so we might want to document it anyway.

Was there a reason to remove private data but not user data?

It isn't necessary for any memory/GC issues (unless someone is keeping removed elements around) but I think if $.cleanData([e]) is called then $.hasData(e) should be false. Currently it is also different in master vs compat/v2.

(I just updated the tests to also do a hasData check)

Was there a reason to remove private data but not user data?

I can't think of one. It does seem odd that we're doing one but not the other.

Was there a reason to remove private data but not user data?

I can't think of one. It does seem odd that we're doing one but not the other.

One potential reason is that user data never leaks and event handlers may register event data on other elements (like in the focusin case) so it has to be cleaned to not leak.

That said, we might want to clean user data as well for consistency with older versions. I don't have a strong opinion here.

We definitely need to call the teardowns for special events, which we are doing, but I think that should still work even if we don't delete the data afterwards.

that should still work even if we don't delete the data afterwards.

That's true but if you then re-attached the element to the DOM, effects could be... interesting. ;)

We're always in the process of letting elements go when they get to this point, but yeah if you pull them out of the trash who knows if you can still eat them or not? 😸

Originally d702b76 stopped clearing both user and private data and only cleared events. Not clearing private data caused #2127 so clearing private data was added back but user data was not. What if something/someone depends on user data similar to the tests depending on private data?

IMO just the consistency with 1.x/2.x/compat and avoiding doc changes is worth the extra code though.

Thanks @jbedard for finding all that info with links! The initial idea was that jQuery.cleanData is not really needed once we attach data directly to elements. Unfortunately, it turned out to be false for private data and I feel convinced by your argumentation that leaving user data there might have similar issues so I'd be fine for clearing both as it was done previously.

cc @rwaldron

@jbedard Could you create an issue and then reference it in the commit message as "Fixes gh-ISSUE_NUMBER"? We try to separate issues & PRs.

Otherwise, this LGTM.

👍

Created #2503 and updated the commit message, and fixed the typos.

@jbedard Landed, thanks! I backported the tests to compat as well but with a slight change as on compat the value for the expando property is set to undefined, not deleted. This discrepancy should be removed when #2479 lands.