Skip to content
This repository has been archived by the owner on Apr 12, 2024. It is now read-only.

ngInclude leaks if template contains comments #1620

Closed
suedama1756 opened this issue Nov 28, 2012 · 5 comments
Closed

ngInclude leaks if template contains comments #1620

suedama1756 opened this issue Nov 28, 2012 · 5 comments

Comments

@suedama1756
Copy link

The ngInclude directive leaks if your template contains comments. For some reason Angular seems to associate random scope objects with comments. When jQuery.cleanData is eventually called it performs a getElementsByName('*'). As comment nodes and text nodes are not included in this selector the scopes associated with the comments are not cleared. Also, due to scope prototype inheritance, all parent scopes are also leaked.

@IgorMinar
Copy link
Contributor

plnkr.co example would be helpful. any comment e.g. <!-- foo --> will trigger this leak? I assume that the leak occurs only if the src of the ngInclude changes and the view with the comment is destroyed.

@ghost ghost assigned IgorMinar Nov 28, 2012
@suedama1756
Copy link
Author

Yep, any comment seems to do it. The leak occurs whenever the content is torn down, e.g. src changing, inside nested ng-repeat, etc.

I'll try to put a standalone sample together if I get time.

@suedama1756
Copy link
Author

Right, I've slightly mislead you with my previous statements so I'll give you the full history.

We are currently using angular (with require) to dynamically load/unload widgets written in angular. Each widget is isolated as an Angular application. When un-installing our widgets we have come across 3 separate leaks.

  1. The browser object attaches to events on window. This means that when you tear down the application browser doesn't get cleaned up and so neither does your root scope.

If you look at the last comment on this issue #1537 you will find our rather hacky fix to this. Ideally I think subscriptions to document/window events should be managed by a service tied to the root scope, when the rootElement is destroyed that service could then unhook all events automatically.

Once you get around the browser issue, I think the other two issues are actually related. They both stem from the fact that the element supplied to compile is not necessarily the same as the element passed to link. This is due to the cloning that occurs during transclusion.

  1. The ng-include leak is demonstrated here http://plnkr.co/edit/zWJW5j?p=preview, the problem you will find however, is that this sample leaks because of issue 1 (browser). So you would have to first apply a patch to see this leak for what it really is.
  2. The final leak is logged here ngSwitch directive leaks when inside transcluded content #1621 This occurs when the switch is transcluded. Its easy to demonstrate, put a break point on the compile and link phases, you will see the element is different in between.

Hope that makes sense

@suedama1756
Copy link
Author

Any update on this issue? The initial response was quick, but its been over a month now....

@IgorMinar
Copy link
Contributor

I can't reproduce the ng-include leak as you describe. This is what I tried: http://plnkr.co/edit/l0M31vYlZCm5VC6vRChk?p=preview

plain old comments should not leak any DOM nodes. we however use comments to mark a place in the dom was fully transcluded (like ngRepeat or ngSwitch), so that's likely what you've seen. the comments in this case are just a symptom and not the cause.

you are right about the ngSwitch leak - that is a valid one, but I don't see how without ngSwitch ngInclude leaks any memory.

lastly #1537 does offer have some good suggestions that we should implement.

I'm going to close this issue and fix ngSwitch. if you can repro the leak without global destruction of the app or without ngSwitch then please reopen it and provide repro instructions.

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

No branches or pull requests

2 participants