Always Trigger The $destroy Event Before Removing Elements In AngularJS Directives
If you're building custom AngularJS directives, you will inevitably find yourself in a situation where you have to clone elements, create new scopes, remove elements, and destroy old scopes. When you do this, the order of operations is very important. At first, it may not seem like an issue; but, over time, if the order of operations is incorrect, it can lead to unexpected behaviors and memory leaks.
Run this demo in my JavaScript Demos project on GitHub.
Ultimately, the order of operations in your AngularJS directive is important because of the way jQuery implements the .remove() method. When you remove an element from the DOM using .remove() or .empty(), jQuery will clear out the event bindings and the data associated with the element so as to avoid memory leaks. This means that if you remove the element before you trigger the "$destroy" event, the element will be in a "sanitized state" by the time your $destroy event handler is executed.
To see this in action, I have created a demo that defines two "if" directives, modeled on the old ui-if directive in Angular-UI. The two directives differ only in the order in which they remove the transcluded element and destroy the associated child scope. For each transcluded element, I am also using a directive which sets "data" and then attempts to read that data in the "$destroy" event handler.
<!doctype html>
<html ng-app="Demo">
<head>
<meta charset="utf-8" />
<title>
Always Trigger The $destroy Event Before Removing Elements In AngularJS Directives
</title>
<style type="text/css">
a[ ng-click ] {
cursor: pointer ;
text-decoration: underline ;
}
</style>
</head>
<body ng-controller="AppController">
<h1>
Always Trigger The $destroy Event Before Removing Elements In AngularJS Directives
</h1>
<p>
<a ng-click="toggleContainer()">Toggle Container</a>
</p>
<p bn-bad-if="isShowingContainer">
<span bn-data-test="Bad">This is a bad directive test</span>
</p>
<p bn-good-if="isShowingContainer">
<span bn-data-test="Good">This is a good directive test</span>
</p>
<!-- Load scripts. -->
<script type="text/javascript" src="../../vendor/jquery/jquery-2.1.0.min.js"></script>
<script type="text/javascript" src="../../vendor/angularjs/angular-1.2.22.min.js"></script>
<script type="text/javascript">
// Create an application module for our demo.
var app = angular.module( "Demo", [] );
// -------------------------------------------------- //
// -------------------------------------------------- //
// I control the root of the application.
app.controller(
"AppController",
function( $scope ) {
$scope.isShowingContainer = false;
// ---
// PUBLIC METHODS.
// ---
// I show or hide the container, depending on its current state.
$scope.toggleContainer = function() {
$scope.isShowingContainer = ! $scope.isShowingContainer;
};
}
);
// -------------------------------------------------- //
// -------------------------------------------------- //
// I define jQuery data and then test to see if it's available in the $destroy
// event of the scope.
app.directive(
"bnDataTest",
function() {
// Bind the JavaScript behaviors to the local scope.
function link( scope, element, attributes ) {
element.data( "test", "jQuery data is available." );
// When the destroy event is triggered, check to see if the above
// data is still available.
scope.$on(
"$destroy",
function handleDestroyEvent() {
console.log( attributes.bnDataTest, ":", element.data( "test" ) );
}
);
}
// Return the directive configuration.
return({
link: link,
restrict: "A"
});
}
);
// -------------------------------------------------- //
// -------------------------------------------------- //
// I define a "bad" version of ng-if / ui-if.
// --
// NOTE: This is, more or less, a copy of the old ui-if directive.
app.directive(
"bnBadIf",
function() {
// Bind the JavaScript behaviors to the local scope.
function link( scope, element, attributes, _, transclude ) {
// I keep track of the currently-injected element and its defined
// scope. We need the injected element to have its own scope so we
// can destroy it when we remove the element.
var cloneElement = null;
var cloneScope = null;
// When the model changes, adjust the element existence.
scope.$watch(
attributes.bnBadIf,
function handleWatchChange( newValue, oldValue ) {
// If we have an existing item, remove it.
if ( cloneElement ) {
// ***************************************************
// NOTE: We are removing the element BEFORE we are
// destroying the scope associated with the element.
// ***************************************************
cloneElement.remove();
cloneElement = null;
cloneScope.$destroy();
cloneScope = null;
}
// If the new value is truthy, inject the element.
if ( newValue ) {
cloneScope = scope.$new();
cloneElement = transclude(
cloneScope,
function injectClonedElement( clone ) {
element.after( clone );
}
);
}
}
);
}
// Return the directive configuration.
return({
link: link,
priority: 1000,
restrict: "A",
terminal: true,
transclude: "element"
});
}
);
// -------------------------------------------------- //
// -------------------------------------------------- //
// I define a "good" version of ng-if / ui-if.
// --
// NOTE: This is, more or less, a copy of the old ui-if directive.
app.directive(
"bnGoodIf",
function() {
// Bind the JavaScript behaviors to the local scope.
function link( scope, element, attributes, _, transclude ) {
// I keep track of the currently-injected element and its defined
// scope. We need the injected element to have its own scope so we
// can destroy it when we remove the element.
var cloneElement = null;
var cloneScope = null;
// When the model changes, adjust the element existence.
scope.$watch(
attributes.bnGoodIf,
function handleWatchChange( newValue, oldValue ) {
// If we have an existing item, remove it.
if ( cloneElement ) {
// ***************************************************
// NOTE: We are removing the element AFTER we are
// destroying the scope associated with the element.
// ***************************************************
cloneScope.$destroy();
cloneScope = null;
cloneElement.remove();
cloneElement = null;
}
// If the new value is truthy, inject the element.
if ( newValue ) {
cloneScope = scope.$new();
cloneElement = transclude(
cloneScope,
function injectClonedElement( clone ) {
element.after( clone );
}
);
}
}
);
}
// Return the directive configuration.
return({
link: link,
priority: 1000,
restrict: "A",
terminal: true,
transclude: "element"
});
}
);
</script>
</body>
</html>
As you can see, the bnBadIf directive removes the element and then destroys the scope. The bnGoodIf directive, on the other hand, destroys the scope and then removes the element. When we run this code and toggle the container, we get the following console output:
Bad : undefined
Good : jQuery data is available.
As you can see, the bnBadIf loses access to the jQuery data in the context of the $destroy event handler.
Most of the time, you probably won't even notice that the order of operations matters. But, if you have a directive that applies a jQuery plugin to the DOM (Document Object Model), that's where things get interesting. Many jQuery plugins rely on the .data() being available. And, if you try to teardown your jQuery plugins in the $destroy event, the order of operations will either allow that to happen; or, it will lead to a slow and steady memory leak. As such, always make sure that you trigger the $destroy event before you actually remove the associated element from the DOM.
Want to use code from this post? Check out the license.
Reader Comments
Another very important point is to not always destroy scope. If you want to remove the element, you should not always destroy the scope. I have written directives that remove elements that cannot destroy the scope because it is using the existing scope. Example: http://jsfiddle.net/mslocum/cLoh0p91/
Also, normally you should not create a new scope inside your directive, but instead use the 'scope:' option when creating a directive. If you find yourself make a new scope in a directive you are probably doing something wrong, unless you really know what you are doing.
Good point though, if you have your own scope you should always destroy before remove.
@Matt & @Ben,
Interesting points by both of you. It would be interesting to see a variation using isolate scope.
Similar concerns exist with isolate scope, but as Matt says, if you destroy the scope using inherited scope then you are damaging that parent scope.
I've been working on some directive tips and this is one of the key events. What would be awesome is a profiling tool for angular (akin to the ng-hint work but more like a gulp/grunt plugin) that would scan your code for these types of things.
@Matt, @John,
I would think that you have to be very careful about explicitly unbinding watchers and event handlers if you are going to remove a UI component that is part of a shared scope. The beautiful thing about making sure you have a new scope associated with a UI element is that when you destroy it, all of the $watch() and $on() bindings are automatically detached from the application (since they stop receiving digests and events, respectively).
Theoretically, you could run $compile() on random UI elements and then link them to the currently-shared $scope. But, then you would need to manually clean-up any directives linked (if you remove the UI element) which sounds quite difficult - if it's even possible at all.
Since the beginning of AngularJS, people have always had a problem with the fact that certain directives (basically anything that transcludes content) creates a new Scope - generally people have a problem with how this interacts with ngModel and prototypal inheritance. But, creating a new scope is really the only way that it is possible to clean up state when the transcluded content is removed.
Ok, end of rant :D
If a parent view or directive is removed does it fire the $destroy event down through it's children or do you have to explicitly destroy them?
A different, but related great article about nested scopes by Gleb (spoke at ng-conf 2015): http://glebbahmutov.com/blog/separate-model-from-view-in-angular/
I think destroying at the wrong time and creating scope within a directive's link is due to not following best practices. Granted, there are always exceptions.
@Matt With the creating scope bit aside, I don't understand why you think destroying a scope is considered not following the best practices.
Sharing scopes between directives are always more problematic than just using an isolate scope. One example that you illustrated is that you end up polluting/damaging the parent scope. Another one is that it confuses the next developer who comes on.
Could you further explain why do you think that?
@Matt With the creating scope bit aside, I don't understand why you think destroying a scope is considered not following the best practices.
Sharing scopes between directives are always more problematic than just using an isolate scope. One example that you illustrated is that you end up polluting/damaging the parent scope. Another one is that it confuses the next developer who comes on.
Could you further explain why do you think that?
Sure @Douglas,
The problem here is that we are all thinking of different types of directives. There are many different types of directives. I can think of three styles off the top of my head. There are probably more.
1. Sometimes they are widgets (probably most common). These often have isolate scope or at least a child scope. The way to do this is in the directive definition by doing scope: {} (optional scope variables can be mapped here). Child scope is done with scope: true. There is no need to destroy scope here since this is done automatically by angular.
2. Sometimes they are DOM manipulators (ng-style, ng-show). These do not create a new scope at all. Instead they use the parent scope. I've created many of these that just add some visual change, but they are not a stand-alone widget and do not need their own scope because they work with whatever parent scope data exists. Adding a new scope here can cause problems because putting a new scope on ng-show would break child node ng-models off of the main controller scope (unless you are using controllerAs obviously). Another good example here would be a tooltip generator, that automatically adds tooltip DOM elements around an element. There would be no need for a new scope if all you are doing is adding some DOM.
3. Sometimes they are functionality manipulators (I need a better term for this one) (ng-click, ng-model, input, validators). These are designed to add functionality most often through events or binding steps (validation). These rarely add scope either.
ng-if is a great example of an exception to the rule. Because of the way it actually removes its own element from the DOM, it must create its own scope and destroy its own scope outside of the angular built-in way. Although, this seems to be mostly for efficiency, so it doesn't have to do full re-link and re-binding of all the elements inside the ng-if when the element gets shown again. Unless you are being fancy like angular's ng-if, then use the built-in scope creation from the directive definition.
I probably missed stuff. My definition of "best practice" stems from the fact that angular has a built-in mechanism, so why try and do it yourself and risk doing it wrong. I don't know what the actual angular team would say.
Not useful and not necessary anymore ;)
Keep forward on the edge with angular 2
sheesh this angular framework is yukky.
hi . how i can delete all directive by one click?