fix(ivy): should mark OnPush ancestor of dynamically created views as dirty (#28687)

While marking a given views tree as dirty we should go all the way to the
root of the views tree and cross boundaries of dynamically inserted views.
In other words the markForCheck functionality should consider parents of
dynamically inserted views.

PR Close #28687

packages/core/src/render3/instructions.ts

@@ -30,7 +30,7 @@ import {AttributeMarker, InitialInputData, InitialInputs, LocalRefExtractor, Pro
 import {PlayerFactory} from './interfaces/player';
 import {CssSelectorList, NG_PROJECT_AS_ATTR_NAME} from './interfaces/projection';
 import {LQueries} from './interfaces/query';
-import {GlobalTargetResolver, ProceduralRenderer3, RComment, RElement, RNode, RText, Renderer3, RendererFactory3, isProceduralRenderer} from './interfaces/renderer';
+import {GlobalTargetResolver, ProceduralRenderer3, RComment, RElement, RText, Renderer3, RendererFactory3, isProceduralRenderer} from './interfaces/renderer';
 import {SanitizerFn} from './interfaces/sanitization';
 import {BINDING_INDEX, CLEANUP, CONTAINER_INDEX, CONTEXT, DECLARATION_VIEW, FLAGS, HEADER_OFFSET, HOST, INJECTOR, InitPhaseState, LView, LViewFlags, NEXT, OpaqueViewState, PARENT, QUERIES, RENDERER, RENDERER_FACTORY, RootContext, RootContextFlags, SANITIZER, TAIL, TData, TVIEW, TView, T_HOST} from './interfaces/view';
 import {assertNodeOfPossibleTypes, assertNodeType} from './node_assert';
@@ -41,7 +41,7 @@ import {getInitialClassNameValue, getInitialStyleStringValue, initializeStaticCo
 import {BoundPlayerFactory} from './styling/player_factory';
 import {ANIMATION_PROP_PREFIX, allocateDirectiveIntoContext, createEmptyStylingContext, forceClassesAsString, forceStylesAsString, getStylingContext, hasClassInput, hasStyleInput, hasStyling, isAnimationProp} from './styling/util';
 import {NO_CHANGE} from './tokens';
-import {INTERPOLATION_DELIMITER, findComponentView, getComponentViewByIndex, getNativeByIndex, getNativeByTNode, getRootContext, getRootView, getTNode, isComponent, isComponentDef, isContentQueryHost, loadInternal, readElementValue, readPatchedLView, renderStringify} from './util';
+import {INTERPOLATION_DELIMITER, findComponentView, getComponentViewByIndex, getNativeByIndex, getNativeByTNode, getRootContext, getRootView, getTNode, isComponent, isComponentDef, isContentQueryHost, isRootView, loadInternal, readElementValue, readPatchedLView, renderStringify} from './util';
 
 
 
@@ -2729,15 +2729,16 @@ function wrapListener(
  * @returns the root LView
  */
 export function markViewDirty(lView: LView): LView|null {
-  while (lView && !(lView[FLAGS] & LViewFlags.IsRoot)) {
+  while (lView) {
     lView[FLAGS] |= LViewFlags.Dirty;
+    // Stop traversing up as soon as you find a root view that wasn't attached to any container
+    if (isRootView(lView) && lView[CONTAINER_INDEX] === -1) {
+      return lView;
+    }
+    // continue otherwise
     lView = lView[PARENT] !;
   }
-  // Detached views do not have a PARENT and also aren't root views
+  return null;
-  if (lView) {
-    lView[FLAGS] |= LViewFlags.Dirty;
-  }
-  return lView;
 }
 
 /**

packages/core/src/render3/util.ts

@@ -16,7 +16,7 @@ import {NO_PARENT_INJECTOR, RelativeInjectorLocation, RelativeInjectorLocationFl
 import {TContainerNode, TElementNode, TNode, TNodeFlags, TNodeType} from './interfaces/node';
 import {RComment, RElement, RText} from './interfaces/renderer';
 import {StylingContext} from './interfaces/styling';
-import {CONTEXT, DECLARATION_VIEW, FLAGS, HEADER_OFFSET, HOST, LView, LViewFlags, PARENT, RootContext, TData, TVIEW, TView, T_HOST} from './interfaces/view';
+import {CONTEXT, DECLARATION_VIEW, FLAGS, HEADER_OFFSET, HOST, LView, LViewFlags, PARENT, RootContext, TData, TVIEW, T_HOST} from './interfaces/view';
 
 
 

packages/core/test/acceptance/change_detection_spec.ts

@@ -7,7 +7,7 @@
  */
 
 
-import {ApplicationRef, Component, Directive, EmbeddedViewRef, TemplateRef, ViewContainerRef} from '@angular/core';
+import {ApplicationRef, ChangeDetectionStrategy, Component, ComponentFactoryResolver, ComponentRef, Directive, EmbeddedViewRef, NgModule, TemplateRef, Type, ViewChild, ViewContainerRef} from '@angular/core';
 import {TestBed} from '@angular/core/testing';
 import {expect} from '@angular/platform-browser/testing/src/matchers';
 
@@ -68,4 +68,63 @@ describe('change detection', () => {
 
   });
 
+  describe('markForCheck', () => {
+
+    it('should mark OnPush ancestor of dynamically created component views as dirty', () => {
+
+      @Component({
+        selector: `test-cmpt`,
+        template: `{{counter}}|<ng-template #vc></ng-template>`,
+        changeDetection: ChangeDetectionStrategy.OnPush
+      })
+      class TestCmpt {
+        counter = 0;
+        @ViewChild('vc', {read: ViewContainerRef}) vcRef !: ViewContainerRef;
+
+        constructor(private _cfr: ComponentFactoryResolver) {}
+
+        createComponentView<T>(cmptType: Type<T>): ComponentRef<T> {
+          const cf = this._cfr.resolveComponentFactory(cmptType);
+          return this.vcRef.createComponent(cf);
+        }
+      }
+
+      @Component({
+        selector: 'dynamic-cmpt',
+        template: `dynamic`,
+        changeDetection: ChangeDetectionStrategy.OnPush
+      })
+      class DynamicCmpt {
+      }
+
+      @NgModule({declarations: [DynamicCmpt], entryComponents: [DynamicCmpt]})
+      class DynamicModule {
+      }
+
+      TestBed.configureTestingModule({imports: [DynamicModule], declarations: [TestCmpt]});
+
+      const fixture = TestBed.createComponent(TestCmpt);
+
+      // initial CD to have query results
+      // NOTE: we call change detection without checkNoChanges to have clearer picture
+      fixture.detectChanges(false);
+      expect(fixture.nativeElement).toHaveText('0|');
+
+      // insert a dynamic component
+      const dynamicCmptRef = fixture.componentInstance.createComponentView(DynamicCmpt);
+      fixture.detectChanges(false);
+      expect(fixture.nativeElement).toHaveText('0|dynamic');
+
+      // update model in the OnPush component - should not update UI
+      fixture.componentInstance.counter = 1;
+      fixture.detectChanges(false);
+      expect(fixture.nativeElement).toHaveText('0|dynamic');
+
+      // now mark the dynamically inserted component as dirty
+      dynamicCmptRef.changeDetectorRef.markForCheck();
+      fixture.detectChanges(false);
+      expect(fixture.nativeElement).toHaveText('1|dynamic');
+    });
+  });
+
 });