refactor(core): newly created and any dirty views should get refreshed during CD (#53022)

When a view has the `Dirty` flag and is reattached, we should ensure that it is
reached and refreshed during the next change detection run from above.

In addition, when a view is created and attached, we should ensure that it is reached
and refreshed during change detection. This can happen if the view is
created and attached outside a change run or when it is created and
attached after its insertion view was already checked. In both cases, we
should ensure that the view is reached and refreshed during either the
current change detection or the next one (if change detection is not
already running).

We can achieve this by creating all views with the `Dirty` flag set.

However, this does happen to be a breaking change in some scenarios.
The one identified internally was actually depending on change detection
_not_ running immediately because it relied on an input value that was
set using `ngModel`. Because `ngModel` sets its value in a `Promise`, it
is not available until the _next_ change detection cycle. Ensuring
created views run in the current change change detection will result in
different behavior in this case.

Making option the default is the solution to #52928. That will have to
wait for a major version.

PR Close #53022

packages/core/src/change_detection/flags.ts

@@ -0,0 +1,17 @@
+/**
+ * @license
+ * Copyright Google LLC All Rights Reserved.
+ *
+ * Use of this source code is governed by an MIT-style license that can be
+ * found in the LICENSE file at https://angular.io/license
+ */
+
+// TODO(atscott): flip default internally ASAP and externally for v18 (#52928)
+let _ensureDirtyViewsAreAlwaysReachable = false;
+
+export function getEnsureDirtyViewsAreAlwaysReachable() {
+  return _ensureDirtyViewsAreAlwaysReachable;
+}
+export function setEnsureDirtyViewsAreAlwaysReachable(v: boolean) {
+  _ensureDirtyViewsAreAlwaysReachable = v;
+}

packages/core/src/core_private_export.ts

@@ -11,6 +11,7 @@ export {whenStable as ɵwhenStable} from './application/application_ref';
 export {IMAGE_CONFIG as ɵIMAGE_CONFIG, IMAGE_CONFIG_DEFAULTS as ɵIMAGE_CONFIG_DEFAULTS, ImageConfig as ɵImageConfig} from './application/application_tokens';
 export {internalCreateApplication as ɵinternalCreateApplication} from './application/create_application';
 export {defaultIterableDiffers as ɵdefaultIterableDiffers, defaultKeyValueDiffers as ɵdefaultKeyValueDiffers} from './change_detection/change_detection';
+export {getEnsureDirtyViewsAreAlwaysReachable as ɵgetEnsureDirtyViewsAreAlwaysReachable, setEnsureDirtyViewsAreAlwaysReachable as ɵsetEnsureDirtyViewsAreAlwaysReachable} from './change_detection/flags';
 export {Console as ɵConsole} from './console';
 export {DeferBlockDetails as ɵDeferBlockDetails, getDeferBlocks as ɵgetDeferBlocks} from './defer/discovery';
 export {renderDeferBlockState as ɵrenderDeferBlockState, triggerResourceLoading as ɵtriggerResourceLoading} from './defer/instructions';

packages/core/src/render3/component_ref.ts

@@ -220,12 +220,12 @@ export class ComponentFactory<T> extends AbstractComponentFactory<T> {
             hostRenderer, rootSelectorOrNode, this.componentDef.encapsulation, rootViewInjector) :
         createElementNode(hostRenderer, elementName, getNamespace(elementName));
 
-    // Signal components use the granular "RefreshView"  for change detection
-    const signalFlags = (LViewFlags.SignalView | LViewFlags.IsRoot);
-    // Non-signal components use the traditional "CheckAlways or OnPush/Dirty" change detection
-    const nonSignalFlags = this.componentDef.onPush ? LViewFlags.Dirty | LViewFlags.IsRoot :
-                                                      LViewFlags.CheckAlways | LViewFlags.IsRoot;
-    const rootFlags = this.componentDef.signals ? signalFlags : nonSignalFlags;
+    let rootFlags = LViewFlags.IsRoot;
+    if (this.componentDef.signals) {
+      rootFlags |= LViewFlags.SignalView;
+    } else if (!this.componentDef.onPush) {
+      rootFlags |= LViewFlags.CheckAlways;
+    }
 
     let hydrationInfo: DehydratedView|null = null;
     if (hostRNode !== null) {

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

@@ -94,7 +94,8 @@ export function createLView<T>(
     hydrationInfo: DehydratedView|null): LView<T> {
   const lView = tView.blueprint.slice() as LView;
   lView[HOST] = host;
-  lView[FLAGS] = flags | LViewFlags.CreationMode | LViewFlags.Attached | LViewFlags.FirstLViewPass;
+  lView[FLAGS] = flags | LViewFlags.CreationMode | LViewFlags.Attached | LViewFlags.FirstLViewPass |
+      LViewFlags.Dirty;
   if (embeddedViewInjector !== null ||
       (parentLView && (parentLView[FLAGS] & LViewFlags.HasEmbeddedViewInjector))) {
     lView[FLAGS] |= LViewFlags.HasEmbeddedViewInjector;

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

@@ -6,6 +6,7 @@
  * found in the LICENSE file at https://angular.io/license
  */
 
+import {getEnsureDirtyViewsAreAlwaysReachable} from '../../change_detection/flags';
 import {RuntimeError, RuntimeErrorCode} from '../../errors';
 import {assertDefined, assertGreaterThan, assertGreaterThanOrEqual, assertIndexInRange, assertLessThan} from '../../util/assert';
 import {assertTNode, assertTNodeForLView} from '../assert';
@@ -206,6 +207,12 @@ export function requiresRefreshOrTraversal(lView: LView) {
  * parents above.
  */
 export function updateAncestorTraversalFlagsOnAttach(lView: LView) {
+  // When we attach a view that's marked `Dirty`, we should ensure that it is reached during the
+  // next CD traversal so we add the `RefreshView` flag and mark ancestors accordingly.
+  if (lView[FLAGS] & LViewFlags.Dirty && getEnsureDirtyViewsAreAlwaysReachable()) {
+    lView[FLAGS] |= LViewFlags.RefreshView;
+  }
+
   if (!requiresRefreshOrTraversal(lView)) {
     return;
   }

packages/core/test/acceptance/change_detection_spec.ts

@@ -8,7 +8,7 @@
 
 
 import {CommonModule} from '@angular/common';
-import {ApplicationRef, ChangeDetectionStrategy, ChangeDetectorRef, Component, ComponentRef, createComponent, Directive, DoCheck, EmbeddedViewRef, ErrorHandler, EventEmitter, inject, Input, NgModule, OnInit, Output, QueryList, TemplateRef, Type, ViewChild, ViewChildren, ViewContainerRef} from '@angular/core';
+import {ApplicationRef, ChangeDetectionStrategy, ChangeDetectorRef, Component, ComponentRef, Directive, DoCheck, EmbeddedViewRef, ErrorHandler, EventEmitter, inject, Input, NgModule, OnInit, Output, QueryList, TemplateRef, Type, ViewChild, ViewChildren, ViewContainerRef, ɵgetEnsureDirtyViewsAreAlwaysReachable, ɵsetEnsureDirtyViewsAreAlwaysReachable} from '@angular/core';
 import {ComponentFixture, fakeAsync, TestBed, tick} from '@angular/core/testing';
 import {expect} from '@angular/platform-browser/testing/src/matchers';
 import {BehaviorSubject} from 'rxjs';
@@ -184,17 +184,18 @@ describe('change detection', () => {
 
   describe('markForCheck', () => {
     it('should mark OnPush ancestor of dynamically created component views as dirty', () => {
+      const previous = ɵgetEnsureDirtyViewsAreAlwaysReachable();
+      ɵsetEnsureDirtyViewsAreAlwaysReachable(true);
       @Component({
         selector: `test-cmpt`,
         template: `{{counter}}|<ng-template #vc></ng-template>`,
-        changeDetection: ChangeDetectionStrategy.OnPush
+        changeDetection: ChangeDetectionStrategy.OnPush,
+        standalone: true,
       })
       class TestCmpt {
         counter = 0;
         @ViewChild('vc', {read: ViewContainerRef}) vcRef!: ViewContainerRef;
 
-        constructor() {}
-
         createComponentView<T>(cmptType: Type<T>): ComponentRef<T> {
           return this.vcRef.createComponent(cmptType);
         }
@@ -202,18 +203,14 @@ describe('change detection', () => {
 
       @Component({
         selector: 'dynamic-cmpt',
-        template: `dynamic`,
+        template: `dynamic|{{binding}}`,
+        standalone: true,
         changeDetection: ChangeDetectionStrategy.OnPush
       })
       class DynamicCmpt {
+        @Input() binding = 'binding';
       }
 
-      @NgModule({declarations: [DynamicCmpt]})
-      class DynamicModule {
-      }
-
-      TestBed.configureTestingModule({imports: [DynamicModule], declarations: [TestCmpt]});
-
       const fixture = TestBed.createComponent(TestCmpt);
 
       // initial CD to have query results
@@ -221,20 +218,34 @@ describe('change detection', () => {
       fixture.detectChanges(false);
       expect(fixture.nativeElement).toHaveText('0|');
 
-      // insert a dynamic component
+      // insert a dynamic component, but do not specifically mark parent dirty
+      // (dynamic components with OnPush flag are created with the `Dirty` flag)
       const dynamicCmptRef = fixture.componentInstance.createComponentView(DynamicCmpt);
       fixture.detectChanges(false);
-      expect(fixture.nativeElement).toHaveText('0|dynamic');
+      expect(fixture.nativeElement).toHaveText('0|dynamic|binding');
 
       // update model in the OnPush component - should not update UI
       fixture.componentInstance.counter = 1;
       fixture.detectChanges(false);
-      expect(fixture.nativeElement).toHaveText('0|dynamic');
+      expect(fixture.nativeElement).toHaveText('0|dynamic|binding');
 
       // now mark the dynamically inserted component as dirty
       dynamicCmptRef.changeDetectorRef.markForCheck();
       fixture.detectChanges(false);
-      expect(fixture.nativeElement).toHaveText('1|dynamic');
+      expect(fixture.nativeElement).toHaveText('1|dynamic|binding');
+
+      // Update, mark for check, and detach before change detection, should not update
+      dynamicCmptRef.setInput('binding', 'updatedBinding');
+      dynamicCmptRef.changeDetectorRef.markForCheck();
+      dynamicCmptRef.changeDetectorRef.detach();
+      fixture.detectChanges(false);
+      expect(fixture.nativeElement).toHaveText('1|dynamic|binding');
+
+      // reattaching and run CD from the top should update
+      dynamicCmptRef.changeDetectorRef.reattach();
+      fixture.detectChanges(false);
+      expect(fixture.nativeElement).toHaveText('1|dynamic|updatedBinding');
+      ɵsetEnsureDirtyViewsAreAlwaysReachable(previous);
     });
 
     it('should support re-enterant change detection', () => {

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

@@ -533,6 +533,9 @@
   {
     "name": "_enable_super_gross_mode_that_will_cause_bad_things"
   },
+  {
+    "name": "_ensureDirtyViewsAreAlwaysReachable"
+  },
   {
     "name": "_findMatchingDehydratedViewImpl"
   },
@@ -1334,6 +1337,9 @@
   {
     "name": "init_fields"
   },
+  {
+    "name": "init_flags"
+  },
   {
     "name": "init_forward_ref"
   },

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

@@ -638,6 +638,9 @@
   {
     "name": "_enable_super_gross_mode_that_will_cause_bad_things"
   },
+  {
+    "name": "_ensureDirtyViewsAreAlwaysReachable"
+  },
   {
     "name": "_getInsertInFrontOfRNodeWithI18n"
   },

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

@@ -629,6 +629,9 @@
   {
     "name": "_enable_super_gross_mode_that_will_cause_bad_things"
   },
+  {
+    "name": "_ensureDirtyViewsAreAlwaysReachable"
+  },
   {
     "name": "_getInsertInFrontOfRNodeWithI18n"
   },

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

@@ -863,6 +863,9 @@
   {
     "name": "_enable_super_gross_mode_that_will_cause_bad_things"
   },
+  {
+    "name": "_ensureDirtyViewsAreAlwaysReachable"
+  },
   {
     "name": "_getInsertInFrontOfRNodeWithI18n"
   },

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

@@ -554,6 +554,9 @@
   {
     "name": "_enable_super_gross_mode_that_will_cause_bad_things"
   },
+  {
+    "name": "_ensureDirtyViewsAreAlwaysReachable"
+  },
   {
     "name": "_getInsertInFrontOfRNodeWithI18n"
   },

packages/core/test/linker/change_detection_integration_spec.ts

@@ -7,7 +7,7 @@
  */
 
 import {ResourceLoader} from '@angular/compiler';
-import {AfterContentChecked, AfterContentInit, AfterViewChecked, AfterViewInit, ChangeDetectionStrategy, ChangeDetectorRef, Component, ContentChild, DebugElement, Directive, DoCheck, EventEmitter, HostBinding, Injectable, Input, OnChanges, OnDestroy, OnInit, Output, Pipe, PipeTransform, Provider, RendererFactory2, RendererType2, SimpleChange, SimpleChanges, TemplateRef, Type, ViewChild, ViewContainerRef} from '@angular/core';
+import {AfterContentChecked, AfterContentInit, AfterViewChecked, AfterViewInit, ChangeDetectionStrategy, ChangeDetectorRef, Component, ContentChild, DebugElement, Directive, DoCheck, EventEmitter, HostBinding, Injectable, Input, OnChanges, OnDestroy, OnInit, Output, Pipe, PipeTransform, Provider, RendererFactory2, RendererType2, SimpleChange, SimpleChanges, TemplateRef, Type, ViewChild, ViewContainerRef, ɵgetEnsureDirtyViewsAreAlwaysReachable, ɵsetEnsureDirtyViewsAreAlwaysReachable} from '@angular/core';
 import {ComponentFixture, fakeAsync, TestBed} from '@angular/core/testing';
 import {By} from '@angular/platform-browser/src/dom/debug/by';
 import {isTextNode} from '@angular/platform-browser/testing/src/browser_util';
@@ -1135,16 +1135,15 @@ describe(`ChangeDetection`, () => {
          expect(() => ctx.checkNoChanges()).toThrowError(errMsgRegExp);
        }));
 
-    it('should warn when the view has been created in a cd hook', fakeAsync(() => {
-         const ctx = createCompFixture('<div *gh9882>{{ a }}</div>', TestData);
-         ctx.componentInstance.a = 1;
-         expect(() => ctx.detectChanges())
-             .toThrowError(
-                 /It seems like the view has been created after its parent and its children have been dirty checked/);
-
-         // subsequent change detection should run without issues
-         ctx.detectChanges();
-       }));
+    it('should allow view to be created in a cd hook', () => {
+      const previous = ɵgetEnsureDirtyViewsAreAlwaysReachable();
+      ɵsetEnsureDirtyViewsAreAlwaysReachable(true);
+      const ctx = createCompFixture('<div *gh9882>{{ a }}</div>', TestData);
+      ctx.componentInstance.a = 1;
+      ctx.detectChanges();
+      expect(ctx.nativeElement.innerText).toEqual('1');
+      ɵsetEnsureDirtyViewsAreAlwaysReachable(previous);
+    });
 
     it('should not throw when two arrays are structurally the same', fakeAsync(() => {
          const ctx = _bindSimpleValue('a', TestData);