fix(ngSwitch): don't leak when destroyed while not attached

danilsomsikov · IgorMinar · commit 89303fd2dccb · 2013-01-18T00:03:42.000-08:00
The leak can occur when ngSwich is used inside ngRepeat or any other directive which is destroyed while its transcluded content (which includes ngSwitch) is not attached to the DOM. Refactor ngSwitch to use controller instead of storing data on compile node. This means that we don't need to clean up the jq data cache. Controller reference is released when the linking fn is released. Closes angular#1621
diff --git a/src/ng/directive/ngSwitch.js b/src/ng/directive/ngSwitch.js
@@ -62,51 +62,52 @@
 var NG_SWITCH = 'ng-switch';
 var ngSwitchDirective = valueFn({
   restrict: 'EA',
-  compile: function(element, attr) {
+  require: 'ngSwitch',
+  controller: function ngSwitchController() {
+    this.cases = {};
+  },
+  link: function(scope, element, attr, ctrl) {
     var watchExpr = attr.ngSwitch || attr.on,
-        cases = {};
+        selectedTransclude,
+        selectedElement,
+        selectedScope;
 
-    element.data(NG_SWITCH, cases);
-    return function(scope, element){
-      var selectedTransclude,
-          selectedElement,
-          selectedScope;
-
-      scope.$watch(watchExpr, function ngSwitchWatchAction(value) {
-        if (selectedElement) {
-          selectedScope.$destroy();
-          selectedElement.remove();
-          selectedElement = selectedScope = null;
-        }
-        if ((selectedTransclude = cases['!' + value] || cases['?'])) {
-          scope.$eval(attr.change);
-          selectedScope = scope.$new();
-          selectedTransclude(selectedScope, function(caseElement) {
-            selectedElement = caseElement;
-            element.append(caseElement);
-          });
-        }
-      });
-    };
+    scope.$watch(watchExpr, function ngSwitchWatchAction(value) {
+      if (selectedElement) {
+        selectedScope.$destroy();
+        selectedElement.remove();
+        selectedElement = selectedScope = null;
+      }
+      if ((selectedTransclude = ctrl.cases['!' + value] || ctrl.cases['?'])) {
+        scope.$eval(attr.change);
+        selectedScope = scope.$new();
+        selectedTransclude(selectedScope, function(caseElement) {
+          selectedElement = caseElement;
+          element.append(caseElement);
+        });
+      }
+    });
   }
 });
 
 var ngSwitchWhenDirective = ngDirective({
   transclude: 'element',
   priority: 500,
+  require: '^ngSwitch',
   compile: function(element, attrs, transclude) {
-    var cases = element.inheritedData(NG_SWITCH);
-    assertArg(cases);
-    cases['!' + attrs.ngSwitchWhen] = transclude;
+    return function(scope, element, attr, ctrl) {
+      ctrl.cases['!' + attrs.ngSwitchWhen] = transclude;
+    };
   }
 });
 
 var ngSwitchDefaultDirective = ngDirective({
   transclude: 'element',
   priority: 500,
+  require: '^ngSwitch',
   compile: function(element, attrs, transclude) {
-    var cases = element.inheritedData(NG_SWITCH);
-    assertArg(cases);
-    cases['?'] = transclude;
+    return function(scope, element, attr, ctrl) {
+      ctrl.cases['?'] = transclude;
+    };
   }
 });
diff --git a/test/ng/directive/ngSwitchSpec.js b/test/ng/directive/ngSwitchSpec.js
@@ -90,4 +90,19 @@ describe('ngSwitch', function() {
     expect(child2).toBeDefined();
     expect(child2).not.toBe(child1);
   }));
+
+
+  it('should not leak jq data when compiled but not attached to parent when parent is destroyed',
+      inject(function($rootScope, $compile) {
+    element = $compile(
+      '<div ng-repeat="i in []">' +
+        '<ng-switch on="url">' +
+          '<div ng-switch-when="a">{{name}}</div>' +
+        '</ng-switch>' +
+      '</div>')($rootScope);
+    $rootScope.$apply();
+
+    // element now contains only empty repeater. this element is dealocated by local afterEach.
+    // afterwards a global afterEach will check for leaks in jq data cache object
+  }));
 });
