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

Commit a26234f

Browse files
danilsomsikovIgorMinar
authored andcommitted
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
1 parent 6131521 commit a26234f

File tree

2 files changed

+46
-30
lines changed

2 files changed

+46
-30
lines changed

src/ng/directive/ngSwitch.js

+31-30
Original file line numberDiff line numberDiff line change
@@ -62,51 +62,52 @@
6262
var NG_SWITCH = 'ng-switch';
6363
var ngSwitchDirective = valueFn({
6464
restrict: 'EA',
65-
compile: function(element, attr) {
65+
require: 'ngSwitch',
66+
controller: function ngSwitchController() {
67+
this.cases = {};
68+
},
69+
link: function(scope, element, attr, ctrl) {
6670
var watchExpr = attr.ngSwitch || attr.on,
67-
cases = {};
71+
selectedTransclude,
72+
selectedElement,
73+
selectedScope;
6874

69-
element.data(NG_SWITCH, cases);
70-
return function(scope, element){
71-
var selectedTransclude,
72-
selectedElement,
73-
selectedScope;
74-
75-
scope.$watch(watchExpr, function ngSwitchWatchAction(value) {
76-
if (selectedElement) {
77-
selectedScope.$destroy();
78-
selectedElement.remove();
79-
selectedElement = selectedScope = null;
80-
}
81-
if ((selectedTransclude = cases['!' + value] || cases['?'])) {
82-
scope.$eval(attr.change);
83-
selectedScope = scope.$new();
84-
selectedTransclude(selectedScope, function(caseElement) {
85-
selectedElement = caseElement;
86-
element.append(caseElement);
87-
});
88-
}
89-
});
90-
};
75+
scope.$watch(watchExpr, function ngSwitchWatchAction(value) {
76+
if (selectedElement) {
77+
selectedScope.$destroy();
78+
selectedElement.remove();
79+
selectedElement = selectedScope = null;
80+
}
81+
if ((selectedTransclude = ctrl.cases['!' + value] || ctrl.cases['?'])) {
82+
scope.$eval(attr.change);
83+
selectedScope = scope.$new();
84+
selectedTransclude(selectedScope, function(caseElement) {
85+
selectedElement = caseElement;
86+
element.append(caseElement);
87+
});
88+
}
89+
});
9190
}
9291
});
9392

9493
var ngSwitchWhenDirective = ngDirective({
9594
transclude: 'element',
9695
priority: 500,
96+
require: '^ngSwitch',
9797
compile: function(element, attrs, transclude) {
98-
var cases = element.inheritedData(NG_SWITCH);
99-
assertArg(cases);
100-
cases['!' + attrs.ngSwitchWhen] = transclude;
98+
return function(scope, element, attr, ctrl) {
99+
ctrl.cases['!' + attrs.ngSwitchWhen] = transclude;
100+
};
101101
}
102102
});
103103

104104
var ngSwitchDefaultDirective = ngDirective({
105105
transclude: 'element',
106106
priority: 500,
107+
require: '^ngSwitch',
107108
compile: function(element, attrs, transclude) {
108-
var cases = element.inheritedData(NG_SWITCH);
109-
assertArg(cases);
110-
cases['?'] = transclude;
109+
return function(scope, element, attr, ctrl) {
110+
ctrl.cases['?'] = transclude;
111+
};
111112
}
112113
});

test/ng/directive/ngSwitchSpec.js

+15
Original file line numberDiff line numberDiff line change
@@ -90,4 +90,19 @@ describe('ngSwitch', function() {
9090
expect(child2).toBeDefined();
9191
expect(child2).not.toBe(child1);
9292
}));
93+
94+
95+
it('should not leak jq data when compiled but not attached to parent when parent is destroyed',
96+
inject(function($rootScope, $compile) {
97+
element = $compile(
98+
'<div ng-repeat="i in []">' +
99+
'<ng-switch on="url">' +
100+
'<div ng-switch-when="a">{{name}}</div>' +
101+
'</ng-switch>' +
102+
'</div>')($rootScope);
103+
$rootScope.$apply();
104+
105+
// element now contains only empty repeater. this element is dealocated by local afterEach.
106+
// afterwards a global afterEach will check for leaks in jq data cache object
107+
}));
93108
});

0 commit comments

Comments
 (0)