Skip to content
This repository was archived by the owner on Oct 2, 2019. It is now read-only.

Fix angular.element.cache memory leak #1365

Conversation

Teemu
Copy link
Contributor

@Teemu Teemu commented Dec 22, 2015

After doing memory leak hunting in my application I found that ui-select is leaking memory. I'm not sure why this is exactly needed as I would have expected $compile to handle it (I tried looking at uiSelectDirective.js with no results)

@Teemu Teemu force-pushed the fix-angular-element-cache-memory-leak branch from cf5442e to 811caaf Compare February 25, 2016 14:24
@wesleycho
Copy link
Contributor

This is a good catch - LGTM

@Teemu
Copy link
Contributor Author

Teemu commented Mar 29, 2016

Thanks! 👏

@roman-savitski
Copy link

roman-savitski commented Nov 16, 2016

Cause an issue if ui-select is being toggled on page by ng-if.
<div ng-if="expression"> <ui-select>.....</ui-select> </div>
Sometimes this cause choices element to be removed forever.

In this case "compile" called only once, but "link" called each time ng-if is triggered. And I suppose this is the real reason of memory leak, because "link" is binded to "compile" function closure. When this change was made var choices=... was defined inside "link", but not its defined in "compile".

I propose:

  • isolate "link" fn from "compile" scope return (function(groupByExp, groupFilterExp){ return function link(scope, element, attrs, $select){...} })(groupByExp, groupFilterExp);
  • replace tElement usage with element
  • replace choices.remove(); with element.querySelectorAll('.ui-select-choices-row').remove();

@user378230
Copy link
Contributor

@raman-savitski If you want to report a bug you should open a new issue here on github and follow the issue template. (You can obviously reference this PR in your issue).

You or someone else can then submit a PR to resolve it 👍

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants