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
This commit is contained in:
		
							parent
							
								
									44a991e245
								
							
						
					
					
						commit
						01cdd31339
					
				| @ -433,6 +433,7 @@ export class ElementInjector extends TreeNode<ElementInjector> implements Depend | |||||||
|     this._preBuiltObjects = null; |     this._preBuiltObjects = null; | ||||||
|     this._strategy.callOnDestroy(); |     this._strategy.callOnDestroy(); | ||||||
|     this._strategy.dehydrate(); |     this._strategy.dehydrate(); | ||||||
|  |     this._clearQueryLists(); | ||||||
|   } |   } | ||||||
| 
 | 
 | ||||||
|   afterContentChecked(): void { |   afterContentChecked(): void { | ||||||
| @ -837,6 +838,12 @@ export class ElementInjector extends TreeNode<ElementInjector> implements Depend | |||||||
|     var nestedView = view.getNestedView(view.elementOffset + this.getBoundElementIndex()); |     var nestedView = view.getNestedView(view.elementOffset + this.getBoundElementIndex()); | ||||||
|     return isPresent(nestedView) ? nestedView.rootElementInjectors : []; |     return isPresent(nestedView) ? nestedView.rootElementInjectors : []; | ||||||
|   } |   } | ||||||
|  | 
 | ||||||
|  |   private _clearQueryLists(): void { | ||||||
|  |     if (isPresent(this._query0) && this._query0.originator === this) this._query0.reset(); | ||||||
|  |     if (isPresent(this._query1) && this._query1.originator === this) this._query1.reset(); | ||||||
|  |     if (isPresent(this._query2) && this._query2.originator === this) this._query2.reset(); | ||||||
|  |   } | ||||||
| } | } | ||||||
| 
 | 
 | ||||||
| interface _ElementInjectorStrategy { | interface _ElementInjectorStrategy { | ||||||
| @ -1163,4 +1170,9 @@ export class QueryRef { | |||||||
|   private _aggregateDirective(inj: ElementInjector, aggregator: any[]): void { |   private _aggregateDirective(inj: ElementInjector, aggregator: any[]): void { | ||||||
|     inj.addDirectivesMatchingQuery(this.query, aggregator); |     inj.addDirectivesMatchingQuery(this.query, aggregator); | ||||||
|   } |   } | ||||||
|  | 
 | ||||||
|  |   reset(): void { | ||||||
|  |     this.list.reset([]); | ||||||
|  |     this.list.removeAllCallbacks(); | ||||||
|  |   } | ||||||
| } | } | ||||||
|  | |||||||
| @ -89,13 +89,6 @@ class QueryList<T> extends Object | |||||||
|     _dirty = true; |     _dirty = true; | ||||||
|   } |   } | ||||||
| 
 | 
 | ||||||
|   // TODO(rado): hook up with change detection after #995. |  | ||||||
|   void fireCallbacks() { |  | ||||||
|     if (_dirty) { |  | ||||||
|       _callbacks.forEach((c) => c()); |  | ||||||
|       _dirty = false; |  | ||||||
|     } |  | ||||||
|   } |  | ||||||
| 
 | 
 | ||||||
|   void onChange(callback) { |   void onChange(callback) { | ||||||
|     _callbacks.add(callback); |     _callbacks.add(callback); | ||||||
| @ -105,6 +98,10 @@ class QueryList<T> extends Object | |||||||
|     _callbacks.remove(callback); |     _callbacks.remove(callback); | ||||||
|   } |   } | ||||||
| 
 | 
 | ||||||
|  |   void removeAllCallbacks() { | ||||||
|  |     this._callbacks = []; | ||||||
|  |   } | ||||||
|  | 
 | ||||||
|   int get length => _results.length; |   int get length => _results.length; | ||||||
|   T get first => _results.first; |   T get first => _results.first; | ||||||
|   T get last => _results.last; |   T get last => _results.last; | ||||||
| @ -116,4 +113,12 @@ class QueryList<T> extends Object | |||||||
|     // Note: we need to return a list instead of iterable to match JS. |     // Note: we need to return a list instead of iterable to match JS. | ||||||
|     return this._results.map(fn).toList(); |     return this._results.map(fn).toList(); | ||||||
|   } |   } | ||||||
|  | 
 | ||||||
|  |   // Internal to the framework. | ||||||
|  |   void fireCallbacks() { | ||||||
|  |     if (_dirty) { | ||||||
|  |       _callbacks.forEach((c) => c()); | ||||||
|  |       _dirty = false; | ||||||
|  |     } | ||||||
|  |   } | ||||||
| } | } | ||||||
|  | |||||||
| @ -86,17 +86,13 @@ export class QueryList<T> { | |||||||
|     this._dirty = true; |     this._dirty = true; | ||||||
|   } |   } | ||||||
| 
 | 
 | ||||||
|   fireCallbacks(): void { |  | ||||||
|     if (this._dirty) { |  | ||||||
|       ListWrapper.forEach(this._callbacks, (c) => c()); |  | ||||||
|       this._dirty = false; |  | ||||||
|     } |  | ||||||
|   } |  | ||||||
| 
 | 
 | ||||||
|   onChange(callback: () => void): void { this._callbacks.push(callback); } |   onChange(callback: () => void): void { this._callbacks.push(callback); } | ||||||
| 
 | 
 | ||||||
|   removeCallback(callback: () => void): void { ListWrapper.remove(this._callbacks, callback); } |   removeCallback(callback: () => void): void { ListWrapper.remove(this._callbacks, callback); } | ||||||
| 
 | 
 | ||||||
|  |   removeAllCallbacks(): void { this._callbacks = []; } | ||||||
|  | 
 | ||||||
|   toString(): string { return this._results.toString(); } |   toString(): string { return this._results.toString(); } | ||||||
| 
 | 
 | ||||||
|   get length(): number { return this._results.length; } |   get length(): number { return this._results.length; } | ||||||
| @ -106,4 +102,12 @@ export class QueryList<T> { | |||||||
|   map<U>(fn: (item: T) => U): U[] { return this._results.map(fn); } |   map<U>(fn: (item: T) => U): U[] { return this._results.map(fn); } | ||||||
| 
 | 
 | ||||||
|   [Symbol.iterator](): any { return this._results[Symbol.iterator](); } |   [Symbol.iterator](): any { return this._results[Symbol.iterator](); } | ||||||
|  | 
 | ||||||
|  |   // Internal to the framework.
 | ||||||
|  |   fireCallbacks(): void { | ||||||
|  |     if (this._dirty) { | ||||||
|  |       ListWrapper.forEach(this._callbacks, (c) => c()); | ||||||
|  |       this._dirty = false; | ||||||
|  |     } | ||||||
|  |   } | ||||||
| } | } | ||||||
|  | |||||||
| @ -212,6 +212,34 @@ export function main() { | |||||||
|                  view.detectChanges(); |                  view.detectChanges(); | ||||||
|                }); |                }); | ||||||
|          })); |          })); | ||||||
|  | 
 | ||||||
|  |       it('should correctly clean-up when destroyed together with the directives it is querying', | ||||||
|  |          inject([TestComponentBuilder, AsyncTestCompleter], (tcb: TestComponentBuilder, async) => { | ||||||
|  |            var template = | ||||||
|  |                '<needs-query #q *ng-if="shouldShow"><div text="foo"></div></needs-query>'; | ||||||
|  | 
 | ||||||
|  |            tcb.overrideTemplate(MyComp, template) | ||||||
|  |                .createAsync(MyComp) | ||||||
|  |                .then((view) => { | ||||||
|  |                  view.componentInstance.shouldShow = true; | ||||||
|  |                  view.detectChanges(); | ||||||
|  | 
 | ||||||
|  |                  var q: NeedsQuery = view.componentViewChildren[1].getLocal('q'); | ||||||
|  |                  expect(q.query.length).toEqual(1); | ||||||
|  | 
 | ||||||
|  |                  view.componentInstance.shouldShow = false; | ||||||
|  |                  view.detectChanges(); | ||||||
|  | 
 | ||||||
|  |                  view.componentInstance.shouldShow = true; | ||||||
|  |                  view.detectChanges(); | ||||||
|  | 
 | ||||||
|  |                  var q2: NeedsQuery = view.componentViewChildren[1].getLocal('q'); | ||||||
|  | 
 | ||||||
|  |                  expect(q2.query.length).toEqual(1); | ||||||
|  | 
 | ||||||
|  |                  async.done(); | ||||||
|  |                }); | ||||||
|  |          })); | ||||||
|     }); |     }); | ||||||
| 
 | 
 | ||||||
|     describe("querying by var binding", () => { |     describe("querying by var binding", () => { | ||||||
|  | |||||||
| @ -92,6 +92,19 @@ export function main() { | |||||||
|         queryList.fireCallbacks(); |         queryList.fireCallbacks(); | ||||||
|         expect(fires).toEqual(1); |         expect(fires).toEqual(1); | ||||||
|       }); |       }); | ||||||
|  | 
 | ||||||
|  |       it('should support removing all callbacks', () => { | ||||||
|  |         var fires = 0; | ||||||
|  |         var callback = () => fires += 1; | ||||||
|  |         queryList.onChange(callback); | ||||||
|  | 
 | ||||||
|  |         queryList.add('one'); | ||||||
|  |         queryList.removeAllCallbacks(); | ||||||
|  | 
 | ||||||
|  |         queryList.fireCallbacks(); | ||||||
|  | 
 | ||||||
|  |         expect(fires).toEqual(0); | ||||||
|  |       }); | ||||||
|     }); |     }); | ||||||
|   }); |   }); | ||||||
| } | } | ||||||
|  | |||||||
		Loading…
	
	
			
			x
			
			
		
	
		Reference in New Issue
	
	Block a user