fix(ivy): Run ChangeDetection on transplanted views (#33644)

https://hackmd.io/@mhevery/rJUJsvv9H

Closes #33393

PR Close #33644

integration/_payload-limits.json

@@ -12,7 +12,7 @@
     "master": {
       "uncompressed": {
         "runtime-es2015": 1485,
-        "main-es2015": 15040,
+        "main-es2015": 15267,
         "polyfills-es2015": 36808
       }
     }

packages/core/src/render3/instructions/shared.ts

@@ -10,7 +10,7 @@ import {ErrorHandler} from '../../error_handler';
 import {CUSTOM_ELEMENTS_SCHEMA, NO_ERRORS_SCHEMA, SchemaMetadata} from '../../metadata/schema';
 import {validateAgainstEventAttributes, validateAgainstEventProperties} from '../../sanitization/sanitization';
 import {Sanitizer} from '../../sanitization/sanitizer';
-import {assertDataInRange, assertDefined, assertDomNode, assertEqual, assertGreaterThan, assertLessThanOrEqual, assertNotEqual, assertNotSame} from '../../util/assert';
+import {assertDataInRange, assertDefined, assertDomNode, assertEqual, assertGreaterThan, assertNotEqual, assertNotSame} from '../../util/assert';
 import {createNamedArrayType} from '../../util/named_array_type';
 import {initNgDevMode} from '../../util/ng_dev_mode';
 import {normalizeDebugBindingName, normalizeDebugBindingValue} from '../../util/ng_reflect';
@@ -20,7 +20,7 @@ import {getFactoryDef} from '../definition';
 import {diPublicInInjector, getNodeInjectable, getOrCreateNodeInjectorForNode} from '../di';
 import {throwMultipleComponentError} from '../errors';
 import {executeCheckHooks, executeInitAndCheckHooks, incrementInitPhaseFlags, registerPreOrderHooks} from '../hooks';
-import {ACTIVE_INDEX, CONTAINER_HEADER_OFFSET, LContainer} from '../interfaces/container';
+import {ACTIVE_INDEX, CONTAINER_HEADER_OFFSET, LContainer, MOVED_VIEWS} from '../interfaces/container';
 import {ComponentDef, ComponentTemplate, DirectiveDef, DirectiveDefListOrFactory, PipeDefListOrFactory, RenderFlags, ViewQueriesFunction} from '../interfaces/definition';
 import {INJECTOR_BLOOM_PARENT_SIZE, NodeInjectorFactory} from '../interfaces/injector';
 import {AttributeMarker, InitialInputData, InitialInputs, LocalRefExtractor, PropertyAliasValue, PropertyAliases, TAttributes, TConstants, TContainerNode, TDirectiveHostNode, TElementContainerNode, TElementNode, TIcuContainerNode, TNode, TNodeFlags, TNodeProviderIndexes, TNodeType, TProjectionNode, TViewNode} from '../interfaces/node';
@@ -30,13 +30,13 @@ import {isComponentDef, isComponentHost, isContentQueryHost, isLContainer, isRoo
 import {CHILD_HEAD, CHILD_TAIL, CLEANUP, CONTEXT, DECLARATION_VIEW, ExpandoInstructions, FLAGS, HEADER_OFFSET, HOST, INJECTOR, InitPhaseState, LView, LViewFlags, NEXT, PARENT, RENDERER, RENDERER_FACTORY, RootContext, RootContextFlags, SANITIZER, TData, TVIEW, TView, TViewType, T_HOST} from '../interfaces/view';
 import {assertNodeOfPossibleTypes} from '../node_assert';
 import {isNodeMatchingSelectorList} from '../node_selector_matcher';
-import {ActiveElementFlags, enterView, executeElementExitFn, getBindingIndex, getBindingsEnabled, getCheckNoChangesMode, getIsParent, getPreviousOrParentTNode, getSelectedIndex, hasActiveElementFlag, incrementActiveDirectiveId, leaveView, leaveViewProcessExit, setActiveHostElement, setBindingIndex, setBindingRoot, setCheckNoChangesMode, setCurrentDirectiveDef, setCurrentQueryIndex, setPreviousOrParentTNode, setSelectedIndex} from '../state';
+import {ActiveElementFlags, enterView, executeElementExitFn, getBindingsEnabled, getCheckNoChangesMode, getIsParent, getPreviousOrParentTNode, getSelectedIndex, hasActiveElementFlag, incrementActiveDirectiveId, leaveView, leaveViewProcessExit, setActiveHostElement, setBindingIndex, setBindingRoot, setCheckNoChangesMode, setCurrentDirectiveDef, setCurrentQueryIndex, setPreviousOrParentTNode, setSelectedIndex} from '../state';
 import {renderStylingMap, writeStylingValueDirectly} from '../styling/bindings';
 import {NO_CHANGE} from '../tokens';
 import {isAnimationProp} from '../util/attrs_utils';
 import {INTERPOLATION_DELIMITER, renderStringify, stringifyForError} from '../util/misc_utils';
 import {getInitialStylingValue} from '../util/styling_utils';
-import {getLViewParent} from '../util/view_traversal_utils';
+import {findComponentView, getLViewParent} from '../util/view_traversal_utils';
 import {getComponentLViewByIndex, getNativeByIndex, getNativeByTNode, getTNode, isCreationMode, readPatchedLView, resetPreOrderHookFlags, unwrapRNode, viewAttachedToChangeDetector} from '../util/view_utils';
 
 import {selectIndexInternal} from './advance';
@@ -1444,21 +1444,21 @@ const LContainerArray: any = ((typeof ngDevMode === 'undefined' || ngDevMode) &&
  * @returns LContainer
  */
 export function createLContainer(
-    hostNative: RElement | RComment | LView, currentView: LView, native: RComment, tNode: TNode,
+    hostNative: RElement | RComment | LView, currentView: LView, native: RComment,
-    isForViewContainerRef?: boolean): LContainer {
+    tNode: TNode): LContainer {
   ngDevMode && assertLView(currentView);
   ngDevMode && !isProceduralRenderer(currentView[RENDERER]) && assertDomNode(native);
   // https://jsperf.com/array-literal-vs-new-array-really
   const lContainer: LContainer = new (ngDevMode ? LContainerArray : Array)(
-      hostNative,  // host native
+      hostNative,   // host native
-      true,        // Boolean `true` in this position signifies that this is an `LContainer`
+      true,         // Boolean `true` in this position signifies that this is an `LContainer`
-      isForViewContainerRef ? -1 : 0,  // active index
+      -1,           // active index
-      currentView,                     // parent
+      currentView,  // parent
-      null,                            // next
+      null,         // next
-      null,                            // queries
+      null,         // queries
-      tNode,                           // t_host
+      tNode,        // t_host
-      native,                          // native,
+      native,       // native,
-      null,                            // view refs
+      null,         // view refs
       );
   ngDevMode && attachLContainerDebug(lContainer);
   return lContainer;
@@ -1481,6 +1481,32 @@ function refreshDynamicEmbeddedViews(lView: LView) {
         ngDevMode && assertDefined(embeddedTView, 'TView must be allocated');
         refreshView(embeddedLView, embeddedTView, embeddedTView.template, embeddedLView[CONTEXT] !);
       }
+      const movedViews = viewOrContainer[MOVED_VIEWS];
+      if (movedViews !== null) {
+        // We should only CD moved views if the component where they were inserted does not match
+        // the component where they were declared. Moved views also contains intra component moves,
+        // which we don't care about.
+        // TODO(misko): this is not the most efficient way to do this as we have to do a lot of
+        // searches. Will refactor for performance later.
+        const declaredComponentLView = findComponentView(lView);
+        for (let i = 0; i < movedViews.length; i++) {
+          const movedLView = movedViews[i] !;
+          let parentLView = movedLView[PARENT];
+          while (isLContainer(parentLView)) {
+            parentLView = parentLView[PARENT];
+          }
+          const insertedComponentLView = findComponentView(parentLView !);
+          const insertionIsOnPush =
+              (insertedComponentLView[FLAGS] & LViewFlags.CheckAlways) !== LViewFlags.CheckAlways;
+          if (insertionIsOnPush && insertedComponentLView !== declaredComponentLView) {
+            // Here we know that the template has been transplanted across components
+            // (not just moved within a component)
+            const movedTView = movedLView[TVIEW];
+            ngDevMode && assertDefined(movedTView, 'TView must be allocated');
+            refreshView(movedLView, movedTView, movedTView.template, movedLView[CONTEXT] !);
+          }
+        }
+      }
     }
     viewOrContainer = viewOrContainer[NEXT];
   }

packages/core/src/render3/node_manipulation.ts

@@ -256,12 +256,13 @@ export function insertView(lView: LView, lContainer: LContainer, index: number)
  * different LContainer.
  */
 function trackMovedView(declarationContainer: LContainer, lView: LView) {
+  ngDevMode && assertDefined(lView, 'LView required');
   ngDevMode && assertLContainer(declarationContainer);
-  const declaredViews = declarationContainer[MOVED_VIEWS];
+  const movedViews = declarationContainer[MOVED_VIEWS];
-  if (declaredViews === null) {
+  if (movedViews === null) {
     declarationContainer[MOVED_VIEWS] = [lView];
   } else {
-    declaredViews.push(lView);
+    movedViews.push(lView);
   }
 }
 
@@ -270,9 +271,9 @@ function detachMovedView(declarationContainer: LContainer, lView: LView) {
   ngDevMode && assertDefined(
                    declarationContainer[MOVED_VIEWS],
                    'A projected view should belong to a non-empty projected views collection');
-  const projectedViews = declarationContainer[MOVED_VIEWS] !;
+  const movedViews = declarationContainer[MOVED_VIEWS] !;
-  const declaredViewIndex = projectedViews.indexOf(lView);
+  const declaredViewIndex = movedViews.indexOf(lView);
-  projectedViews.splice(declaredViewIndex, 1);
+  movedViews.splice(declaredViewIndex, 1);
 }
 
 /**

packages/core/src/render3/util/view_traversal_utils.ts

@@ -56,9 +56,10 @@ export function getRootView(componentOrLView: LView | {}): LView {
  */
 export function findComponentView(lView: LView): LView {
   let rootTNode = lView[T_HOST];
-  while (rootTNode !== null && rootTNode.type === TNodeType.View) {
+  let declaredView: LView|null;
-    ngDevMode && assertDefined(lView[DECLARATION_VIEW], 'lView[DECLARATION_VIEW]');
+  while (rootTNode !== null && rootTNode.type === TNodeType.View &&
-    lView = lView[DECLARATION_VIEW] !;
+         (declaredView = lView[DECLARATION_VIEW]) !== null) {
+    lView = declaredView;
     rootTNode = lView[T_HOST];
   }
   ngDevMode && assertLView(lView);

packages/core/src/render3/view_engine_compatibility.ts

@@ -25,7 +25,7 @@ import {ACTIVE_INDEX, CONTAINER_HEADER_OFFSET, LContainer, VIEW_REFS} from './in
 import {TContainerNode, TElementContainerNode, TElementNode, TNode, TNodeType, TViewNode} from './interfaces/node';
 import {RComment, RElement, isProceduralRenderer} from './interfaces/renderer';
 import {isComponentHost, isLContainer, isLView, isRootView} from './interfaces/type_checks';
-import {CONTEXT, DECLARATION_LCONTAINER, LView, LViewFlags, QUERIES, RENDERER, TView, T_HOST} from './interfaces/view';
+import {DECLARATION_LCONTAINER, LView, LViewFlags, QUERIES, RENDERER, TView, T_HOST} from './interfaces/view';
 import {assertNodeOfPossibleTypes} from './node_assert';
 import {addRemoveViewFromContainer, appendChild, detachView, getBeforeNodeForView, insertView, nativeInsertBefore, nativeNextSibling, nativeParentNode, removeView} from './node_manipulation';
 import {getParentInjectorTNode} from './node_util';
@@ -353,7 +353,7 @@ export function createContainerRef(
     }
 
     hostView[hostTNode.index] = lContainer =
-        createLContainer(slotValue, hostView, commentNode, hostTNode, true);
+        createLContainer(slotValue, hostView, commentNode, hostTNode);
 
     addToViewTree(hostView, lContainer);
   }

packages/core/test/acceptance/change_detection_spec.ts

@@ -9,7 +9,11 @@
 
 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 {ivyEnabled} from '@angular/core/src/ivy_switch';
+import {markDirty} from '@angular/core/src/render3/index';
+import {CONTEXT} from '@angular/core/src/render3/interfaces/view';
+import {getCheckNoChangesMode, instructionState} from '@angular/core/src/render3/state';
+import {ComponentFixture, TestBed} from '@angular/core/testing';
 import {expect} from '@angular/platform-browser/testing/src/matchers';
 import {BehaviorSubject} from 'rxjs';
 
@@ -1098,4 +1102,140 @@ describe('change detection', () => {
 
   });
 
+  describe('transplanted views', () => {
+    @Component({
+      selector: 'insert-comp',
+      changeDetection: ChangeDetectionStrategy.OnPush,
+      template: `
+        InsertComp({{greeting}})
+        <ng-container
+            [ngTemplateOutlet]="template"
+            [ngTemplateOutletContext]="{$implicit: greeting}">
+        </ng-container>
+      `
+    })
+    class InsertComp {
+      get template(): TemplateRef<any> { return declareComp.myTmpl; }
+      greeting: string = 'Hello';
+      constructor(public changeDetectorRef: ChangeDetectorRef) { insertComp = this; }
+    }
+
+    @Component({
+      selector: `declare-comp`,
+      template: `
+        DeclareComp({{name}})
+        <ng-template #myTmpl let-greeting>
+          {{greeting}} {{nameWithLog}}!
+        </ng-template>
+      `
+    })
+    class DeclareComp {
+      @ViewChild('myTmpl')
+      myTmpl !: TemplateRef<any>;
+      name: string = 'world';
+      get nameWithLog() {
+        if (ivyEnabled && !getCheckNoChangesMode()) {
+          const lFrame = instructionState.lFrame;
+          const parentChangeDetectionLView = lFrame.parent.lView;
+          if (parentChangeDetectionLView[CONTEXT] === declareComp) {
+            log.push('Declare');
+          } else if (parentChangeDetectionLView[CONTEXT] === insertComp) {
+            log.push('Insert');
+          } else {
+            log.push(
+                'UNEXPECTED VIEW: ' + parentChangeDetectionLView ![CONTEXT] !.constructor.name);
+          }
+        }
+        return this.name;
+      }
+      constructor() { declareComp = this; }
+    }
+
+    @Component({
+      template: `
+      <declare-comp *ngIf="showDeclare"></declare-comp>
+      <insert-comp *ngIf="showInsert"></insert-comp>
+      `
+    })
+    class AppComp {
+      showDeclare: boolean = true;
+      showInsert: boolean = true;
+      constructor() { appComp = this; }
+    }
+
+    let log !: string[];
+    let fixture !: ComponentFixture<AppComp>;
+    let appComp !: AppComp;
+    let insertComp !: InsertComp;
+    let declareComp !: DeclareComp;
+
+    beforeEach(() => {
+      TestBed.configureTestingModule({
+        declarations: [InsertComp, DeclareComp, AppComp],
+        imports: [CommonModule],
+      });
+      log = [];
+      fixture = TestBed.createComponent(AppComp);
+    });
+
+    it('should CD with declaration', () => {
+      fixture.detectChanges();
+      ivyEnabled && expect(log).toEqual(['Insert']);
+      log.length = 0;
+      expect(trim(fixture.nativeElement.textContent))
+          .toEqual('DeclareComp(world) InsertComp(Hello) Hello world!');
+
+      declareComp.name = 'Angular';
+      fixture.detectChanges();
+      ivyEnabled && expect(log).toEqual(['Declare']);
+      log.length = 0;
+      // Expect transplanted LView to be CD because the declaration is CD.
+      expect(trim(fixture.nativeElement.textContent))
+          .toEqual('DeclareComp(Angular) InsertComp(Hello) Hello Angular!');
+
+      insertComp.greeting = 'Hi';
+      fixture.detectChanges();
+      ivyEnabled && expect(log).toEqual(['Declare']);
+      log.length = 0;
+      // expect no change because it is on push.
+      expect(trim(fixture.nativeElement.textContent))
+          .toEqual('DeclareComp(Angular) InsertComp(Hello) Hello Angular!');
+
+      insertComp.changeDetectorRef.markForCheck();
+      fixture.detectChanges();
+      ivyEnabled && expect(log).toEqual(['Declare', 'Insert']);
+      log.length = 0;
+      expect(trim(fixture.nativeElement.textContent))
+          .toEqual('DeclareComp(Angular) InsertComp(Hi) Hi Angular!');
+
+      // Destroy insertion should also destroy declaration
+      appComp.showInsert = false;
+      insertComp.changeDetectorRef.markForCheck();
+      fixture.detectChanges();
+      ivyEnabled && expect(log).toEqual([]);  // No update in declaration
+      log.length = 0;
+      expect(trim(fixture.nativeElement.textContent)).toEqual('DeclareComp(Angular)');
+
+      // Restore both
+      appComp.showInsert = true;
+      fixture.detectChanges();
+      ivyEnabled && expect(log).toEqual(['Insert']);
+      log.length = 0;
+      expect(trim(fixture.nativeElement.textContent))
+          .toEqual('DeclareComp(Angular) InsertComp(Hello) Hello Angular!');
+
+      // Destroy declaration, But we should still be able to see updates in insertion
+      appComp.showDeclare = false;
+      insertComp.greeting = 'Hello';
+      insertComp.changeDetectorRef.markForCheck();
+      fixture.detectChanges();
+      ivyEnabled && expect(log).toEqual(['Insert']);
+      log.length = 0;
+      expect(trim(fixture.nativeElement.textContent)).toEqual('InsertComp(Hello) Hello Angular!');
+    });
+  });
 });
+
+function trim(text: string | null): string {
+  return text ? text.replace(/[\s\n]+/gm, ' ').trim() : '';
+}

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

@@ -56,6 +56,9 @@
   {
     "name": "MONKEY_PATCH_KEY_NAME"
   },
+  {
+    "name": "MOVED_VIEWS"
+  },
   {
     "name": "Module"
   },

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

@@ -53,6 +53,9 @@
   {
     "name": "MONKEY_PATCH_KEY_NAME"
   },
+  {
+    "name": "MOVED_VIEWS"
+  },
   {
     "name": "NATIVE"
   },

packages/core/test/render3/perf/setup.ts

@@ -56,7 +56,7 @@ export function setupTestHarness(
       rendererFactory, renderer);
   const mockRCommentNode = renderer.createComment('');
   const lContainer =
-      createLContainer(mockRCommentNode, hostLView, mockRCommentNode, tContainerNode, true);
+      createLContainer(mockRCommentNode, hostLView, mockRCommentNode, tContainerNode);
   addToViewTree(hostLView, lContainer);
 
 

packages/core/test/render3/view_utils_spec.ts

@@ -16,7 +16,7 @@ describe('view_utils', () => {
     const tView = createTView(TViewType.Root, 0, null, 0, 0, null, null, null, null, null);
     const lView = createLView(null, tView, {}, 0, div, null, {} as any, {} as any, null, null);
     const tNode = createTNode(null !, null, 3, 0, 'div', []);
-    const lContainer = createLContainer(lView, lView, div, tNode, true);
+    const lContainer = createLContainer(lView, lView, div, tNode);
 
     expect(isLView(lView)).toBe(true);
     expect(isLView(lContainer)).toBe(false);