refactor(ivy): ViewRef needs embededViewRef declaration (#33074)

PR Close #33074

packages/core/src/render3/view_engine_compatibility.ts

@@ -122,7 +122,7 @@ export function createTemplateRef<T>(
 
         renderView(lView, embeddedTView, context);
 
-        const viewRef = new ViewRef(lView, context, -1);
+        const viewRef = new ViewRef<T>(lView);
         viewRef._tViewNode = lView[T_HOST] as TViewNode;
         return viewRef;
       }
@@ -295,7 +295,7 @@ export function createContainerRef(
 
         const wasDetached =
             view && removeFromArray(this._lContainer[VIEW_REFS] !, adjustedIdx) != null;
-        return wasDetached ? new ViewRef(view !, view ![CONTEXT], -1) : null;
+        return wasDetached ? new ViewRef(view !) : null;
       }
 
       private _adjustIndex(index?: number, shift: number = 0) {
@@ -370,22 +370,27 @@ export function injectChangeDetectorRef(isPipe = false): ViewEngine_ChangeDetect
 /**
  * Creates a ViewRef and stores it on the injector as ChangeDetectorRef (public alias).
  *
- * @param hostTNode The node that is requesting a ChangeDetectorRef
+ * @param tNode The node that is requesting a ChangeDetectorRef
- * @param hostView The view to which the node belongs
+ * @param lView The view to which the node belongs
  * @param isPipe Whether the view is being injected into a pipe.
  * @returns The ChangeDetectorRef to use
  */
-function createViewRef(
+function createViewRef(tNode: TNode, lView: LView, isPipe: boolean): ViewEngine_ChangeDetectorRef {
-    hostTNode: TNode, hostView: LView, isPipe: boolean): ViewEngine_ChangeDetectorRef {
+  // `isComponentView` will be true for Component and Directives (but not for Pipes).
-  if (isComponentHost(hostTNode) && !isPipe) {
+  // See https://github.com/angular/angular/pull/33072 for proper fix
-    const componentIndex = hostTNode.directiveStart;
+  const isComponentView = !isPipe && isComponentHost(tNode);
-    const componentView = getComponentLViewByIndex(hostTNode.index, hostView);
+  if (isComponentView) {
-    return new ViewRef(componentView, null, componentIndex);
+    // The LView represents the location where the component is declared.
+    // Instead we want the LView for the component View and so we need to look it up.
+    const componentView = getComponentLViewByIndex(tNode.index, lView);  // look down
+    return new ViewRef(componentView, componentView);
   } else if (
-      hostTNode.type === TNodeType.Element || hostTNode.type === TNodeType.Container ||
+      tNode.type === TNodeType.Element || tNode.type === TNodeType.Container ||
-      hostTNode.type === TNodeType.ElementContainer) {
+      tNode.type === TNodeType.ElementContainer) {
-    const hostComponentView = findComponentView(hostView);
+    // The LView represents the location where the injection is requested from.
-    return new ViewRef(hostComponentView, hostComponentView[CONTEXT], -1);
+    // We need to locate the containing LView (in case where the `lView` is an embedded view)
+    const hostComponentView = findComponentView(lView);  // look up
+    return new ViewRef(hostComponentView, lView);
   }
   return null !;
 }

packages/core/src/render3/view_ref.ts

@@ -13,7 +13,7 @@ import {EmbeddedViewRef as viewEngine_EmbeddedViewRef, InternalViewRef as viewEn
 
 import {checkNoChangesInRootView, checkNoChangesInternal, detectChangesInRootView, detectChangesInternal, markViewDirty, storeCleanupFn} from './instructions/shared';
 import {TElementNode, TNode, TNodeType, TViewNode} from './interfaces/node';
-import {FLAGS, HOST, LView, LViewFlags, T_HOST} from './interfaces/view';
+import {CONTEXT, FLAGS, HOST, LView, LViewFlags, T_HOST} from './interfaces/view';
 import {destroyLView, renderDetachView} from './node_manipulation';
 import {findComponentView, getLViewParent} from './util/view_traversal_utils';
 import {getNativeByTNode, getNativeByTNodeOrNull} from './util/view_utils';
@@ -35,11 +35,6 @@ export class ViewRef<T> implements viewEngine_EmbeddedViewRef<T>, viewEngine_Int
    */
   public _tViewNode: TViewNode|null = null;
 
-  /**
-   * @internal
-   */
-  public _lView: LView;
-
   get rootNodes(): any[] {
     if (this._lView[HOST] == null) {
       const tView = this._lView[T_HOST] as TViewNode;
@@ -48,11 +43,29 @@ export class ViewRef<T> implements viewEngine_EmbeddedViewRef<T>, viewEngine_Int
     return [];
   }
 
-  constructor(_lView: LView, private _context: T|null, private _componentIndex: number) {
+  constructor(
-    this._lView = _lView;
+      /**
-  }
+       * This represents `LView` associated with the component when ViewRef is a ChangeDetectorRef.
+       *
+       * When ViewRef is created for a dynamic component, this also represents the `LView` for the
+       * component.
+       *
+       * For a "regular" ViewRef created for an embedded view, this is the `LView` for the embedded
+       * view.
+       *
+       * @internal
+       */
+      public _lView: LView,
 
-  get context(): T { return this._context ? this._context : this._lookUpContext(); }
+      /**
+       * This represents the `LView` associated with the point where `ChangeDetectorRef` was
+       * requested.
+       *
+       * This may be different from `_lView` if the `_cdRefInjectingView` is an embedded view.
+       */
+      private _cdRefInjectingView?: LView) {}
+
+  get context(): T { return this._lView[CONTEXT] as T; }
 
   get destroyed(): boolean {
     return (this._lView[FLAGS] & LViewFlags.Destroyed) === LViewFlags.Destroyed;
@@ -109,7 +122,7 @@ export class ViewRef<T> implements viewEngine_EmbeddedViewRef<T>, viewEngine_Int
    * }
    * ```
    */
-  markForCheck(): void { markViewDirty(this._lView); }
+  markForCheck(): void { markViewDirty(this._cdRefInjectingView || this._lView); }
 
   /**
    * Detaches the view from the change detection tree.
@@ -273,15 +286,11 @@ export class ViewRef<T> implements viewEngine_EmbeddedViewRef<T>, viewEngine_Int
     }
     this._appRef = appRef;
   }
-
-  private _lookUpContext(): T {
-    return this._context = getLViewParent(this._lView) ![this._componentIndex] as T;
-  }
 }
 
 /** @internal */
 export class RootViewRef<T> extends ViewRef<T> {
-  constructor(public _view: LView) { super(_view, null, -1); }
+  constructor(public _view: LView) { super(_view); }
 
   detectChanges(): void { detectChangesInRootView(this._view); }
 

packages/core/test/acceptance/change_detection_spec.ts

@@ -11,6 +11,7 @@ import {CommonModule} from '@angular/common';
 import {ApplicationRef, ChangeDetectionStrategy, ChangeDetectorRef, Component, ComponentFactoryResolver, ComponentRef, Directive, DoCheck, EmbeddedViewRef, ErrorHandler, Input, NgModule, OnInit, QueryList, TemplateRef, Type, ViewChild, ViewChildren, ViewContainerRef} from '@angular/core';
 import {TestBed} from '@angular/core/testing';
 import {expect} from '@angular/platform-browser/testing/src/matchers';
+import {BehaviorSubject} from 'rxjs';
 
 describe('change detection', () => {
 
@@ -913,6 +914,41 @@ describe('change detection', () => {
         expect(fixture.nativeElement.textContent).toEqual('two - two');
       });
 
+      it('async pipe should trigger CD for embedded views where the declaration and insertion views are different',
+         () => {
+           @Component({
+             selector: 'insertion',
+             changeDetection: ChangeDetectionStrategy.OnPush,
+             template: ` <ng-container [ngTemplateOutlet]="template"> </ng-container> `
+           })
+           class Insertion {
+             @Input() template !: TemplateRef<{}>;
+           }
+
+           // This component uses async pipe (which calls markForCheck) in a view that has different
+           // insertion and declaration views.
+           @Component({
+             changeDetection: ChangeDetectionStrategy.OnPush,
+             template: `
+          <insertion [template]="ref"></insertion>
+          <ng-template #ref>
+            <span>{{value | async}}</span>
+          </ng-template>
+          `
+           })
+           class Declaration {
+             value = new BehaviorSubject('initial value');
+           }
+
+           const fixture = TestBed.configureTestingModule({declarations: [Insertion, Declaration]})
+                               .createComponent(Declaration);
+           fixture.detectChanges();
+           expect(fixture.debugElement.nativeElement.textContent).toContain('initial value');
+           fixture.componentInstance.value.next('new value');
+           fixture.detectChanges();
+           expect(fixture.debugElement.nativeElement.textContent).toContain('new value');
+         });
+
       // TODO(kara): add test for dynamic views once bug fix is in
     });
 

packages/core/test/bundling/cyclic_import/bundle.golden_symbols.json

@@ -282,7 +282,7 @@
     "name": "getComponentDef"
   },
   {
-    "name": "getComponentViewByIndex"
+    "name": "getComponentLViewByIndex"
   },
   {
     "name": "getContainerRenderParent"

packages/core/test/bundling/hello_world/bundle.golden_symbols.json

@@ -225,7 +225,7 @@
     "name": "getComponentDef"
   },
   {
-    "name": "getComponentViewByIndex"
+    "name": "getComponentLViewByIndex"
   },
   {
     "name": "getContainerRenderParent"

packages/core/test/bundling/todo/bundle.golden_symbols.json

@@ -621,7 +621,7 @@
     "name": "getComponentDef"
   },
   {
-    "name": "getComponentViewByIndex"
+    "name": "getComponentLViewByIndex"
   },
   {
     "name": "getComponentViewByInstance"