fix(ivy): avoid destroy renderer method invocation for child views (#27592)

Since Renderer is shared across root and child views, we need to avoid `destroy` method invocation for child views and only invoke is for root view when needed. Prior to this change, the `destroy` function was called whenever child view was destroyed, thus causing errors at runtime.

PR Close #27592

packages/core/src/render3/node_manipulation.ts

@@ -465,8 +465,9 @@ function cleanUpView(viewOrContainer: LView | LContainer): void {
     removeListeners(view);
     executeOnDestroys(view);
     executePipeOnDestroys(view);
+    const hostTNode = view[HOST_NODE];
     // For component views only, the local renderer is destroyed as clean up time.
-    if (view[TVIEW].id === -1 && isProceduralRenderer(view[RENDERER])) {
+    if (hostTNode && hostTNode.type === TNodeType.Element && isProceduralRenderer(view[RENDERER])) {
       ngDevMode && ngDevMode.rendererDestroy++;
       (view[RENDERER] as ProceduralRenderer3).destroy();
     }

packages/core/test/render3/component_spec.ts

@@ -14,7 +14,7 @@ import {ComponentDef, RenderFlags} from '../../src/render3/interfaces/definition
 
 import {NgIf} from './common_with_def';
 import {getRendererFactory2} from './imported_renderer2';
-import {ComponentFixture, containerEl, createComponent, renderComponent, renderToHtml, requestAnimationFrame, toHtml} from './render_util';
+import {ComponentFixture, MockRendererFactory, containerEl, createComponent, renderComponent, renderToHtml, requestAnimationFrame, toHtml} from './render_util';
 
 describe('component', () => {
   class CounterComponent {
@@ -152,6 +152,65 @@ describe('component', () => {
 
 });
 
+it('should not invoke renderer destroy method for embedded views', () => {
+  let comp: Comp;
+
+  function MyComponent_div_Template_2(rf: any, ctx: any) {
+    if (rf & RenderFlags.Create) {
+      elementStart(0, 'div');
+      text(1, 'Child view');
+      elementEnd();
+    }
+  }
+
+  class Comp {
+    visible = true;
+
+    static ngComponentDef = defineComponent({
+      type: Comp,
+      selectors: [['comp']],
+      consts: 3,
+      vars: 1,
+      factory: () => {
+        comp = new Comp();
+        return comp;
+      },
+      directives: [NgIf],
+      /**
+       *  <div>Root view</div>
+       *  <div *ngIf="visible">Child view</div>
+       */
+      template: function(rf: RenderFlags, ctx: Comp) {
+        if (rf & RenderFlags.Create) {
+          elementStart(0, 'div');
+          text(1, 'Root view');
+          elementEnd();
+          template(2, MyComponent_div_Template_2, 2, 0, null, [1, 'ngIf']);
+        }
+        if (rf & RenderFlags.Update) {
+          elementProperty(2, 'ngIf', bind(ctx.visible));
+        }
+      }
+    });
+  }
+
+  const rendererFactory = new MockRendererFactory(['destroy']);
+  const fixture = new ComponentFixture(Comp, {rendererFactory});
+
+  comp !.visible = false;
+  fixture.update();
+
+  comp !.visible = true;
+  fixture.update();
+
+  const renderer = rendererFactory.lastRenderer !;
+  const destroySpy = renderer.spies['destroy'];
+
+  // we should never see `destroy` method being called
+  // in case child views are created/removed
+  expect(destroySpy.calls.count()).toBe(0);
+});
+
 describe('component with a container', () => {
 
   function showItems(rf: RenderFlags, ctx: {items: string[]}) {

packages/core/test/render3/integration_spec.ts

@@ -7,12 +7,12 @@
  */
 
 import {ElementRef, TemplateRef, ViewContainerRef} from '@angular/core';
-import {RendererStyleFlags2, RendererType2} from '../../src/render/api';
+import {RendererType2} from '../../src/render/api';
 import {AttributeMarker, defineComponent, defineDirective, templateRefExtractor} from '../../src/render3/index';
 
 import {allocHostVars, bind, container, containerRefreshEnd, containerRefreshStart, element, elementAttribute, elementClassProp, elementContainerEnd, elementContainerStart, elementEnd, elementProperty, elementStart, elementStyleProp, elementStyling, elementStylingApply, embeddedViewEnd, embeddedViewStart, interpolation1, interpolation2, interpolation3, interpolation4, interpolation5, interpolation6, interpolation7, interpolation8, interpolationV, load, projection, projectionDef, reference, text, textBinding, template, elementStylingMap, directiveInject} from '../../src/render3/instructions';
 import {InitialStylingFlags, RenderFlags} from '../../src/render3/interfaces/definition';
-import {RElement, Renderer3, RendererFactory3, domRendererFactory3, RText, RComment, RNode, RendererStyleFlags3, ProceduralRenderer3} from '../../src/render3/interfaces/renderer';
+import {RElement, Renderer3, RendererFactory3, domRendererFactory3} from '../../src/render3/interfaces/renderer';
 import {NO_CHANGE} from '../../src/render3/tokens';
 import {HEADER_OFFSET, CONTEXT} from '../../src/render3/interfaces/view';
 import {enableBindings, disableBindings} from '../../src/render3/state';
@@ -20,7 +20,7 @@ import {sanitizeUrl} from '../../src/sanitization/sanitization';
 import {Sanitizer, SecurityContext} from '../../src/sanitization/security';
 
 import {NgIf} from './common_with_def';
-import {ComponentFixture, TemplateFixture, createComponent, renderToHtml} from './render_util';
+import {ComponentFixture, MockRendererFactory, TemplateFixture, createComponent, renderToHtml} from './render_util';
 import {getLContext} from '../../src/render3/context_discovery';
 import {StylingIndex} from '../../src/render3/interfaces/styling';
 import {MONKEY_PATCH_KEY_NAME} from '../../src/render3/interfaces/context';
@@ -2652,58 +2652,4 @@ class ProxyRenderer3Factory implements RendererFactory3 {
     this.lastCapturedType = rendererType;
     return domRendererFactory3.createRenderer(hostElement, rendererType);
   }
-}
+}
-
-class MockRendererFactory implements RendererFactory3 {
-  lastRenderer: any;
-  private _spyOnMethods: string[];
-
-  constructor(spyOnMethods?: string[]) { this._spyOnMethods = spyOnMethods || []; }
-
-  createRenderer(hostElement: RElement|null, rendererType: RendererType2|null): Renderer3 {
-    const renderer = this.lastRenderer = new MockRenderer(this._spyOnMethods);
-    return renderer;
-  }
-}
-
-class MockRenderer implements ProceduralRenderer3 {
-  public spies: {[methodName: string]: any} = {};
-
-  constructor(spyOnMethods: string[]) {
-    spyOnMethods.forEach(methodName => {
-      this.spies[methodName] = spyOn(this as any, methodName).and.callThrough();
-    });
-  }
-
-  destroy(): void {}
-  createComment(value: string): RComment { return document.createComment(value); }
-  createElement(name: string, namespace?: string|null): RElement {
-    return document.createElement(name);
-  }
-  createText(value: string): RText { return document.createTextNode(value); }
-  appendChild(parent: RElement, newChild: RNode): void { parent.appendChild(newChild); }
-  insertBefore(parent: RNode, newChild: RNode, refChild: RNode|null): void {
-    parent.insertBefore(newChild, refChild, false);
-  }
-  removeChild(parent: RElement, oldChild: RNode): void { parent.removeChild(oldChild); }
-  selectRootElement(selectorOrNode: string|any): RElement {
-    return ({} as any);
-  }
-  parentNode(node: RNode): RElement|null { return node.parentNode as RElement; }
-  nextSibling(node: RNode): RNode|null { return node.nextSibling; }
-  setAttribute(el: RElement, name: string, value: string, namespace?: string|null): void {}
-  removeAttribute(el: RElement, name: string, namespace?: string|null): void {}
-  addClass(el: RElement, name: string): void {}
-  removeClass(el: RElement, name: string): void {}
-  setStyle(
-      el: RElement, style: string, value: any,
-      flags?: RendererStyleFlags2|RendererStyleFlags3): void {}
-  removeStyle(el: RElement, style: string, flags?: RendererStyleFlags2|RendererStyleFlags3): void {}
-  setProperty(el: RElement, name: string, value: any): void {}
-  setValue(node: RText, value: string): void {}
-
-  // TODO(misko): Deprecate in favor of addEventListener/removeEventListener
-  listen(target: RNode, eventName: string, callback: (event: any) => boolean | void): () => void {
-    return () => {};
-  }
-}

packages/core/test/render3/render_util.ts

@@ -19,7 +19,7 @@ import {Injector, SWITCH_INJECTOR_FACTORY__POST_R3__ as R3_INJECTOR_FACTORY} fro
 import {SWITCH_ELEMENT_REF_FACTORY__POST_R3__ as R3_ELEMENT_REF_FACTORY} from '../../src/linker/element_ref';
 import {SWITCH_TEMPLATE_REF_FACTORY__POST_R3__ as R3_TEMPLATE_REF_FACTORY} from '../../src/linker/template_ref';
 import {SWITCH_VIEW_CONTAINER_REF_FACTORY__POST_R3__ as R3_VIEW_CONTAINER_REF_FACTORY} from '../../src/linker/view_container_ref';
-import {SWITCH_RENDERER2_FACTORY__POST_R3__ as R3_RENDERER2_FACTORY} from '../../src/render/api';
+import {RendererStyleFlags2, RendererType2, SWITCH_RENDERER2_FACTORY__POST_R3__ as R3_RENDERER2_FACTORY} from '../../src/render/api';
 import {CreateComponentOptions} from '../../src/render3/component';
 import {getDirectivesAtNodeIndex, getLContext, isComponentInstance} from '../../src/render3/context_discovery';
 import {extractDirectiveDef, extractPipeDef} from '../../src/render3/definition';
@@ -28,7 +28,7 @@ import {ComponentTemplate, ComponentType, DirectiveDef, DirectiveType, RenderFla
 import {renderTemplate} from '../../src/render3/instructions';
 import {DirectiveDefList, DirectiveTypesOrFactory, PipeDef, PipeDefList, PipeTypesOrFactory} from '../../src/render3/interfaces/definition';
 import {PlayerHandler} from '../../src/render3/interfaces/player';
-import {RElement, RText, Renderer3, RendererFactory3, domRendererFactory3} from '../../src/render3/interfaces/renderer';
+import {ProceduralRenderer3, RComment, RElement, RNode, RText, Renderer3, RendererFactory3, RendererStyleFlags3, domRendererFactory3} from '../../src/render3/interfaces/renderer';
 import {HEADER_OFFSET, LView} from '../../src/render3/interfaces/view';
 import {Sanitizer} from '../../src/sanitization/security';
 import {Type} from '../../src/type';
@@ -354,3 +354,64 @@ export function enableIvyInjectableFactories() {
   (Renderer2 as any)[NG_ELEMENT_ID] = () => R3_RENDERER2_FACTORY();
   (Injector as any)[NG_ELEMENT_ID] = () => R3_INJECTOR_FACTORY();
 }
+
+export class MockRendererFactory implements RendererFactory3 {
+  lastRenderer: any;
+  private _spyOnMethods: string[];
+
+  constructor(spyOnMethods?: string[]) { this._spyOnMethods = spyOnMethods || []; }
+
+  createRenderer(hostElement: RElement|null, rendererType: RendererType2|null): Renderer3 {
+    const renderer = this.lastRenderer = new MockRenderer(this._spyOnMethods);
+    return renderer;
+  }
+}
+
+class MockRenderer implements ProceduralRenderer3 {
+  public spies: {[methodName: string]: any} = {};
+
+  constructor(spyOnMethods: string[]) {
+    spyOnMethods.forEach(methodName => {
+      this.spies[methodName] = spyOn(this as any, methodName).and.callThrough();
+    });
+  }
+
+  destroy(): void {}
+  createComment(value: string): RComment { return document.createComment(value); }
+  createElement(name: string, namespace?: string|null): RElement {
+    return document.createElement(name);
+  }
+  createText(value: string): RText { return document.createTextNode(value); }
+  appendChild(parent: RElement, newChild: RNode): void { parent.appendChild(newChild); }
+  insertBefore(parent: RNode, newChild: RNode, refChild: RNode|null): void {
+    parent.insertBefore(newChild, refChild, false);
+  }
+  removeChild(parent: RElement, oldChild: RNode): void { parent.removeChild(oldChild); }
+  selectRootElement(selectorOrNode: string|any): RElement {
+    return ({} as any);
+  }
+  parentNode(node: RNode): RElement|null { return node.parentNode as RElement; }
+  nextSibling(node: RNode): RNode|null { return node.nextSibling; }
+  setAttribute(el: RElement, name: string, value: string, namespace?: string|null): void {
+    // set all synthetic attributes as properties
+    if (name[0] === '@') {
+      this.setProperty(el, name, value);
+    } else {
+      el.setAttribute(name, value);
+    }
+  }
+  removeAttribute(el: RElement, name: string, namespace?: string|null): void {}
+  addClass(el: RElement, name: string): void {}
+  removeClass(el: RElement, name: string): void {}
+  setStyle(
+      el: RElement, style: string, value: any,
+      flags?: RendererStyleFlags2|RendererStyleFlags3): void {}
+  removeStyle(el: RElement, style: string, flags?: RendererStyleFlags2|RendererStyleFlags3): void {}
+  setProperty(el: RElement, name: string, value: any): void { (el as any)[name] = value; }
+  setValue(node: RText, value: string): void { node.textContent = value; }
+
+  // TODO(misko): Deprecate in favor of addEventListener/removeEventListener
+  listen(target: RNode, eventName: string, callback: (event: any) => boolean | void): () => void {
+    return () => {};
+  }
+}

packages/platform-browser/animations/test/animation_renderer_spec.ts

@@ -195,77 +195,75 @@ import {el} from '../../testing/src/browser_util';
            });
          });
 
-      fixmeIvy(
+      it('should only queue up dom removals if the element itself contains a valid leave animation',
-          `FW-801: Components with animations throw with "Cannot read property 'hostElement' of undefined" error`)
+         () => {
-          .it('should only queue up dom removals if the element itself contains a valid leave animation',
+           @Component({
-              () => {
+             selector: 'my-cmp',
-                @Component({
+             template: `
-                  selector: 'my-cmp',
-                  template: `
                <div #elm1 *ngIf="exp1"></div>
                <div #elm2 @animation1 *ngIf="exp2"></div>
                <div #elm3 @animation2 *ngIf="exp3"></div>
             `,
-                  animations: [
+             animations: [
-                    trigger('animation1', [transition('a => b', [])]),
+               trigger('animation1', [transition('a => b', [])]),
-                    trigger('animation2', [transition(':leave', [])]),
+               trigger('animation2', [transition(':leave', [])]),
-                  ]
+             ]
-                })
+           })
-                class Cmp {
+           class Cmp {
-                  exp1: any = true;
+             exp1: any = true;
-                  exp2: any = true;
+             exp2: any = true;
-                  exp3: any = true;
+             exp3: any = true;
 
-                  @ViewChild('elm1') public elm1: any;
+             @ViewChild('elm1') public elm1: any;
 
-                  @ViewChild('elm2') public elm2: any;
+             @ViewChild('elm2') public elm2: any;
 
-                  @ViewChild('elm3') public elm3: any;
+             @ViewChild('elm3') public elm3: any;
-                }
+           }
 
-                TestBed.configureTestingModule({
+           TestBed.configureTestingModule({
-                  providers: [{provide: AnimationEngine, useClass: InjectableAnimationEngine}],
+             providers: [{provide: AnimationEngine, useClass: InjectableAnimationEngine}],
-                  declarations: [Cmp]
+             declarations: [Cmp]
-                });
+           });
 
-                const engine = TestBed.get(AnimationEngine);
+           const engine = TestBed.get(AnimationEngine);
-                const fixture = TestBed.createComponent(Cmp);
+           const fixture = TestBed.createComponent(Cmp);
-                const cmp = fixture.componentInstance;
+           const cmp = fixture.componentInstance;
 
-                fixture.detectChanges();
+           fixture.detectChanges();
-                const elm1 = cmp.elm1;
+           const elm1 = cmp.elm1;
-                const elm2 = cmp.elm2;
+           const elm2 = cmp.elm2;
-                const elm3 = cmp.elm3;
+           const elm3 = cmp.elm3;
-                assertHasParent(elm1);
+           assertHasParent(elm1);
-                assertHasParent(elm2);
+           assertHasParent(elm2);
-                assertHasParent(elm3);
+           assertHasParent(elm3);
-                engine.flush();
+           engine.flush();
-                finishPlayers(engine.players);
+           finishPlayers(engine.players);
 
-                cmp.exp1 = false;
+           cmp.exp1 = false;
-                fixture.detectChanges();
+           fixture.detectChanges();
-                assertHasParent(elm1, false);
+           assertHasParent(elm1, false);
-                assertHasParent(elm2);
+           assertHasParent(elm2);
-                assertHasParent(elm3);
+           assertHasParent(elm3);
-                engine.flush();
+           engine.flush();
-                expect(engine.players.length).toEqual(0);
+           expect(engine.players.length).toEqual(0);
 
-                cmp.exp2 = false;
+           cmp.exp2 = false;
-                fixture.detectChanges();
+           fixture.detectChanges();
-                assertHasParent(elm1, false);
+           assertHasParent(elm1, false);
-                assertHasParent(elm2, false);
+           assertHasParent(elm2, false);
-                assertHasParent(elm3);
+           assertHasParent(elm3);
-                engine.flush();
+           engine.flush();
-                expect(engine.players.length).toEqual(0);
+           expect(engine.players.length).toEqual(0);
 
-                cmp.exp3 = false;
+           cmp.exp3 = false;
-                fixture.detectChanges();
+           fixture.detectChanges();
-                assertHasParent(elm1, false);
+           assertHasParent(elm1, false);
-                assertHasParent(elm2, false);
+           assertHasParent(elm2, false);
-                assertHasParent(elm3);
+           assertHasParent(elm3);
-                engine.flush();
+           engine.flush();
-                expect(engine.players.length).toEqual(1);
+           expect(engine.players.length).toEqual(1);
-              });
+         });
     });
   });