fix(core): afterRender hooks now only run on ApplicationRef.tick (angular#52455)

The `afterRender` hooks currently run after `ApplicationRef.tick` but
also run after any call to `ChangeDetectorRef.detectChanges`. This is
problematic because code which uses `afterRender` cannot expect the
component it's registered from to be rendered when the callback
executes. If there is a call to `ChangeDetectorRef.detectChanges` before
the global change detection, that will cause the hooks to run earlier
than expected.

This behavior is somewhat of a blocker for the zoneless project. There
is plenty of application code that do things like `setTimeout(() =>
doSomethingThatExpectsComponentToBeRendered())`, `NgZone.onStable(() =>
...)` or `ApplicationRef.onStable...`. `ApplicationRef.onStable` is a
should likely work similarly, but all of these are really wanting an API
that is `afterRender` with the requirement that the hook runs after the
global render, not an individual CDRef instance.

This change updates the `afterRender` hooks to only run when
`ApplicationRef.tick` happens.

fixes angular#52429
fixes angular#53232

PR Close angular#52455

Author: atscott

goldens/public-api/core/errors.md

@@ -117,8 +117,6 @@ export const enum RuntimeErrorCode {
     // (undocumented)
     RECURSIVE_APPLICATION_REF_TICK = 101,
     // (undocumented)
-    RECURSIVE_APPLICATION_RENDER = 102,
-    // (undocumented)
     RENDERER_NOT_FOUND = 407,
     // (undocumented)
     REQUIRE_SYNC_WITHOUT_SYNC_EMIT = 601,

packages/core/src/application/application_ref.ts

@@ -29,6 +29,7 @@ import {NgModuleFactory, NgModuleRef} from '../linker/ng_module_factory';
 import {ViewRef} from '../linker/view_ref';
 import {isComponentResourceResolutionQueueEmpty, resolveComponentResources} from '../metadata/resource_loading';
 import {PendingTasks} from '../pending_tasks';
+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';
@@ -323,6 +324,7 @@ export class ApplicationRef {
   /** @internal */
   _views: InternalViewRef<unknown>[] = [];
   private readonly internalErrorHandler = inject(INTERNAL_APPLICATION_ERROR_HANDLER);
+  private readonly afterRenderEffectManager = inject(AfterRenderEventManager);
 
   /**
    * Indicates whether this instance was destroyed.
@@ -555,6 +557,18 @@ export class ApplicationRef {
       // Attention: Don't rethrow as it could cancel subscriptions to Observables!
       this.internalErrorHandler(e);
     } finally {
+      // Catch any `ExpressionChanged...` errors and report them to error handler like above
+      try {
+        const callbacksExecuted = this.afterRenderEffectManager.execute();
+        if ((typeof ngDevMode === 'undefined' || ngDevMode) && callbacksExecuted) {
+          for (let view of this._views) {
+            view.checkNoChanges();
+          }
+        }
+      } catch (e) {
+        this.internalErrorHandler(e);
+      }
+
       this._runningTick = false;
     }
   }

packages/core/src/defer/dom_triggers.ts

@@ -83,9 +83,6 @@ export function onInteraction(trigger: Element, callback: VoidFunction): VoidFun
     entry = new DeferEventEntry();
     interactionTriggers.set(trigger, entry);
 
-    // Ensure that the handler runs in the NgZone
-    ngDevMode && NgZone.assertInAngularZone();
-
     for (const name of interactionEventNames) {
       trigger.addEventListener(name, entry!.listener, eventListenerOptions);
     }
@@ -120,9 +117,6 @@ export function onHover(trigger: Element, callback: VoidFunction): VoidFunction
     entry = new DeferEventEntry();
     hoverTriggers.set(trigger, entry);
 
-    // Ensure that the handler runs in the NgZone
-    ngDevMode && NgZone.assertInAngularZone();
-
     for (const name of hoverEventNames) {
       trigger.addEventListener(name, entry!.listener, eventListenerOptions);
     }

packages/core/src/errors.ts

@@ -30,7 +30,6 @@ export const enum RuntimeErrorCode {
   // Change Detection Errors
   EXPRESSION_CHANGED_AFTER_CHECKED = -100,
   RECURSIVE_APPLICATION_REF_TICK = 101,
-  RECURSIVE_APPLICATION_RENDER = 102,
   INFINITE_CHANGE_DETECTION = 103,
 
   // Dependency Injection Errors

packages/core/src/render3/after_render_hooks.ts

@@ -349,13 +349,6 @@ class AfterRenderCallback {
  * Implements `afterRender` and `afterNextRender` callback handler logic.
  */
 interface AfterRenderCallbackHandler {
-  /**
-   * Validate that it's safe for a render operation to begin,
-   * throwing if not. Not guaranteed to be called if a render
-   * operation is started before handler was registered.
-   */
-  validateBegin(): void;
-
   /**
    * Register a new callback.
    */
@@ -367,9 +360,9 @@ interface AfterRenderCallbackHandler {
   unregister(callback: AfterRenderCallback): void;
 
   /**
-   * Execute callbacks.
+   * Execute callbacks. Returns `true` if any callbacks were executed.
    */
-  execute(): void;
+  execute(): boolean;
 
   /**
    * Perform any necessary cleanup.
@@ -392,16 +385,6 @@ class AfterRenderCallbackHandlerImpl implements AfterRenderCallbackHandler {
   };
   private deferredCallbacks = new Set<AfterRenderCallback>();
 
-  validateBegin(): void {
-    if (this.executingCallbacks) {
-      throw new RuntimeError(
-          RuntimeErrorCode.RECURSIVE_APPLICATION_RENDER,
-          ngDevMode &&
-              'A new render operation began before the previous operation ended. ' +
-                  'Did you trigger change detection from afterRender or afterNextRender?');
-    }
-  }
-
   register(callback: AfterRenderCallback): void {
     // If we're currently running callbacks, new callbacks should be deferred
     // until the next render operation.
@@ -414,10 +397,12 @@ class AfterRenderCallbackHandlerImpl implements AfterRenderCallbackHandler {
     this.deferredCallbacks.delete(callback);
   }
 
-  execute(): void {
+  execute(): boolean {
+    let callbacksExecuted = false;
     this.executingCallbacks = true;
     for (const bucket of Object.values(this.buckets)) {
       for (const callback of bucket) {
+        callbacksExecuted = true;
         callback.invoke();
       }
     }
@@ -427,6 +412,7 @@ class AfterRenderCallbackHandlerImpl implements AfterRenderCallbackHandler {
       this.buckets[callback.phase].add(callback);
     }
     this.deferredCallbacks.clear();
+    return callbacksExecuted;
   }
 
   destroy(): void {
@@ -442,41 +428,26 @@ class AfterRenderCallbackHandlerImpl implements AfterRenderCallbackHandler {
  * Delegates to an optional `AfterRenderCallbackHandler` for implementation.
  */
 export class AfterRenderEventManager {
-  private renderDepth = 0;
-
   /* @internal */
   handler: AfterRenderCallbackHandler|null = null;
 
   /* @internal */
   internalCallbacks: VoidFunction[] = [];
 
   /**
-   * Mark the beginning of a render operation (i.e. CD cycle).
-   * Throws if called while executing callbacks.
+   * Executes callbacks. Returns `true` if any callbacks executed.
    */
-  begin() {
-    this.handler?.validateBegin();
-    this.renderDepth++;
-  }
-
-  /**
-   * Mark the end of a render operation. Callbacks will be
-   * executed if there are no more pending operations.
-   */
-  end() {
-    ngDevMode && assertGreaterThan(this.renderDepth, 0, 'renderDepth must be greater than 0');
-    this.renderDepth--;
-
-    if (this.renderDepth === 0) {
-      // Note: internal callbacks power `internalAfterNextRender`. Since internal callbacks
-      // are fairly trivial, they are kept separate so that `AfterRenderCallbackHandlerImpl`
-      // can still be tree-shaken unless used by the application.
-      for (const callback of this.internalCallbacks) {
-        callback();
-      }
-      this.internalCallbacks.length = 0;
-      this.handler?.execute();
+  execute(): boolean {
+    // Note: internal callbacks power `internalAfterNextRender`. Since internal callbacks
+    // are fairly trivial, they are kept separate so that `AfterRenderCallbackHandlerImpl`
+    // can still be tree-shaken unless used by the application.
+    const callbacks = [...this.internalCallbacks];
+    this.internalCallbacks.length = 0;
+    for (const callback of callbacks) {
+      callback();
     }
+    const handlerCallbacksExecuted = this.handler?.execute();
+    return !!handlerCallbacksExecuted || callbacks.length > 0;
   }
 
   ngOnDestroy() {

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

@@ -30,7 +30,6 @@ const MAXIMUM_REFRESH_RERUNS = 100;
 export function detectChangesInternal(lView: LView, notifyErrorHandler = true) {
   const environment = lView[ENVIRONMENT];
   const rendererFactory = environment.rendererFactory;
-  const afterRenderEventManager = environment.afterRenderEventManager;
 
   // Check no changes mode is a dev only mode used to verify that bindings have not changed
   // since they were assigned. We do not want to invoke renderer factory functions in that mode
@@ -39,7 +38,6 @@ export function detectChangesInternal(lView: LView, notifyErrorHandler = true) {
 
   if (!checkNoChangesMode) {
     rendererFactory.begin?.();
-    afterRenderEventManager?.begin();
   }
 
   try {
@@ -56,9 +54,6 @@ export function detectChangesInternal(lView: LView, notifyErrorHandler = true) {
       // One final flush of the effects queue to catch any effects created in `ngAfterViewInit` or
       // other post-order hooks.
       environment.inlineEffectRunner?.flush();
-
-      // Invoke all callbacks registered via `after*Render`, if needed.
-      afterRenderEventManager?.end();
     }
   }
 }