X Tutup
Skip to content

Commit 01cdd31

Browse files
rkirovrkirov
committed
fix(query): clean-up queryref during dehydration
The QueryRef objects persists during dehydration but needs to be cleaned-up by removing callbacks and previous elements. Closes #3944 Closes #3948
1 parent 44a991e commit 01cdd31

File tree

5 files changed

+75
-13
lines changed

5 files changed

+75
-13
lines changed

‎modules/angular2/src/core/compiler/element_injector.ts‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -433,6 +433,7 @@ export class ElementInjector extends TreeNode<ElementInjector> implements Depend
433433
this._preBuiltObjects = null;
434434
this._strategy.callOnDestroy();
435435
this._strategy.dehydrate();
436+
this._clearQueryLists();
436437
}
437438

438439
afterContentChecked(): void {
@@ -837,6 +838,12 @@ export class ElementInjector extends TreeNode<ElementInjector> implements Depend
837838
var nestedView = view.getNestedView(view.elementOffset + this.getBoundElementIndex());
838839
return isPresent(nestedView) ? nestedView.rootElementInjectors : [];
839840
}
841+
842+
private _clearQueryLists(): void {
843+
if (isPresent(this._query0) && this._query0.originator === this) this._query0.reset();
844+
if (isPresent(this._query1) && this._query1.originator === this) this._query1.reset();
845+
if (isPresent(this._query2) && this._query2.originator === this) this._query2.reset();
846+
}
840847
}
841848

842849
interface _ElementInjectorStrategy {
@@ -1163,4 +1170,9 @@ export class QueryRef {
11631170
private _aggregateDirective(inj: ElementInjector, aggregator: any[]): void {
11641171
inj.addDirectivesMatchingQuery(this.query, aggregator);
11651172
}
1173+
1174+
reset(): void {
1175+
this.list.reset([]);
1176+
this.list.removeAllCallbacks();
1177+
}
11661178
}

‎modules/angular2/src/core/compiler/query_list.dart‎

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -89,13 +89,6 @@ class QueryList<T> extends Object
8989
_dirty = true;
9090
}
9191

92-
// TODO(rado): hook up with change detection after #995.
93-
void fireCallbacks() {
94-
if (_dirty) {
95-
_callbacks.forEach((c) => c());
96-
_dirty = false;
97-
}
98-
}
9992

10093
void onChange(callback) {
10194
_callbacks.add(callback);
@@ -105,6 +98,10 @@ class QueryList<T> extends Object
10598
_callbacks.remove(callback);
10699
}
107100

101+
void removeAllCallbacks() {
102+
this._callbacks = [];
103+
}
104+
108105
int get length => _results.length;
109106
T get first => _results.first;
110107
T get last => _results.last;
@@ -116,4 +113,12 @@ class QueryList<T> extends Object
116113
// Note: we need to return a list instead of iterable to match JS.
117114
return this._results.map(fn).toList();
118115
}
116+
117+
// Internal to the framework.
118+
void fireCallbacks() {
119+
if (_dirty) {
120+
_callbacks.forEach((c) => c());
121+
_dirty = false;
122+
}
123+
}
119124
}

‎modules/angular2/src/core/compiler/query_list.ts‎

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -86,17 +86,13 @@ export class QueryList<T> {
8686
this._dirty = true;
8787
}
8888

89-
fireCallbacks(): void {
90-
if (this._dirty) {
91-
ListWrapper.forEach(this._callbacks, (c) => c());
92-
this._dirty = false;
93-
}
94-
}
9589

9690
onChange(callback: () => void): void { this._callbacks.push(callback); }
9791

9892
removeCallback(callback: () => void): void { ListWrapper.remove(this._callbacks, callback); }
9993

94+
removeAllCallbacks(): void { this._callbacks = []; }
95+
10096
toString(): string { return this._results.toString(); }
10197

10298
get length(): number { return this._results.length; }
@@ -106,4 +102,12 @@ export class QueryList<T> {
106102
map<U>(fn: (item: T) => U): U[] { return this._results.map(fn); }
107103

108104
[Symbol.iterator](): any { return this._results[Symbol.iterator](); }
105+
106+
// Internal to the framework.
107+
fireCallbacks(): void {
108+
if (this._dirty) {
109+
ListWrapper.forEach(this._callbacks, (c) => c());
110+
this._dirty = false;
111+
}
112+
}
109113
}

‎modules/angular2/test/core/compiler/query_integration_spec.ts‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -212,6 +212,34 @@ export function main() {
212212
view.detectChanges();
213213
});
214214
}));
215+
216+
it('should correctly clean-up when destroyed together with the directives it is querying',
217+
inject([TestComponentBuilder, AsyncTestCompleter], (tcb: TestComponentBuilder, async) => {
218+
var template =
219+
'<needs-query #q *ng-if="shouldShow"><div text="foo"></div></needs-query>';
220+
221+
tcb.overrideTemplate(MyComp, template)
222+
.createAsync(MyComp)
223+
.then((view) => {
224+
view.componentInstance.shouldShow = true;
225+
view.detectChanges();
226+
227+
var q: NeedsQuery = view.componentViewChildren[1].getLocal('q');
228+
expect(q.query.length).toEqual(1);
229+
230+
view.componentInstance.shouldShow = false;
231+
view.detectChanges();
232+
233+
view.componentInstance.shouldShow = true;
234+
view.detectChanges();
235+
236+
var q2: NeedsQuery = view.componentViewChildren[1].getLocal('q');
237+
238+
expect(q2.query.length).toEqual(1);
239+
240+
async.done();
241+
});
242+
}));
215243
});
216244

217245
describe("querying by var binding", () => {

‎modules/angular2/test/core/compiler/query_list_spec.ts‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,19 @@ export function main() {
9292
queryList.fireCallbacks();
9393
expect(fires).toEqual(1);
9494
});
95+
96+
it('should support removing all callbacks', () => {
97+
var fires = 0;
98+
var callback = () => fires += 1;
99+
queryList.onChange(callback);
100+
101+
queryList.add('one');
102+
queryList.removeAllCallbacks();
103+
104+
queryList.fireCallbacks();
105+
106+
expect(fires).toEqual(0);
107+
});
95108
});
96109
});
97110
}

0 commit comments

Comments
 (0)
X Tutup