fix($rootScope): allow destroying a root scope

[vojtajina]

When running (i.e. bootstrapping and killing) multiple apps on the same page,
it makes sense to destroy the root scope.

cc @aaronfrost

@IgorMinar @petebacondarwin What was the reason for ignoring $destroy on a root scope? Am I missing something here?

[petebacondarwin]

OMG!! A Pull Request from @vojtajina !!

[petebacondarwin]

I thought that we had fixed this already...

[petebacondarwin]

Looking through the history, it seems like we have just "never" detroyed the root scope and so that convention has followed on without question.

I guess that in the past it was not expected that one would expect to kill an Angular app on a page. In that case, what is the deal with killing other singleton services like $injector?

[lgalfaso]

We never supported shutting down an angular app. There are several PRs that
add this functionality with different solutions. BTW, if only a new
injector for some modules is needed, then this is possible without creating
a new app, this solution is used in several third party modules

[lgalfaso]

Another question, a specific use case would help understand the why of this
change

[aaronfrost]

@geddski wrote an angular layer called Overmind. Overmind affords two things to developers. First, it allows you to lazy-load pieces of your app. The second thing is affords you is an easy way to create multiple apps on the same page. Each app has it's own rootScope, meaning that the digest cycle for any given app doesn't affect the other apps on the page. This can improve performance.

This is the use case. If you need more specifics, I can add them.

[lgalfaso]

Ok, but destroying the root scope will not stop the timed tasks nor release
any of the resources. What is not clear is how does this helps you

[petebacondarwin]

I guess that this isn't going to break anything and it seems that @vojtajina, @aaronfrost and probably @geddski feel it would benefit their use case. Unless there are any strong objections we should merge this PR this week.

[shahata]

Looks very similar to #7564 which was closed without merging :) I think we should definitely merge this. I think DOM listeners are lower priority since I imagine that both in overmind and angular-widget the DOM is destroyed along with the micro-app. As you can see in https://github.com/wix/angular-widget/blob/master/app/scripts/services/widgets.js#L93-L96 it was also required for me to unset $$ChildScope in order to fix the memory leak, but I don't remember why...

[petebacondarwin]

But @IgorMinar felt that #8005 was a more complete solution. This may be so, but I think we should be OK for now with this. What do you think @lgalfaso ?

[lgalfaso]

It is not only the listeners at the DOM, but everybody that called $browser.addPollFn and $interval

[shahata]

$browser.addPollFn will be removed soon in the ngCookies PR.

[petebacondarwin]

@lgalfaso - from looking at https://github.com/geddski/overmind/blob/master/src/overmind.js I think you are right that more would need to be done when killing a "sub" app than just removing listeners.

I am thinking two things:

• this PR is not sufficient to clean up an app properly but it does take a step in that direction without breaking anything else
• we should consider providing an "app destroy" feature that really does close down everything in the current app so that people could remove it from the browser without reloading.

What do you think?

[geddski]

Spot on Pete. That would be great.

[lgalfaso]

SGTM

[petebacondarwin]

So, let's merge this and @geddski when you are not busy running a business and organising conferences might you like to look into a PR for the other stuff?

[geddski]

@petebacondarwin really what is needed is the cleanup counterpart to #11015. Not sure what all needs to go into that, probably will depend on the lazy loading implementation.

[petebacondarwin]

I just looked at merging this. It breaks a unit test but I think that it is not really testing the right thing anyway. I'll update and create a new PR.