fix(core): afterRender hooks should allow updating state (#54074)

It was always the intent to have `afterRender` hooks allow updating
state, as long as those state updates specifically mark views for
(re)check. This commit delivers that behavior. Developers can now use
`afterRender` hooks to perform further updates within the same change
detection cycle rather than needing to defer this work to another round
(i.e. `queueMicrotask(() => <<updateState>>)`). This is an important
change to support migrating from the `queueMicrotask`-style deferred
updates, which are not entirely compatible with zoneless or event `NgZone` run
coalescing.

When using a microtask to update state after a render, it
doesn't work with coalescing because the render may not have happened
yet (coalescing and zoneless use `requestAnimationFrame` while the
default for `NgZone` is effectively a microtask-based change detection
scheduler). Second, when using a `microtask` _during_ change detection to defer
updates to the next cycle, this can cause updates to be split across
multiple frames with run coalescing and with zoneless (again, because of
`requestAnimationFrame`/`macrotask` change detection scheduling).

PR Close #54074

packages/core/src/application/application_ref.ts

@@ -33,9 +33,12 @@ import {AfterRenderEventManager} from '../render3/after_render_hooks';
 import {assertNgModuleType} from '../render3/assert';
 import {ComponentFactory as R3ComponentFactory} from '../render3/component_ref';
 import {isStandalone} from '../render3/definition';
+import {ChangeDetectionMode, detectChangesInternal, MAXIMUM_REFRESH_RERUNS} from '../render3/instructions/change_detection';
+import {FLAGS, LView, LViewFlags} from '../render3/interfaces/view';
 import {setJitOptions} from '../render3/jit/jit_options';
 import {NgModuleFactory as R3NgModuleFactory} from '../render3/ng_module_ref';
 import {publishDefaultGlobalUtils as _publishDefaultGlobalUtils} from '../render3/util/global_utils';
+import {requiresRefreshOrTraversal} from '../render3/util/view_utils';
 import {ViewRef as InternalViewRef} from '../render3/view_ref';
 import {TESTABILITY} from '../testability/testability';
 import {isPromise} from '../util/lang';
@@ -546,10 +549,9 @@ export class ApplicationRef {
 
     try {
       this._runningTick = true;
-      for (let view of this._views) {
-        view.detectChanges();
-      }
-      this.afterRenderEffectManager.execute();
+
+      this.detectChangesInAttachedViews();
+
       if ((typeof ngDevMode === 'undefined' || ngDevMode)) {
         for (let view of this._views) {
           view.checkNoChanges();
@@ -563,6 +565,53 @@ export class ApplicationRef {
     }
   }
 
+  private detectChangesInAttachedViews() {
+    let runs = 0;
+    do {
+      if (runs === MAXIMUM_REFRESH_RERUNS) {
+        throw new RuntimeError(
+            RuntimeErrorCode.INFINITE_CHANGE_DETECTION,
+            ngDevMode &&
+                'Changes in afterRender or afterNextRender hooks caused infinite change detection while refresh views.');
+      }
+
+      const isFirstPass = runs === 0;
+      for (let {_lView, notifyErrorHandler} of this._views) {
+        // When re-checking, only check views which actually need it.
+        if (!isFirstPass && !shouldRecheckView(_lView)) {
+          continue;
+        }
+        this.detectChangesInView(_lView, notifyErrorHandler, isFirstPass);
+      }
+      this.afterRenderEffectManager.execute();
+
+      runs++;
+    } while (this._views.some(({_lView}) => shouldRecheckView(_lView)));
+  }
+
+  private detectChangesInView(lView: LView, notifyErrorHandler: boolean, isFirstPass: boolean) {
+    let mode: ChangeDetectionMode;
+    if (isFirstPass) {
+      // The first pass is always in Global mode, which includes `CheckAlways` views.
+      mode = ChangeDetectionMode.Global;
+      // Add `RefreshView` flag to ensure this view is refreshed if not already dirty.
+      // `RefreshView` flag is used intentionally over `Dirty` because it gets cleared before
+      // executing any of the actual refresh code while the `Dirty` flag doesn't get cleared
+      // until the end of the refresh. Using `RefreshView` prevents creating a potential
+      // difference in the state of the LViewFlags during template execution.
+      lView[FLAGS] |= LViewFlags.RefreshView;
+    } else if (lView[FLAGS] & LViewFlags.Dirty) {
+      // The root view has been explicitly marked for check, so check it in Global mode.
+      mode = ChangeDetectionMode.Global;
+    } else {
+      // The view has not been marked for check, but contains a view marked for refresh
+      // (likely via a signal). Start this change detection in Targeted mode to skip the root
+      // view and check just the view(s) that need refreshed.
+      mode = ChangeDetectionMode.Targeted;
+    }
+    detectChangesInternal(lView, notifyErrorHandler, mode);
+  }
+
   /**
    * Attaches a view so that it will be dirty checked.
    * The view will be automatically detached when it is destroyed.
@@ -715,3 +764,8 @@ export function whenStable(applicationRef: ApplicationRef): Promise<void> {
 
   return whenStablePromise;
 }
+
+
+function shouldRecheckView(view: LView): boolean {
+  return requiresRefreshOrTraversal(view) || !!(view[FLAGS] & LViewFlags.Dirty);
+}

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

@@ -25,9 +25,10 @@ import {executeTemplate, executeViewQueryFn, handleError, processHostBindingOpCo
 /**
  * The maximum number of times the change detection traversal will rerun before throwing an error.
  */
-const MAXIMUM_REFRESH_RERUNS = 100;
+export const MAXIMUM_REFRESH_RERUNS = 100;
 
-export function detectChangesInternal(lView: LView, notifyErrorHandler = true) {
+export function detectChangesInternal(
+    lView: LView, notifyErrorHandler = true, mode = ChangeDetectionMode.Global) {
   const environment = lView[ENVIRONMENT];
   const rendererFactory = environment.rendererFactory;
 
@@ -41,7 +42,7 @@ export function detectChangesInternal(lView: LView, notifyErrorHandler = true) {
   }
 
   try {
-    detectChangesInViewWhileDirty(lView);
+    detectChangesInViewWhileDirty(lView, mode);
   } catch (error) {
     if (notifyErrorHandler) {
       handleError(lView, error);
@@ -58,8 +59,8 @@ export function detectChangesInternal(lView: LView, notifyErrorHandler = true) {
   }
 }
 
-function detectChangesInViewWhileDirty(lView: LView) {
-  detectChangesInView(lView, ChangeDetectionMode.Global);
+function detectChangesInViewWhileDirty(lView: LView, mode: ChangeDetectionMode) {
+  detectChangesInView(lView, mode);
 
   let retries = 0;
   // If after running change detection, this view still needs to be refreshed or there are
@@ -99,7 +100,7 @@ export function checkNoChangesInternal(lView: LView, notifyErrorHandler = true)
  * The change detection traversal algorithm switches between these modes based on various
  * conditions.
  */
-const enum ChangeDetectionMode {
+export const enum ChangeDetectionMode {
   /**
    * In `Global` mode, `Dirty` and `CheckAlways` views are refreshed as well as views with the
    * `RefreshView` flag.

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

@@ -197,8 +197,9 @@ export function walkUpViews(nestingLevel: number, currentView: LView): LView {
 }
 
 export function requiresRefreshOrTraversal(lView: LView) {
-  return lView[FLAGS] & (LViewFlags.RefreshView | LViewFlags.HasChildViewsToRefresh) ||
-      lView[REACTIVE_TEMPLATE_CONSUMER]?.dirty;
+  return !!(
+      lView[FLAGS] & (LViewFlags.RefreshView | LViewFlags.HasChildViewsToRefresh) ||
+      lView[REACTIVE_TEMPLATE_CONSUMER]?.dirty);
 }
 
 

packages/core/src/render3/view_ref.ts

@@ -57,7 +57,7 @@ export class ViewRef<T> implements EmbeddedViewRef<T>, ChangeDetectorRefInterfac
        *
        * This may be different from `_lView` if the `_cdRefInjectingView` is an embedded view.
        */
-      private _cdRefInjectingView?: LView, private readonly notifyErrorHandler = true) {}
+      private _cdRefInjectingView?: LView, readonly notifyErrorHandler = true) {}
 
   get context(): T {
     return this._lView[CONTEXT] as unknown as T;

packages/core/test/acceptance/after_render_hook_spec.ts

@@ -7,7 +7,7 @@
  */
 
 import {PLATFORM_BROWSER_ID, PLATFORM_SERVER_ID} from '@angular/common/src/platform_id';
-import {afterNextRender, afterRender, AfterRenderPhase, AfterRenderRef, ApplicationRef, ChangeDetectorRef, Component, computed, createComponent, effect, ErrorHandler, inject, Injector, NgZone, PLATFORM_ID, Type, ViewContainerRef, ɵinternalAfterNextRender as internalAfterNextRender} from '@angular/core';
+import {afterNextRender, afterRender, AfterRenderPhase, AfterRenderRef, ApplicationRef, ChangeDetectorRef, Component, computed, createComponent, effect, ErrorHandler, inject, Injector, NgZone, PLATFORM_ID, signal, Type, ViewContainerRef, ɵinternalAfterNextRender as internalAfterNextRender} from '@angular/core';
 import {NoopNgZone} from '@angular/core/src/zone/ng_zone';
 import {TestBed} from '@angular/core/testing';
 
@@ -894,6 +894,98 @@ describe('after render hooks', () => {
         ]);
       });
     });
+
+    it('allows writing to a signal in afterRender', () => {
+      const counter = signal(0);
+
+      @Component({
+        selector: 'test-component',
+        standalone: true,
+        template: ` {{counter()}} `,
+      })
+      class TestCmp {
+        counter = counter;
+        injector = inject(EnvironmentInjector);
+        ngOnInit() {
+          afterNextRender(() => {
+            this.counter.set(1);
+          }, {injector: this.injector});
+        }
+      }
+      TestBed.configureTestingModule(
+          {providers: [{provide: PLATFORM_ID, useValue: PLATFORM_BROWSER_ID}]});
+
+      const fixture = TestBed.createComponent(TestCmp);
+      const appRef = TestBed.inject(ApplicationRef);
+      appRef.attachView(fixture.componentRef.hostView);
+      appRef.tick();
+      expect(fixture.nativeElement.innerText).toBe('1');
+    });
+
+    it('allows updating state and calling markForCheck in afterRender', () => {
+      @Component({
+        selector: 'test-component',
+        standalone: true,
+        template: ` {{counter}} `,
+      })
+      class TestCmp {
+        counter = 0;
+        injector = inject(EnvironmentInjector);
+        cdr = inject(ChangeDetectorRef);
+        ngOnInit() {
+          afterNextRender(() => {
+            this.counter = 1;
+            this.cdr.markForCheck();
+          }, {injector: this.injector});
+        }
+      }
+      TestBed.configureTestingModule(
+          {providers: [{provide: PLATFORM_ID, useValue: PLATFORM_BROWSER_ID}]});
+
+      const fixture = TestBed.createComponent(TestCmp);
+      const appRef = TestBed.inject(ApplicationRef);
+      appRef.attachView(fixture.componentRef.hostView);
+      appRef.tick();
+      expect(fixture.nativeElement.innerText).toBe('1');
+    });
+
+    it('throws error when causing infinite updates', () => {
+      const counter = signal(0);
+
+      @Component({
+        selector: 'test-component',
+        standalone: true,
+        template: ` {{counter()}} `,
+      })
+      class TestCmp {
+        counter = counter;
+        injector = inject(EnvironmentInjector);
+        ngOnInit() {
+          afterRender(() => {
+            this.counter.update(v => v + 1);
+          }, {injector: this.injector});
+        }
+      }
+      TestBed.configureTestingModule({
+        providers: [
+          {provide: PLATFORM_ID, useValue: PLATFORM_BROWSER_ID},
+          {
+            provide: ErrorHandler, useClass: class extends ErrorHandler {
+              override handleError(error: any) {
+                throw error;
+              }
+            }
+          },
+        ]
+      });
+
+      const fixture = TestBed.createComponent(TestCmp);
+      const appRef = TestBed.inject(ApplicationRef);
+      appRef.attachView(fixture.componentRef.hostView);
+      expect(() => {
+        appRef.tick();
+      }).toThrowError(/NG0103.*(afterRender|afterNextRender)/);
+    });
   });
 
   describe('server', () => {

packages/core/test/acceptance/change_detection_signals_in_zones_spec.ts

@@ -887,36 +887,6 @@ describe('OnPush components with signals', () => {
     expect(fixture.nativeElement.innerText).toEqual('2');
   });
 
-  // Note: We probably don't want this to throw but need to decide how to handle reruns
-  // This asserts current behavior and should be updated when this is handled
-  it('throws error when writing to a signal in afterRender', () => {
-    const counter = signal(0);
-
-    @Component({
-      selector: 'test-component',
-      standalone: true,
-      template: ` {{counter()}} `,
-    })
-    class TestCmp {
-      counter = counter;
-      injector = inject(EnvironmentInjector);
-      ngOnInit() {
-        afterNextRender(() => {
-          this.counter.set(1);
-        }, {injector: this.injector});
-      }
-    }
-    TestBed.configureTestingModule(
-        {providers: [{provide: PLATFORM_ID, useValue: PLATFORM_BROWSER_ID}]});
-
-    const fixture = TestBed.createComponent(TestCmp);
-    const appRef = TestBed.inject(ApplicationRef);
-    appRef.attachView(fixture.componentRef.hostView);
-    const spy = spyOn(console, 'error');
-    appRef.tick();
-    expect(spy.calls.first().args[1]).toMatch(/ExpressionChanged/);
-  });
-
   it('destroys all signal consumers when destroying the view tree', () => {
     const val = signal(1);
     const double = computed(() => val() * 2);

packages/core/test/bundling/animations-standalone/bundle.golden_symbols.json

@@ -797,6 +797,9 @@
   {
     "name": "detectChangesInViewIfAttached"
   },
+  {
+    "name": "detectChangesInternal"
+  },
   {
     "name": "diPublicInInjector"
   },
@@ -1352,6 +1355,9 @@
   {
     "name": "shimStylesContent"
   },
+  {
+    "name": "shouldRecheckView"
+  },
   {
     "name": "shouldSearchParent"
   },

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

@@ -863,6 +863,9 @@
   {
     "name": "detectChangesInViewIfAttached"
   },
+  {
+    "name": "detectChangesInternal"
+  },
   {
     "name": "diPublicInInjector"
   },
@@ -1424,6 +1427,9 @@
   {
     "name": "shimStylesContent"
   },
+  {
+    "name": "shouldRecheckView"
+  },
   {
     "name": "shouldSearchParent"
   },

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

@@ -647,6 +647,9 @@
   {
     "name": "detectChangesInViewIfAttached"
   },
+  {
+    "name": "detectChangesInternal"
+  },
   {
     "name": "diPublicInInjector"
   },
@@ -1130,6 +1133,9 @@
   {
     "name": "shimStylesContent"
   },
+  {
+    "name": "shouldRecheckView"
+  },
   {
     "name": "shouldSearchParent"
   },

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

@@ -740,6 +740,9 @@
   {
     "name": "detectChangesInViewIfAttached"
   },
+  {
+    "name": "detectChangesInternal"
+  },
   {
     "name": "diPublicInInjector"
   },
@@ -2255,6 +2258,9 @@
   {
     "name": "shimStylesContent"
   },
+  {
+    "name": "shouldRecheckView"
+  },
   {
     "name": "shouldSearchParent"
   },

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

@@ -905,6 +905,9 @@
   {
     "name": "detectChangesInViewIfAttached"
   },
+  {
+    "name": "detectChangesInternal"
+  },
   {
     "name": "diPublicInInjector"
   },
@@ -1661,6 +1664,9 @@
   {
     "name": "shouldAddViewToDom"
   },
+  {
+    "name": "shouldRecheckView"
+  },
   {
     "name": "shouldSearchParent"
   },