Datepicker Memory Leak

[porterclev]

Building off #2268, there is a problem when destroying the datepicker where it doesn’t completely get removed from memory. This pull request, https://github.com/jquery/jquery-ui/commit/817ce38, removes the instance reference but the actual UI reference is tied to

https://github.com/jquery/jquery-ui/blob/54f96eea31b21d9ecb00912261df3e5aaebf8cce/ui/widgets/datepicker.js#L163

When the widget is destroyed, the datepicker UI element remains in memory preventing it from closing. https://github.com/jquery/jquery-ui/pull/2268 patches this issue by hiding the component, however dpDiv is still in memory. One solution could be to remove dpDiv from the instance and then set both _curInst = null and this.dpDiv = null. https://github.com/jquery/jquery-ui/pull/2268/commits/5e4f1b86296c5a977abc7efaff671ea9004d4d37 However, many unit tests will break because they rely on dpDiv to still exist in memory, requiring the datepicker to be recreated for each test. There is also a problem that many developers rely on hiding the datepicker before destroying it $.datepicker("hide").destroy("destroy") which will fail because dpDiv will be null.

[mgol]

Thanks for the report.

PR https://github.com/jquery/jquery-ui/pull/1362 fixing Trac issue #10668 and PR https://github.com/jquery/jquery-ui/pull/1852 fixing Trac issue #15270 removed references to a Datepicker instance, which is way more than just the div representing the datepicker UI. Also, $.datepicker.dpDiv is created only once - and even if you load multiple versions of jQuery & jQuery UI on a page, the div is only attached to the document once. Thus, this issue is not fully comparable to the previous ones.

I think we'll just need to accept this dpDiv instance as an inherent part of Datepicker and its memory usage. There's no memory leak per se as creating multiple datepickers & destroying them will not build up memory from past destroyed date pickers. It's just maybe taking up a bit more memory that it could - but it's not even clear to me it's a bad thing as not having to recreate the datepicker container has its advantages as well.

One thing that could be considered is calling this.dpDiv.empty() when destroy is called on a date picker which was the most recently shown (or is still shown) using this global dpDiv, since we already empty it on showing here: https://github.com/jquery/jquery-ui/blob/54f96eea31b21d9ecb00912261df3e5aaebf8cce/ui/widgets/datepicker.js#L831-L832 so the previous contents would not be used anyway. We need to be careful here, though, to not delete contents meant for a different datepicker instance, especially when multiple jQuery UI versions are running on the same page.

I'd be willing to review a PR, but it might not be a trivial thing to do right.

(Since the issue is already in 1.12, given limited team resources it's not likely to be fixed by the UI team; see the project status at https://blog.jqueryui.com/2021/10/jquery-maintainers-update-and-transition-jquery-ui-as-part-of-overall-modernization-efforts/.)