feat(query): view query is properly updated when dom changes.

Fixes a bug in view manager util where sibling injector is not correctly
calculated.

ViewQuery no longer includes the view's initiating component injector.

Includes some refactoring of view methods and a removal of a polymorphic
map call.

Closes angular#3033

Closes angular#3439

Author: rkirov

modules/angular2/src/change_detection/interfaces.ts

@@ -3,6 +3,7 @@ import {CONST} from 'angular2/src/facade/lang';
 import {Locals} from './parser/locals';
 import {BindingRecord} from './binding_record';
 import {DirectiveIndex, DirectiveRecord} from './directive_record';
+import {ChangeDetectorRef} from './change_detector_ref';
 
 /**
  * Interface used by Angular to control the change detection strategy for an application.
@@ -50,6 +51,7 @@ export interface ChangeDispatcher {
 export interface ChangeDetector {
   parent: ChangeDetector;
   mode: string;
+  ref: ChangeDetectorRef;
 
   addChild(cd: ChangeDetector): void;
   addShadowDomChild(cd: ChangeDetector): void;

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

@@ -421,7 +421,7 @@ class _Context {
 
 export class ElementInjector extends TreeNode<ElementInjector> implements DependencyProvider {
   private _host: ElementInjector;
-  private _preBuiltObjects = null;
+  private _preBuiltObjects: PreBuiltObjects = null;
 
   // Queries are added during construction or linking with a new parent.
   // They are removed only through unlinking.
@@ -477,19 +477,35 @@ export class ElementInjector extends TreeNode<ElementInjector> implements Depend
     this._host = host;
     this._preBuiltObjects = preBuiltObjects;
 
-    this._reattachInjectors(imperativelyCreatedInjector);
-    this._strategy.hydrate();
-
     if (isPresent(host)) {
       this._addViewQueries(host);
     }
 
+    this._reattachInjectors(imperativelyCreatedInjector);
+    this._strategy.hydrate();
+
     this._addDirectivesToQueries();
     this._addVarBindingsToQueries();
 
+    // TODO(rado): optimize this call, if view queries are not moved around,
+    // simply appending to the query list is faster than updating.
+    this._updateViewQueries();
+
     this.hydrated = true;
   }
 
+  private _updateViewQueries() {
+    if (isPresent(this._query0) && this._query0.isViewQuery) {
+      this._query0.update();
+    }
+    if (isPresent(this._query1) && this._query1.isViewQuery) {
+      this._query1.update();
+    }
+    if (isPresent(this._query2) && this._query2.isViewQuery) {
+      this._query2.update();
+    }
+  }
+
   private _debugContext(): any {
     var p = this._preBuiltObjects;
     var index = p.elementRef.boundElementIndex - p.view.elementOffset;
@@ -651,22 +667,19 @@ export class ElementInjector extends TreeNode<ElementInjector> implements Depend
   }
 
   private _addViewQueries(host: ElementInjector): void {
-    if (isPresent(host._query0) && host._query0.originator == host &&
-        host._query0.query.isViewQuery)
-      this._addViewQuery(host._query0);
-    if (isPresent(host._query1) && host._query1.originator == host &&
-        host._query1.query.isViewQuery)
-      this._addViewQuery(host._query1);
-    if (isPresent(host._query2) && host._query2.originator == host &&
-        host._query2.query.isViewQuery)
-      this._addViewQuery(host._query2);
-  }
-
-  private _addViewQuery(queryRef: QueryRef): void {
-    // TODO(rado): Replace this.parent check with distanceToParent = 1 when
-    // https://github.com/angular/angular/issues/2707 is fixed.
-    if (!queryRef.query.descendants && isPresent(this.parent)) return;
-    this._assignQueryRef(queryRef);
+    this._addViewQuery(host._query0, host);
+    this._addViewQuery(host._query1, host);
+    this._addViewQuery(host._query2, host);
+  }
+
+  private _addViewQuery(queryRef: QueryRef, host: ElementInjector): void {
+    if (isBlank(queryRef) || !queryRef.isViewQuery || this._hasQuery(queryRef)) return;
+    if (host._query0.originator == host) {
+      // TODO(rado): Replace this.parent check with distanceToParent = 1 when
+      // https://github.com/angular/angular/issues/2707 is fixed.
+      if (!queryRef.query.descendants && isPresent(this.parent)) return;
+      this._assignQueryRef(queryRef);
+    }
   }
 
   private _addVarBindingsToQueries(): void {
@@ -694,6 +707,7 @@ export class ElementInjector extends TreeNode<ElementInjector> implements Depend
 
   private _addDirectivesToQuery(queryRef: QueryRef): void {
     if (isBlank(queryRef) || queryRef.query.isVarBindingQuery) return;
+    if (queryRef.isViewQuery && queryRef.originator == this) return;
 
     var matched = [];
     this.addDirectivesMatchingQuery(queryRef.query, matched);
@@ -754,42 +768,37 @@ export class ElementInjector extends TreeNode<ElementInjector> implements Depend
     this._addParentQueries();
   }
 
+  unlink(): void {
+    var parent = this.parent;
+    this.remove();
+    this._removeParentQueries(parent);
+  }
+
   private _addParentQueries(): void {
     if (isBlank(this.parent)) return;
-    if (isPresent(this.parent._query0) && !this.parent._query0.query.isViewQuery) {
-      this._addQueryToTree(this.parent._query0);
-      if (this.hydrated) this.parent._query0.update();
-    }
-    if (isPresent(this.parent._query1) && !this.parent._query1.query.isViewQuery) {
-      this._addQueryToTree(this.parent._query1);
-      if (this.hydrated) this.parent._query1.update();
-    }
-    if (isPresent(this.parent._query2) && !this.parent._query2.query.isViewQuery) {
-      this._addQueryToTree(this.parent._query2);
-      if (this.hydrated) this.parent._query2.update();
-    }
+    this._addParentQuery(this.parent._query0);
+    this._addParentQuery(this.parent._query1);
+    this._addParentQuery(this.parent._query2);
   }
 
-  unlink(): void {
-    var queriesToUpdate = [];
-    if (isPresent(this.parent._query0)) {
-      this._pruneQueryFromTree(this.parent._query0);
-      queriesToUpdate.push(this.parent._query0);
-    }
-    if (isPresent(this.parent._query1)) {
-      this._pruneQueryFromTree(this.parent._query1);
-      queriesToUpdate.push(this.parent._query1);
-    }
-    if (isPresent(this.parent._query2)) {
-      this._pruneQueryFromTree(this.parent._query2);
-      queriesToUpdate.push(this.parent._query2);
+  private _addParentQuery(query): void {
+    if (isPresent(query) && !this._hasQuery(query)) {
+      this._addQueryToTree(query);
+      if (this.hydrated) query.update();
     }
+  }
 
-    this.remove();
-    // TODO(rado): update should work on view queries too, however currently it
-    // is not implemented, so we filter to non-view queries.
-    var nonViewQueries = ListWrapper.filter(queriesToUpdate, (q) => !q.query.isViewQuery);
-    ListWrapper.forEach(nonViewQueries, (q) => q.update());
+  private _removeParentQueries(parent: ElementInjector): void {
+    this._removeParentQuery(parent._query0);
+    this._removeParentQuery(parent._query1);
+    this._removeParentQuery(parent._query2);
+  }
+
+  private _removeParentQuery(query: QueryRef) {
+    if (isPresent(query)) {
+      this._pruneQueryFromTree(query);
+      query.update();
+    }
   }
 
   private _pruneQueryFromTree(query: QueryRef): void {
@@ -852,6 +861,12 @@ export class ElementInjector extends TreeNode<ElementInjector> implements Depend
   getHost(): ElementInjector { return this._host; }
 
   getBoundElementIndex(): number { return this._proto.index; }
+
+  getRootViewInjectors(): ElementInjector[] {
+    var view = this._preBuiltObjects.view;
+    return view.getNestedView(view.elementOffset + this.getBoundElementIndex())
+        .rootElementInjectors;
+  }
 }
 
 interface _ElementInjectorStrategy {
@@ -1133,9 +1148,19 @@ export class QueryRef {
   constructor(public query: Query, public list: QueryList<any>,
               public originator: ElementInjector) {}
 
+  get isViewQuery(): boolean { return this.query.isViewQuery; }
+
   update(): void {
     var aggregator = [];
-    this.visit(this.originator, aggregator);
+    if (this.query.isViewQuery) {
+      // intentionally skipping originator for view queries.
+      var rootViewInjectors = this.originator.getRootViewInjectors();
+      for (var i = 0; i < rootViewInjectors.length; i++) {
+        this.visit(rootViewInjectors[i], aggregator);
+      }
+    } else {
+      this.visit(this.originator, aggregator);
+    }
     this.list.reset(aggregator);
   }
 

modules/angular2/src/core/compiler/view_manager_utils.ts

@@ -120,13 +120,14 @@ export class AppViewManagerUtils {
       parentView.viewContainers[boundElementIndex] = viewContainer;
     }
     ListWrapper.insert(viewContainer.views, atIndex, view);
+    var elementInjector = contextView.elementInjectors[contextBoundElementIndex];
+
     var sibling;
     if (atIndex == 0) {
-      sibling = null;
+      sibling = elementInjector;
     } else {
       sibling = ListWrapper.last(viewContainer.views[atIndex - 1].rootElementInjectors);
     }
-    var elementInjector = contextView.elementInjectors[contextBoundElementIndex];
     for (var i = view.rootElementInjectors.length - 1; i >= 0; i--) {
       if (isPresent(elementInjector.parent)) {
         view.rootElementInjectors[i].linkAfter(elementInjector.parent, sibling);

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

@@ -355,7 +355,7 @@ export function main() {
 
                  view.detectChanges();
 
-                 expect(q.query.map((d: TextDirective) => d.text)).toEqual(["self", "1", "2", "3"]);
+                 expect(q.query.map((d: TextDirective) => d.text)).toEqual(["1", "2", "3"]);
 
                  async.done();
                });
@@ -407,26 +407,29 @@ export function main() {
                });
          }));
 
-      /* TODO(rado): fix and reenable.
 
       it('should maintain directives in pre-order depth-first DOM order after dynamic insertion',
-        inject([TestComponentBuilder, AsyncTestCompleter], (tcb:TestComponentBuilder, async) => {
-          var template = '<needs-view-query-order #q text="self"></needs-view-query-order>';
+         inject([TestComponentBuilder, AsyncTestCompleter], (tcb: TestComponentBuilder, async) => {
+           var template = '<needs-view-query-order #q></needs-view-query-order>';
+
+           tcb.overrideTemplate(MyComp, template)
+               .createAsync(MyComp)
+               .then((view) => {
+                 var q: NeedsViewQueryOrder = view.componentViewChildren[0].getLocal("q");
 
-          tcb.overrideTemplate(MyComp, template)
-            .createAsync(MyComp)
-            .then((view) => {
-              var q:NeedsViewQueryOrder = view.componentViewChildren[0].getLocal("q");
+                 view.detectChanges();
 
-              view.detectChanges();
+                 expect(q.query.map((d: TextDirective) => d.text)).toEqual(["1", "2", "3", "4"]);
+
+                 q.list = ["-3", "2"];
+                 view.detectChanges();
 
-              expect(q.query.length).toBe(4);
-              expect(q.query.first.text).toEqual("1");
-              expect(q.query.first.text).toEqual("4");
 
-              async.done();
-            });
-        }));*/
+                 expect(q.query.map((d: TextDirective) => d.text)).toEqual(["1", "-3", "2", "4"]);
+
+                 async.done();
+               });
+         }));
     });
   });
 }
@@ -551,17 +554,23 @@ class NeedsViewQueryNestedIf {
 }
 
 
+// TODO(rado): once https://github.com/angular/angular/issues/3438 is resolved
+// duplicate the test without InertDirective.
 @Component({selector: 'needs-view-query-order'})
 @View({
-  directives: [NgFor, TextDirective],
-  template: '<div text="1">' +
-                '<div *ng-for="var i of [\'2\', \'3\']" [text]="i"></div>' +
-                '<div text="4">'
+  directives: [NgFor, TextDirective, InertDirective],
+  template: '<div dir><div text="1"></div>' +
+                '<div *ng-for="var i of list" [text]="i"></div>' +
+                '<div text="4"></div></div>'
 })
 @Injectable()
 class NeedsViewQueryOrder {
   query: QueryList<TextDirective>;
-  constructor(@ViewQuery(TextDirective) query: QueryList<TextDirective>) { this.query = query; }
+  list: string[];
+  constructor(@ViewQuery(TextDirective, {descendants: true}) query: QueryList<TextDirective>) {
+    this.query = query;
+    this.list = ['2', '3'];
+  }
 }
 
 @Component({selector: 'needs-tpl'})

modules/angular2/test/core/compiler/view_manager_utils_spec.ts

@@ -171,7 +171,8 @@ export function main() {
            createViews(2);
            utils.attachViewInContainer(parentView, 0, contextView, 1, 0, childView);
            expect(childView.rootElementInjectors[0].spy('linkAfter'))
-               .toHaveBeenCalledWith(contextView.elementInjectors[0], null);
+               .toHaveBeenCalledWith(contextView.elementInjectors[0],
+                                     contextView.elementInjectors[1]);
          });
     });