From cced1aecc419c1c54d7ff99bbb3423b6f979aff0 Mon Sep 17 00:00:00 2001 From: danilsomsikov Date: Fri, 11 Jan 2013 18:21:13 +0100 Subject: [PATCH] fix(ngSwitch): not leak when transcluded and cloned The leak can occur when ngSwich is used inside ngRepeat or any other directive which does not attached transcluded content to DOM but clones it. Refactor ngSwitch to use controller instead of storing data on compile node. Closes #1621 --- src/ng/directive/ngSwitch.js | 65 ++++++++++++++++--------------- test/ng/directive/ngSwitchSpec.js | 12 ++++++ 2 files changed, 45 insertions(+), 32 deletions(-) diff --git a/src/ng/directive/ngSwitch.js b/src/ng/directive/ngSwitch.js index 0c9e2a8db26c..4d54bf1fb72f 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, - compile: function(element, attrs, transclude) { - var cases = element.inheritedData(NG_SWITCH); - assertArg(cases); - cases['!' + attrs.ngSwitchWhen] = transclude; + require: '^ngSwitch', + compile: function(element, attrs, transclude, ctrl) { + return function(scope, element, attr, ctrl) { + ctrl.cases['!' + attrs.ngSwitchWhen] = transclude; + }; } }); var ngSwitchDefaultDirective = ngDirective({ transclude: 'element', priority: 500, - compile: function(element, attrs, transclude) { - var cases = element.inheritedData(NG_SWITCH); - assertArg(cases); - cases['?'] = transclude; + require: '^ngSwitch', + compile: function(element, attrs, transclude, ctrl) { + 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..ccd065b67ae5 100644 --- a/test/ng/directive/ngSwitchSpec.js +++ b/test/ng/directive/ngSwitchSpec.js @@ -90,4 +90,16 @@ describe('ngSwitch', function() { expect(child2).toBeDefined(); expect(child2).not.toBe(child1); })); + + + it('should not leak when comiled node is not attached to DOM but cloned', + inject(function($rootScope, $compile) { + element = $compile( + '
' + + '' + + '
{{name}}
' + + '
' + + '
')($rootScope); + $rootScope.$apply(); + })); });