fix(core): Defer afterRender until after first CD (#59455) (#59551)

This reverts commit ac2dbe3.

PR Close #59551

Author: mmalerba

packages/core/src/application/application_ref.ts

@@ -299,13 +299,6 @@ export class ApplicationRef {
    */
   dirtyFlags = ApplicationRefDirtyFlags.None;
 
-  /**
-   * Like `dirtyFlags` but don't cause `tick()` to loop.
-   *
-   * @internal
-   */
-  deferredDirtyFlags = ApplicationRefDirtyFlags.None;
-
   /**
    * Most recent snapshot from the `TracingService`, if any.
    *
@@ -634,10 +627,6 @@ export class ApplicationRef {
       this._rendererFactory = this._injector.get(RendererFactory2, null, {optional: true});
     }
 
-    // When beginning synchronization, all deferred dirtiness becomes active dirtiness.
-    this.dirtyFlags |= this.deferredDirtyFlags;
-    this.deferredDirtyFlags = ApplicationRefDirtyFlags.None;
-
     let runs = 0;
     while (this.dirtyFlags !== ApplicationRefDirtyFlags.None && runs++ < MAXIMUM_REFRESH_RERUNS) {
       profiler(ProfilerEvent.ChangeDetectionSyncStart);
@@ -660,10 +649,6 @@ export class ApplicationRef {
    * Perform a single synchronization pass.
    */
   private synchronizeOnce(): void {
-    // If we happened to loop, deferred dirtiness can be processed as active dirtiness again.
-    this.dirtyFlags |= this.deferredDirtyFlags;
-    this.deferredDirtyFlags = ApplicationRefDirtyFlags.None;
-
     // First, process any dirty root effects.
     if (this.dirtyFlags & ApplicationRefDirtyFlags.RootEffects) {
       this.dirtyFlags &= ~ApplicationRefDirtyFlags.RootEffects;

packages/core/src/change_detection/scheduling/zoneless_scheduling.ts

@@ -33,7 +33,6 @@ export const enum NotificationSource {
   // but we should execute render hooks:
   // Render hooks are guaranteed to execute with the schedulers timing.
   RenderHook,
-  DeferredRenderHook,
   // Views might be created outside and manipulated in ways that
   // we cannot be aware of. When a view is attached, Angular now "knows"
   // about it and we now know that DOM might have changed (and we should

packages/core/src/change_detection/scheduling/zoneless_scheduling_impl.ts

@@ -25,10 +25,10 @@ import {NgZone, NgZonePrivate, NoopNgZone, angularZoneInstanceIdProperty} from '
 import {
   ChangeDetectionScheduler,
   NotificationSource,
-  ZONELESS_ENABLED,
   PROVIDED_ZONELESS,
-  ZONELESS_SCHEDULER_DISABLED,
   SCHEDULE_IN_ROOT_ZONE,
+  ZONELESS_ENABLED,
+  ZONELESS_SCHEDULER_DISABLED,
 } from './zoneless_scheduling';
 import {TracingService} from '../../application/tracing';
 
@@ -140,13 +140,6 @@ export class ChangeDetectionSchedulerImpl implements ChangeDetectionScheduler {
         this.appRef.dirtyFlags |= ApplicationRefDirtyFlags.ViewTreeCheck;
         break;
       }
-      case NotificationSource.DeferredRenderHook: {
-        // Render hooks are "deferred" when they're triggered from other render hooks. Using the
-        // deferred dirty flags ensures that adding new hooks doesn't automatically trigger a loop
-        // inside tick().
-        this.appRef.deferredDirtyFlags |= ApplicationRefDirtyFlags.AfterRender;
-        break;
-      }
       case NotificationSource.CustomElement: {
         // We use `ViewTreeTraversal` to ensure we refresh the element even if this is triggered
         // during CD. In practice this is a no-op since the elements code also calls via a

packages/core/src/defer/dom_triggers.ts

@@ -6,8 +6,9 @@
  * found in the LICENSE file at https://angular.dev/license
  */
 
-import {afterNextRender} from '../render3/after_render/hooks';
 import type {Injector} from '../di';
+import {AfterRenderRef} from '../render3/after_render/api';
+import {afterRender} from '../render3/after_render/hooks';
 import {assertLContainer, assertLView} from '../render3/assert';
 import {CONTAINER_HEADER_OFFSET} from '../render3/interfaces/container';
 import {TNode} from '../render3/interfaces/node';
@@ -280,9 +281,11 @@ export function registerDomTrigger(
 ) {
   const injector = initialLView[INJECTOR];
   const zone = injector.get(NgZone);
+  let poll: AfterRenderRef;
   function pollDomTrigger() {
     // If the initial view was destroyed, we don't need to do anything.
     if (isDestroyed(initialLView)) {
+      poll.destroy();
       return;
     }
 
@@ -294,17 +297,20 @@ export function registerDomTrigger(
       renderedState !== DeferBlockInternalState.Initial &&
       renderedState !== DeferBlockState.Placeholder
     ) {
+      poll.destroy();
       return;
     }
 
     const triggerLView = getTriggerLView(initialLView, tNode, walkUpTimes);
 
     // Keep polling until we resolve the trigger's LView.
     if (!triggerLView) {
-      afterNextRender({read: pollDomTrigger}, {injector});
+      // Keep polling.
       return;
     }
 
+    poll.destroy();
+
     // It's possible that the trigger's view was destroyed before we resolved the trigger element.
     if (isDestroyed(triggerLView)) {
       return;
@@ -339,5 +345,5 @@ export function registerDomTrigger(
   }
 
   // Begin polling for the trigger.
-  afterNextRender({read: pollDomTrigger}, {injector});
+  poll = afterRender({read: pollDomTrigger}, {injector});
 }

packages/core/src/render3/after_render/hooks.ts

@@ -13,6 +13,7 @@ import {inject} from '../../di/injector_compatibility';
 import {DestroyRef} from '../../linker/destroy_ref';
 import {performanceMarkFeature} from '../../util/performance';
 import {assertNotInReactiveContext} from '../reactivity/asserts';
+import {ViewContext} from '../view_context';
 import {AfterRenderPhase, AfterRenderRef} from './api';
 import {
   AfterRenderHooks,
@@ -459,9 +460,11 @@ function afterRenderImpl(
 
   const hooks = options?.phase ?? AfterRenderPhase.MixedReadWrite;
   const destroyRef = options?.manualCleanup !== true ? injector.get(DestroyRef) : null;
+  const viewContext = injector.get(ViewContext, null, {optional: true});
   const sequence = new AfterRenderSequence(
     manager.impl,
     getHooks(callbackOrSpec, hooks),
+    viewContext?.view,
     once,
     destroyRef,
     tracing?.snapshot(null),

packages/core/src/render3/after_render/manager.ts

@@ -6,19 +6,21 @@
  * found in the LICENSE file at https://angular.dev/license
  */
 
-import {AfterRenderPhase, AfterRenderRef} from './api';
-import {NgZone} from '../../zone';
-import {inject} from '../../di/injector_compatibility';
-import {ɵɵdefineInjectable} from '../../di/interface/defs';
-import {ErrorHandler} from '../../error_handler';
+import {TracingAction, TracingService, TracingSnapshot} from '../../application/tracing';
 import {
   ChangeDetectionScheduler,
   NotificationSource,
 } from '../../change_detection/scheduling/zoneless_scheduling';
+import {inject} from '../../di/injector_compatibility';
+import {ɵɵdefineInjectable} from '../../di/interface/defs';
+import {ErrorHandler} from '../../error_handler';
 import {type DestroyRef} from '../../linker/destroy_ref';
-import {TracingAction, TracingService, TracingSnapshot} from '../../application/tracing';
+import {NgZone} from '../../zone';
+import {AFTER_RENDER_SEQUENCES_TO_ADD, FLAGS, LView, LViewFlags} from '../interfaces/view';
 import {profiler} from '../profiler';
 import {ProfilerEvent} from '../profiler_types';
+import {markAncestorsForTraversal} from '../util/view_utils';
+import {AfterRenderPhase, AfterRenderRef} from './api';
 
 export class AfterRenderManager {
   impl: AfterRenderImpl | null = null;
@@ -111,7 +113,7 @@ export class AfterRenderImpl {
       this.sequences.add(sequence);
     }
     if (this.deferredRegistrations.size > 0) {
-      this.scheduler.notify(NotificationSource.DeferredRenderHook);
+      this.scheduler.notify(NotificationSource.RenderHook);
     }
     this.deferredRegistrations.clear();
 
@@ -121,16 +123,28 @@ export class AfterRenderImpl {
   }
 
   register(sequence: AfterRenderSequence): void {
-    if (!this.executing) {
-      this.sequences.add(sequence);
-      // Trigger an `ApplicationRef.tick()` if one is not already pending/running, because we have a
-      // new render hook that needs to run.
-      this.scheduler.notify(NotificationSource.RenderHook);
+    const {view} = sequence;
+    if (view !== undefined) {
+      // Delay adding it to the manager, add it to the view instead.
+      (view[AFTER_RENDER_SEQUENCES_TO_ADD] ??= []).push(sequence);
+
+      // Mark the view for traversal to ensure we eventually schedule the afterNextRender.
+      markAncestorsForTraversal(view);
+      view[FLAGS] |= LViewFlags.HasChildViewsToRefresh;
+    } else if (!this.executing) {
+      this.addSequence(sequence);
     } else {
       this.deferredRegistrations.add(sequence);
     }
   }
 
+  addSequence(sequence: AfterRenderSequence): void {
+    this.sequences.add(sequence);
+    // Trigger an `ApplicationRef.tick()` if one is not already pending/running, because we have a
+    // new render hook that needs to run.
+    this.scheduler.notify(NotificationSource.RenderHook);
+  }
+
   unregister(sequence: AfterRenderSequence): void {
     if (this.executing && this.sequences.has(sequence)) {
       // We can't remove an `AfterRenderSequence` in the middle of iteration.
@@ -185,6 +199,7 @@ export class AfterRenderSequence implements AfterRenderRef {
   constructor(
     readonly impl: AfterRenderImpl,
     readonly hooks: AfterRenderHooks,
+    readonly view: LView | undefined,
     public once: boolean,
     destroyRef: DestroyRef | null,
     public snapshot: TracingSnapshot | null = null,
@@ -207,5 +222,9 @@ export class AfterRenderSequence implements AfterRenderRef {
   destroy(): void {
     this.impl.unregister(this);
     this.unregisterOnDestroy?.();
+    const scheduled = this.view?.[AFTER_RENDER_SEQUENCES_TO_ADD];
+    if (scheduled) {
+      this.view[AFTER_RENDER_SEQUENCES_TO_ADD] = scheduled.filter((s) => s !== this);
+    }
   }
 }

packages/core/src/render3/after_render/view.ts

@@ -0,0 +1,18 @@
+/**
+ * @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.dev/license
+ */
+
+import {AFTER_RENDER_SEQUENCES_TO_ADD, LView} from '../interfaces/view';
+
+export function addAfterRenderSequencesForView(lView: LView) {
+  if (lView[AFTER_RENDER_SEQUENCES_TO_ADD] !== null) {
+    for (const sequence of lView[AFTER_RENDER_SEQUENCES_TO_ADD]) {
+      sequence.impl.addSequence(sequence);
+    }
+    lView[AFTER_RENDER_SEQUENCES_TO_ADD].length = 0;
+  }
+}

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

@@ -17,6 +17,7 @@ import {
 
 import {RuntimeError, RuntimeErrorCode} from '../../errors';
 import {assertDefined, assertEqual} from '../../util/assert';
+import {addAfterRenderSequencesForView} from '../after_render/view';
 import {executeCheckHooks, executeInitAndCheckHooks, incrementInitPhaseFlags} from '../hooks';
 import {CONTAINER_HEADER_OFFSET, LContainerFlags, MOVED_VIEWS} from '../interfaces/container';
 import {ComponentTemplate, HostBindingsFunction, RenderFlags} from '../interfaces/definition';
@@ -33,8 +34,8 @@ import {
   TView,
 } from '../interfaces/view';
 import {
-  getOrCreateTemporaryConsumer,
   getOrBorrowReactiveLViewConsumer,
+  getOrCreateTemporaryConsumer,
   maybeReturnReactiveLViewConsumer,
   ReactiveLViewConsumer,
   viewShouldHaveReactiveConsumer,
@@ -64,11 +65,11 @@ import {
 } from '../util/view_utils';
 
 import {isDestroyed} from '../interfaces/type_checks';
-import {ProfilerEvent} from '../profiler_types';
 import {profiler} from '../profiler';
+import {ProfilerEvent} from '../profiler_types';
+import {executeViewQueryFn, refreshContentQueries} from '../queries/query_execution';
 import {runEffectsInView} from '../reactivity/view_effect_runner';
 import {executeTemplate, handleError} from './shared';
-import {executeViewQueryFn, refreshContentQueries} from '../queries/query_execution';
 
 /**
  * The maximum number of times the change detection traversal will rerun before throwing an error.
@@ -354,6 +355,8 @@ export function refreshView<T>(
     // no changes cycle, the component would be not be dirty for the next update pass. This would
     // be different in production mode where the component dirty state is not reset.
     if (!isInCheckNoChangesPass) {
+      addAfterRenderSequencesForView(lView);
+
       lView[FLAGS] &= ~(LViewFlags.Dirty | LViewFlags.FirstLViewPass);
     }
   } catch (e) {
@@ -506,6 +509,9 @@ function detectChangesInView(lView: LView, mode: ChangeDetectionMode) {
     if (components !== null) {
       detectChangesInChildComponents(lView, components, ChangeDetectionMode.Targeted);
     }
+    if (!isInCheckNoChangesPass) {
+      addAfterRenderSequencesForView(lView);
+    }
   }
 }
 

packages/core/src/render3/interfaces/view.ts

@@ -13,6 +13,7 @@ import {ProviderToken} from '../../di/provider_token';
 import {DehydratedView} from '../../hydration/interfaces';
 import {SchemaMetadata} from '../../metadata/schema';
 import {Sanitizer} from '../../sanitization/sanitizer';
+import type {AfterRenderSequence} from '../after_render/manager';
 import type {ReactiveLViewConsumer} from '../reactive_lview_consumer';
 import type {ViewEffectNode} from '../reactivity/effect';
 
@@ -67,6 +68,7 @@ export const ON_DESTROY_HOOKS = 21;
 export const EFFECTS_TO_SCHEDULE = 22;
 export const EFFECTS = 23;
 export const REACTIVE_TEMPLATE_CONSUMER = 24;
+export const AFTER_RENDER_SEQUENCES_TO_ADD = 25;
 
 /**
  * Size of LView's header. Necessary to adjust for it when setting slots.
@@ -75,7 +77,7 @@ export const REACTIVE_TEMPLATE_CONSUMER = 24;
  * instruction index into `LView` index. All other indexes should be in the `LView` index space and
  * there should be no need to refer to `HEADER_OFFSET` anywhere else.
  */
-export const HEADER_OFFSET = 25;
+export const HEADER_OFFSET = 26;
 
 // This interface replaces the real LView interface if it is an arg or a
 // return value of a public instruction. This ensures we don't need to expose
@@ -362,6 +364,9 @@ export interface LView<T = unknown> extends Array<any> {
    * if any signals were read.
    */
   [REACTIVE_TEMPLATE_CONSUMER]: ReactiveLViewConsumer | null;
+
+  // AfterRenderSequences that need to be scheduled
+  [AFTER_RENDER_SEQUENCES_TO_ADD]: AfterRenderSequence[] | null;
 }
 
 /**

packages/core/src/render3/reactivity/after_render_effect.ts

@@ -19,24 +19,26 @@ import {
 import {type Signal} from '../reactivity/api';
 import {type EffectCleanupFn, type EffectCleanupRegisterFn} from './effect';
 
+import {TracingService, TracingSnapshot} from '../../application/tracing';
 import {
   ChangeDetectionScheduler,
   NotificationSource,
 } from '../../change_detection/scheduling/zoneless_scheduling';
+import {assertInInjectionContext} from '../../di/contextual';
 import {Injector} from '../../di/injector';
 import {inject} from '../../di/injector_compatibility';
+import {DestroyRef} from '../../linker/destroy_ref';
+import {AfterRenderPhase, type AfterRenderRef} from '../after_render/api';
+import {NOOP_AFTER_RENDER_REF, type AfterRenderOptions} from '../after_render/hooks';
 import {
   AFTER_RENDER_PHASES,
   AfterRenderImpl,
   AfterRenderManager,
   AfterRenderSequence,
 } from '../after_render/manager';
-import {AfterRenderPhase, type AfterRenderRef} from '../after_render/api';
-import {NOOP_AFTER_RENDER_REF, type AfterRenderOptions} from '../after_render/hooks';
-import {DestroyRef} from '../../linker/destroy_ref';
+import {LView} from '../interfaces/view';
+import {ViewContext} from '../view_context';
 import {assertNotInReactiveContext} from './asserts';
-import {assertInInjectionContext} from '../../di/contextual';
-import {TracingService, TracingSnapshot} from '../../application/tracing';
 
 const NOT_SET = Symbol('NOT_SET');
 const EMPTY_CLEANUP_SET = new Set<() => void>();
@@ -174,13 +176,14 @@ class AfterRenderEffectSequence extends AfterRenderSequence {
   constructor(
     impl: AfterRenderImpl,
     effectHooks: Array<AfterRenderPhaseEffectHook | undefined>,
+    view: LView | undefined,
     readonly scheduler: ChangeDetectionScheduler,
     destroyRef: DestroyRef,
     snapshot: TracingSnapshot | null = null,
   ) {
     // Note that we also initialize the underlying `AfterRenderSequence` hooks to `undefined` and
     // populate them as we create reactive nodes below.
-    super(impl, [undefined, undefined, undefined, undefined], false, destroyRef, snapshot);
+    super(impl, [undefined, undefined, undefined, undefined], view, false, destroyRef, snapshot);
 
     // Setup a reactive node for each phase.
     for (const phase of AFTER_RENDER_PHASES) {
@@ -380,9 +383,12 @@ export function afterRenderEffect<E = never, W = never, M = never>(
     spec = {mixedReadWrite: callbackOrSpec as any};
   }
 
+  const viewContext = injector.get(ViewContext, null, {optional: true});
+
   const sequence = new AfterRenderEffectSequence(
     manager.impl,
     [spec.earlyRead, spec.write, spec.mixedReadWrite, spec.read] as AfterRenderPhaseEffectHook[],
+    viewContext?.view,
     scheduler,
     injector.get(DestroyRef),
     tracing?.snapshot(null),