fix(core): Ensure views marked for check are refreshed during change …

…detection (angular#54735)

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.

fixes angular#52928
fixes angular#15634

BREAKING CHANGE: Newly created and views marked for check and reattached
during change detection are now guaranteed to be refreshed in that same
change detection cycle. Previously, if they were attached at a location
in the view tree that was already checked, they would either throw
`ExpressionChangedAfterItHasBeenCheckedError` or not be refreshed until
some future round of change detection. In rare circumstances, this
correction can cause issues. We identified one instance that relied on
the previous behavior by reading a value on initialization which was
queued to be updated in a microtask instead of being available in the
current change detection round. The component only read this value during
initialization and did not read it again after the microtask updated it.

PR Close angular#54735

packages/core/src/change_detection/flags.ts

@@ -6,8 +6,9 @@
  * found in the LICENSE file at https://angular.io/license
  */
 
-// TODO(atscott): flip default internally ASAP and externally for v18 (#52928)
-let _ensureDirtyViewsAreAlwaysReachable = false;
+// TODO(atscott): Remove prior to v18 release. Keeping this around in case anyone internally needs
+// to opt out temporarily.
+let _ensureDirtyViewsAreAlwaysReachable = true;
 
 export function getEnsureDirtyViewsAreAlwaysReachable() {
   return _ensureDirtyViewsAreAlwaysReachable;

packages/core/test/acceptance/change_detection_spec.ts

@@ -8,7 +8,7 @@
 
 
 import {CommonModule} from '@angular/common';
-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 {ApplicationRef, ChangeDetectionStrategy, ChangeDetectorRef, Component, ComponentRef, Directive, DoCheck, EmbeddedViewRef, ErrorHandler, EventEmitter, inject, Input, NgModule, OnInit, Output, QueryList, TemplateRef, Type, ViewChild, ViewChildren, ViewContainerRef} 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,8 +184,6 @@ 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>`,
@@ -245,7 +243,6 @@ describe('change detection', () => {
       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/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, ɵgetEnsureDirtyViewsAreAlwaysReachable, ɵsetEnsureDirtyViewsAreAlwaysReachable} 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} 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';
@@ -1136,13 +1136,10 @@ describe(`ChangeDetection`, () => {
        }));
 
     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(() => {