From 89303fd2dccb780bea594b16b8ec85d9f3bd8f92 Mon Sep 17 00:00:00 2001 From: danilsomsikov Date: Fri, 11 Jan 2013 18:21:13 +0100 Subject: [PATCH] fix(ngSwitch): don't leak when destroyed while not attached 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 #1621 --- src/ng/directive/ngSwitch.js | 61 ++++++++++++++++--------------- test/ng/directive/ngSwitchSpec.js | 15 ++++++++ 2 files changed, 46 insertions(+), 30 deletions(-) diff --git a/src/ng/directive/ngSwitch.js b/src/ng/directive/ngSwitch.js index 0c9e2a8db26c..24c6047ac809 100644 --- 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 index 66ae5562d651..ee91f79d8667 100644 --- 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( + '
' + + '' + + '
{{name}}
' + + '
' + + '
')($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 + })); });