fix(query): view query should not be updated when subviews are attached.

rkirov · rkirov · commit 34acef58e7c4 · 2015-07-29T22:33:15.000Z
diff --git a/modules/angular2/src/core/compiler/element_injector.ts b/modules/angular2/src/core/compiler/element_injector.ts
@@ -789,8 +789,10 @@ export class ElementInjector extends TreeNode<ElementInjector> implements Depend
     }
 
     this.remove();
-
-    ListWrapper.forEach(queriesToUpdate, (q) => q.update());
+    // 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 _pruneQueryFromTree(query: QueryRef): void {
diff --git a/modules/angular2/test/core/compiler/query_integration_spec.ts b/modules/angular2/test/core/compiler/query_integration_spec.ts
@@ -365,6 +365,30 @@ export function main() {
                });
          }));
 
+      it('should not be affected by other changes in the component',
+         inject([TestComponentBuilder, AsyncTestCompleter], (tcb: TestComponentBuilder, async) => {
+           var template = '<needs-view-query-nested-if #q></needs-view-query-nested-if>';
+
+           tcb.overrideTemplate(MyComp, template)
+               .createAsync(MyComp)
+               .then((view) => {
+                 var q: NeedsViewQueryNestedIf = view.componentViewChildren[0].getLocal("q");
+
+                 view.detectChanges();
+
+                 expect(q.query.length).toEqual(1);
+                 expect(q.query.first.text).toEqual("1");
+
+                 q.show = false;
+                 view.detectChanges();
+
+                 expect(q.query.length).toEqual(1);
+                 expect(q.query.first.text).toEqual("1");
+
+                 async.done();
+               });
+         }));
+
       /* TODO(rado): fix and reenable.
 
       it('should maintain directives in pre-order depth-first DOM order after dynamic insertion',
@@ -396,6 +420,12 @@ class TextDirective {
   constructor() {}
 }
 
+@Directive({selector: '[dir]'})
+@Injectable()
+class InertDirective {
+  constructor() {}
+}
+
 @Component({selector: 'needs-query'})
 @View({
   directives: [NgFor, TextDirective],
@@ -487,6 +517,22 @@ class NeedsViewQueryIf {
 }
 
 
+@Component({selector: 'needs-view-query-nested-if'})
+@View({
+  directives: [NgIf, InertDirective, TextDirective],
+  template: '<div text="1"><div *ng-if="show"><div dir></div></div></div>'
+})
+@Injectable()
+class NeedsViewQueryNestedIf {
+  show: boolean;
+  query: QueryList<TextDirective>;
+  constructor(@ViewQuery(TextDirective) query: QueryList<TextDirective>) {
+    this.query = query;
+    this.show = true;
+  }
+}
+
+
 @Component({selector: 'needs-view-query-order'})
 @View({
   directives: [NgFor, TextDirective],
@@ -511,8 +557,10 @@ class NeedsViewQueryOrder {
     NeedsViewQuery,
     NeedsViewQueryDesc,
     NeedsViewQueryIf,
+    NeedsViewQueryNestedIf,
     NeedsViewQueryOrder,
     TextDirective,
+    InertDirective,
     NgIf,
     NgFor
   ]

Original file line number	Diff line number	Diff line change
`@@ -789,8 +789,10 @@ export class ElementInjector extends TreeNode<ElementInjector> implements Depend`
`789`	`789`	`}`
`790`	`790`
`791`	`791`	`this.remove();`
`792`		`-`
`793`		`- ListWrapper.forEach(queriesToUpdate, (q) => q.update());`
	`792`	`+ // TODO(rado): update should work on view queries too, however currently it`
	`793`	`+ // is not implemented, so we filter to non-view queries.`
	`794`	`+ var nonViewQueries = ListWrapper.filter(queriesToUpdate, (q) => !q.query.isViewQuery);`
	`795`	`+ ListWrapper.forEach(nonViewQueries, (q) => q.update());`
`794`	`796`	`}`
`795`	`797`
`796`	`798`	`private _pruneQueryFromTree(query: QueryRef): void {`