Skip to content
This repository has been archived by the owner on Apr 12, 2024. It is now read-only.

Commit

Permalink
fix(ngSwitch): don't leak when destroyed while not attached
Browse files Browse the repository at this point in the history
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
  • Loading branch information
danilsomsikov authored and IgorMinar committed Jan 18, 2013
1 parent 6131521 commit a26234f
Show file tree
Hide file tree
Showing 2 changed files with 46 additions and 30 deletions.
61 changes: 31 additions & 30 deletions src/ng/directive/ngSwitch.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
};
}
});
15 changes: 15 additions & 0 deletions test/ng/directive/ngSwitchSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
}));
});

0 comments on commit a26234f

Please sign in to comment.