fix(core): detach projected views when a parent view is destroyed (an…

…gular#16592)

This also clarifies via a test 
that we only update projected views when the view is created or destroyed,
but not when it is attached/detached/moved.

Fixes angular#15578

PR Close angular#16592

packages/core/src/view/types.ts

@@ -329,9 +329,10 @@ export const enum ViewState {
   FirstCheck = 1 << 1,
   Attached = 1 << 2,
   ChecksEnabled = 1 << 3,
-  CheckProjectedView = 1 << 4,
-  CheckProjectedViews = 1 << 5,
-  Destroyed = 1 << 6,
+  IsProjectedView = 1 << 4,
+  CheckProjectedView = 1 << 5,
+  CheckProjectedViews = 1 << 6,
+  Destroyed = 1 << 7,
 
   CatDetectChanges = Attached | ChecksEnabled,
   CatInit = BeforeFirstCheck | CatDetectChanges

packages/core/src/view/view_attach.ts

@@ -18,26 +18,38 @@ export function attachEmbeddedView(
   }
   view.viewContainerParent = parentView;
   addToArray(embeddedViews, viewIndex !, view);
-  const dvcElementData = declaredViewContainer(view);
-  if (dvcElementData && dvcElementData !== elementData) {
-    let projectedViews = dvcElementData.template._projectedViews;
-    if (!projectedViews) {
-      projectedViews = dvcElementData.template._projectedViews = [];
-    }
-    projectedViews.push(view);
-    if (view.parent) {
-      // Note: we are changing the NodeDef here as we cannot calculate
-      // the fact whether a template is used for projection during compilation.
-      markNodeAsProjectedTemplate(view.parent.def, view.parentNodeDef !);
-    }
-  }
+  attachProjectedView(elementData, view);
 
   Services.dirtyParentQueries(view);
 
   const prevView = viewIndex ! > 0 ? embeddedViews[viewIndex ! - 1] : null;
   renderAttachEmbeddedView(elementData, prevView, view);
 }
 
+function attachProjectedView(vcElementData: ElementData, view: ViewData) {
+  const dvcElementData = declaredViewContainer(view);
+  if (!dvcElementData || dvcElementData === vcElementData ||
+      view.state & ViewState.IsProjectedView) {
+    return;
+  }
+  // Note: For performance reasons, we
+  // - add a view to template._projectedViews only 1x throughout its lifetime,
+  //   and remove it not until the view is destroyed.
+  //   (hard, as when a parent view is attached/detached we would need to attach/detach all
+  //    nested projected views as well, even accross component boundaries).
+  // - don't track the insertion order of views in the projected views array
+  //   (hard, as when the views of the same template are inserted different view containers)
+  view.state |= ViewState.IsProjectedView;
+  let projectedViews = dvcElementData.template._projectedViews;
+  if (!projectedViews) {
+    projectedViews = dvcElementData.template._projectedViews = [];
+  }
+  projectedViews.push(view);
+  // Note: we are changing the NodeDef here as we cannot calculate
+  // the fact whether a template is used for projection during compilation.
+  markNodeAsProjectedTemplate(view.parent !.def, view.parentNodeDef !);
+}
+
 function markNodeAsProjectedTemplate(viewDef: ViewDefinition, nodeDef: NodeDef) {
   if (nodeDef.flags & NodeFlags.ProjectedTemplate) {
     return;
@@ -63,19 +75,28 @@ export function detachEmbeddedView(elementData: ElementData, viewIndex?: number)
   view.viewContainerParent = null;
   removeFromArray(embeddedViews, viewIndex);
 
-  const dvcElementData = declaredViewContainer(view);
-  if (dvcElementData && dvcElementData !== elementData) {
-    const projectedViews = dvcElementData.template._projectedViews;
-    removeFromArray(projectedViews, projectedViews.indexOf(view));
-  }
-
+  // See attachProjectedView for why we don't update projectedViews here.
   Services.dirtyParentQueries(view);
 
   renderDetachView(view);
 
   return view;
 }
 
+export function detachProjectedView(view: ViewData) {
+  if (!(view.state & ViewState.IsProjectedView)) {
+    return;
+  }
+  const dvcElementData = declaredViewContainer(view);
+  if (dvcElementData) {
+    const projectedViews = dvcElementData.template._projectedViews;
+    if (projectedViews) {
+      removeFromArray(projectedViews, projectedViews.indexOf(view));
+      Services.dirtyParentQueries(view);
+    }
+  }
+}
+
 export function moveEmbeddedView(
     elementData: ElementData, oldViewIndex: number, newViewIndex: number): ViewData {
   const embeddedViews = elementData.viewContainer !._embeddedViews;

packages/core/test/linker/query_integration_spec.ts

@@ -536,6 +536,52 @@ export function main() {
         expect(q.query.length).toBe(0);
       });
 
+      // Note: This tests is just document our current behavior, which we do
+      // for performance reasons.
+      it('should not affected queries for projected templates if views are detached or moved', () => {
+        const template =
+            '<manual-projecting #q><ng-template let-x="x"><div [text]="x"></div></ng-template></manual-projecting>';
+        const view = createTestCmpAndDetectChanges(MyComp0, template);
+        const q = view.debugElement.children[0].references !['q'] as ManualProjecting;
+        expect(q.query.length).toBe(0);
+
+        const view1 = q.vc.createEmbeddedView(q.template, {'x': '1'});
+        const view2 = q.vc.createEmbeddedView(q.template, {'x': '2'});
+        view.detectChanges();
+        expect(q.query.map((d: TextDirective) => d.text)).toEqual(['1', '2']);
+
+        q.vc.detach(1);
+        q.vc.detach(0);
+
+        view.detectChanges();
+        expect(q.query.map((d: TextDirective) => d.text)).toEqual(['1', '2']);
+
+        q.vc.insert(view2);
+        q.vc.insert(view1);
+
+        view.detectChanges();
+        expect(q.query.map((d: TextDirective) => d.text)).toEqual(['1', '2']);
+      });
+
+      it('should remove manually projected templates if their parent view is destroyed', () => {
+        const template = `
+          <manual-projecting #q><ng-template #tpl><div text="1"></div></ng-template></manual-projecting>
+          <div *ngIf="shouldShow">
+            <ng-container [ngTemplateOutlet]="tpl"></ng-container>
+          </div>
+        `;
+        const view = createTestCmp(MyComp0, template);
+        const q = view.debugElement.children[0].references !['q'];
+        view.componentInstance.shouldShow = true;
+        view.detectChanges();
+
+        expect(q.query.length).toBe(1);
+
+        view.componentInstance.shouldShow = false;
+        view.detectChanges();
+        expect(q.query.length).toBe(0);
+      });
+
       it('should not throw if a content template is queried and created in the view during change detection',
          () => {
            @Component(