Skip to content

Commit 9870b64

Browse files
mmalerbathePunderWoman
mmalerba
authored and
thePunderWoman
committed
fix(core): Defer afterRender until after first CD (#58250)
For `afterRender`/`afterNextRender` calls associated with a particular view, ensure that they are not registered until after the first time the view is rendered. Co-authored-by: Alex Rickabaugh <alxhub@users.noreply.github.com> PR Close #58250
1 parent a25396d commit 9870b64

File tree

24 files changed

+198
-138
lines changed

24 files changed

+198
-138
lines changed

‎packages/core/src/application/application_ref.ts‎

Lines changed: 1 addition & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -44,9 +44,9 @@ import {TESTABILITY} from '../testability/testability';
4444
import {isPromise} from '../util/lang';
4545
import {NgZone} from '../zone/ng_zone';
4646

47+
import {EffectScheduler} from '../render3/reactivity/root_effect_scheduler';
4748
import {ApplicationInitStatus} from './application_init';
4849
import {TracingAction, TracingService, TracingSnapshot} from './tracing';
49-
import {EffectScheduler} from '../render3/reactivity/root_effect_scheduler';
5050

5151
/**
5252
* A DI token that provides a set of callbacks to
@@ -321,13 +321,6 @@ export class ApplicationRef {
321321
*/
322322
dirtyFlags = ApplicationRefDirtyFlags.None;
323323

324-
/**
325-
* Like `dirtyFlags` but don't cause `tick()` to loop.
326-
*
327-
* @internal
328-
*/
329-
deferredDirtyFlags = ApplicationRefDirtyFlags.None;
330-
331324
/**
332325
* Most recent snapshot from the `TracingService`, if any.
333326
*
@@ -647,10 +640,6 @@ export class ApplicationRef {
647640
this._rendererFactory = this._injector.get(RendererFactory2, null, {optional: true});
648641
}
649642

650-
// When beginning synchronization, all deferred dirtiness becomes active dirtiness.
651-
this.dirtyFlags |= this.deferredDirtyFlags;
652-
this.deferredDirtyFlags = ApplicationRefDirtyFlags.None;
653-
654643
let runs = 0;
655644
while (this.dirtyFlags !== ApplicationRefDirtyFlags.None && runs++ < MAXIMUM_REFRESH_RERUNS) {
656645
this.synchronizeOnce();
@@ -671,10 +660,6 @@ export class ApplicationRef {
671660
* Perform a single synchronization pass.
672661
*/
673662
private synchronizeOnce(): void {
674-
// If we happened to loop, deferred dirtiness can be processed as active dirtiness again.
675-
this.dirtyFlags |= this.deferredDirtyFlags;
676-
this.deferredDirtyFlags = ApplicationRefDirtyFlags.None;
677-
678663
// First, process any dirty root effects.
679664
if (this.dirtyFlags & ApplicationRefDirtyFlags.RootEffects) {
680665
this.dirtyFlags &= ~ApplicationRefDirtyFlags.RootEffects;

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

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,6 @@ export const enum NotificationSource {
3333
// but we should execute render hooks:
3434
// Render hooks are guaranteed to execute with the schedulers timing.
3535
RenderHook,
36-
DeferredRenderHook,
3736
// Views might be created outside and manipulated in ways that
3837
// we cannot be aware of. When a view is attached, Angular now "knows"
3938
// about it and we now know that DOM might have changed (and we should

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

Lines changed: 2 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -25,10 +25,10 @@ import {NgZone, NgZonePrivate, NoopNgZone, angularZoneInstanceIdProperty} from '
2525
import {
2626
ChangeDetectionScheduler,
2727
NotificationSource,
28-
ZONELESS_ENABLED,
2928
PROVIDED_ZONELESS,
30-
ZONELESS_SCHEDULER_DISABLED,
3129
SCHEDULE_IN_ROOT_ZONE,
30+
ZONELESS_ENABLED,
31+
ZONELESS_SCHEDULER_DISABLED,
3232
} from './zoneless_scheduling';
3333
import {TracingService} from '../../application/tracing';
3434

@@ -140,13 +140,6 @@ export class ChangeDetectionSchedulerImpl implements ChangeDetectionScheduler {
140140
this.appRef.dirtyFlags |= ApplicationRefDirtyFlags.ViewTreeCheck;
141141
break;
142142
}
143-
case NotificationSource.DeferredRenderHook: {
144-
// Render hooks are "deferred" when they're triggered from other render hooks. Using the
145-
// deferred dirty flags ensures that adding new hooks doesn't automatically trigger a loop
146-
// inside tick().
147-
this.appRef.deferredDirtyFlags |= ApplicationRefDirtyFlags.AfterRender;
148-
break;
149-
}
150143
case NotificationSource.CustomElement: {
151144
// We use `ViewTreeTraversal` to ensure we refresh the element even if this is triggered
152145
// during CD. In practice this is a no-op since the elements code also calls via a

‎packages/core/src/defer/dom_triggers.ts‎

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,9 @@
66
* found in the LICENSE file at https://angular.dev/license
77
*/
88

9-
import {afterNextRender} from '../render3/after_render/hooks';
109
import type {Injector} from '../di';
10+
import {AfterRenderRef} from '../render3/after_render/api';
11+
import {afterRender} from '../render3/after_render/hooks';
1112
import {assertLContainer, assertLView} from '../render3/assert';
1213
import {CONTAINER_HEADER_OFFSET} from '../render3/interfaces/container';
1314
import {TNode} from '../render3/interfaces/node';
@@ -280,9 +281,11 @@ export function registerDomTrigger(
280281
) {
281282
const injector = initialLView[INJECTOR];
282283
const zone = injector.get(NgZone);
284+
let poll: AfterRenderRef;
283285
function pollDomTrigger() {
284286
// If the initial view was destroyed, we don't need to do anything.
285287
if (isDestroyed(initialLView)) {
288+
poll.destroy();
286289
return;
287290
}
288291

@@ -294,17 +297,20 @@ export function registerDomTrigger(
294297
renderedState !== DeferBlockInternalState.Initial &&
295298
renderedState !== DeferBlockState.Placeholder
296299
) {
300+
poll.destroy();
297301
return;
298302
}
299303

300304
const triggerLView = getTriggerLView(initialLView, tNode, walkUpTimes);
301305

302306
// Keep polling until we resolve the trigger's LView.
303307
if (!triggerLView) {
304-
afterNextRender({read: pollDomTrigger}, {injector});
308+
// Keep polling.
305309
return;
306310
}
307311

312+
poll.destroy();
313+
308314
// It's possible that the trigger's view was destroyed before we resolved the trigger element.
309315
if (isDestroyed(triggerLView)) {
310316
return;
@@ -339,5 +345,5 @@ export function registerDomTrigger(
339345
}
340346

341347
// Begin polling for the trigger.
342-
afterNextRender({read: pollDomTrigger}, {injector});
348+
poll = afterRender({read: pollDomTrigger}, {injector});
343349
}

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import {inject} from '../../di/injector_compatibility';
1313
import {DestroyRef} from '../../linker/destroy_ref';
1414
import {performanceMarkFeature} from '../../util/performance';
1515
import {assertNotInReactiveContext} from '../reactivity/asserts';
16+
import {ViewContext} from '../view_context';
1617
import {AfterRenderPhase, AfterRenderRef} from './api';
1718
import {
1819
AfterRenderHooks,
@@ -459,9 +460,11 @@ function afterRenderImpl(
459460

460461
const hooks = options?.phase ?? AfterRenderPhase.MixedReadWrite;
461462
const destroyRef = options?.manualCleanup !== true ? injector.get(DestroyRef) : null;
463+
const viewContext = injector.get(ViewContext, null, {optional: true});
462464
const sequence = new AfterRenderSequence(
463465
manager.impl,
464466
getHooks(callbackOrSpec, hooks),
467+
viewContext?.view,
465468
once,
466469
destroyRef,
467470
tracing?.snapshot(null),

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

Lines changed: 31 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -6,17 +6,19 @@
66
* found in the LICENSE file at https://angular.dev/license
77
*/
88

9-
import {AfterRenderPhase, AfterRenderRef} from './api';
10-
import {NgZone} from '../../zone';
11-
import {inject} from '../../di/injector_compatibility';
12-
import {ɵɵdefineInjectable} from '../../di/interface/defs';
13-
import {ErrorHandler} from '../../error_handler';
9+
import {TracingAction, TracingService, TracingSnapshot} from '../../application/tracing';
1410
import {
1511
ChangeDetectionScheduler,
1612
NotificationSource,
1713
} from '../../change_detection/scheduling/zoneless_scheduling';
14+
import {inject} from '../../di/injector_compatibility';
15+
import {ɵɵdefineInjectable} from '../../di/interface/defs';
16+
import {ErrorHandler} from '../../error_handler';
1817
import {type DestroyRef} from '../../linker/destroy_ref';
19-
import {TracingAction, TracingService, TracingSnapshot} from '../../application/tracing';
18+
import {NgZone} from '../../zone';
19+
import {AFTER_RENDER_SEQUENCES_TO_ADD, FLAGS, LView, LViewFlags} from '../interfaces/view';
20+
import {markAncestorsForTraversal} from '../util/view_utils';
21+
import {AfterRenderPhase, AfterRenderRef} from './api';
2022

2123
export class AfterRenderManager {
2224
impl: AfterRenderImpl | null = null;
@@ -102,22 +104,34 @@ export class AfterRenderImpl {
102104
this.sequences.add(sequence);
103105
}
104106
if (this.deferredRegistrations.size > 0) {
105-
this.scheduler.notify(NotificationSource.DeferredRenderHook);
107+
this.scheduler.notify(NotificationSource.RenderHook);
106108
}
107109
this.deferredRegistrations.clear();
108110
}
109111

110112
register(sequence: AfterRenderSequence): void {
111-
if (!this.executing) {
112-
this.sequences.add(sequence);
113-
// Trigger an `ApplicationRef.tick()` if one is not already pending/running, because we have a
114-
// new render hook that needs to run.
115-
this.scheduler.notify(NotificationSource.RenderHook);
113+
const {view} = sequence;
114+
if (view !== undefined) {
115+
// Delay adding it to the manager, add it to the view instead.
116+
(view[AFTER_RENDER_SEQUENCES_TO_ADD] ??= []).push(sequence);
117+
118+
// Mark the view for traversal to ensure we eventually schedule the afterNextRender.
119+
markAncestorsForTraversal(view);
120+
view[FLAGS] |= LViewFlags.HasChildViewsToRefresh;
121+
} else if (!this.executing) {
122+
this.addSequence(sequence);
116123
} else {
117124
this.deferredRegistrations.add(sequence);
118125
}
119126
}
120127

128+
addSequence(sequence: AfterRenderSequence): void {
129+
this.sequences.add(sequence);
130+
// Trigger an `ApplicationRef.tick()` if one is not already pending/running, because we have a
131+
// new render hook that needs to run.
132+
this.scheduler.notify(NotificationSource.RenderHook);
133+
}
134+
121135
unregister(sequence: AfterRenderSequence): void {
122136
if (this.executing && this.sequences.has(sequence)) {
123137
// We can't remove an `AfterRenderSequence` in the middle of iteration.
@@ -172,6 +186,7 @@ export class AfterRenderSequence implements AfterRenderRef {
172186
constructor(
173187
readonly impl: AfterRenderImpl,
174188
readonly hooks: AfterRenderHooks,
189+
readonly view: LView | undefined,
175190
public once: boolean,
176191
destroyRef: DestroyRef | null,
177192
public snapshot: TracingSnapshot | null = null,
@@ -194,5 +209,9 @@ export class AfterRenderSequence implements AfterRenderRef {
194209
destroy(): void {
195210
this.impl.unregister(this);
196211
this.unregisterOnDestroy?.();
212+
const scheduled = this.view?.[AFTER_RENDER_SEQUENCES_TO_ADD];
213+
if (scheduled) {
214+
this.view[AFTER_RENDER_SEQUENCES_TO_ADD] = scheduled.filter((s) => s !== this);
215+
}
197216
}
198217
}

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

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
/**
2+
* @license
3+
* Copyright Google LLC All Rights Reserved.
4+
*
5+
* Use of this source code is governed by an MIT-style license that can be
6+
* found in the LICENSE file at https://angular.dev/license
7+
*/
8+
9+
import {AFTER_RENDER_SEQUENCES_TO_ADD, LView} from '../interfaces/view';
10+
11+
export function addAfterRenderSequencesForView(lView: LView) {
12+
if (lView[AFTER_RENDER_SEQUENCES_TO_ADD] !== null) {
13+
for (const sequence of lView[AFTER_RENDER_SEQUENCES_TO_ADD]) {
14+
sequence.impl.addSequence(sequence);
15+
}
16+
lView[AFTER_RENDER_SEQUENCES_TO_ADD].length = 0;
17+
}
18+
}

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

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import {
1717

1818
import {RuntimeError, RuntimeErrorCode} from '../../errors';
1919
import {assertDefined, assertEqual} from '../../util/assert';
20+
import {addAfterRenderSequencesForView} from '../after_render/view';
2021
import {executeCheckHooks, executeInitAndCheckHooks, incrementInitPhaseFlags} from '../hooks';
2122
import {CONTAINER_HEADER_OFFSET, LContainerFlags, MOVED_VIEWS} from '../interfaces/container';
2223
import {ComponentTemplate, RenderFlags} from '../interfaces/definition';
@@ -33,8 +34,8 @@ import {
3334
TView,
3435
} from '../interfaces/view';
3536
import {
36-
getOrCreateTemporaryConsumer,
3737
getOrBorrowReactiveLViewConsumer,
38+
getOrCreateTemporaryConsumer,
3839
maybeReturnReactiveLViewConsumer,
3940
ReactiveLViewConsumer,
4041
viewShouldHaveReactiveConsumer,
@@ -61,15 +62,15 @@ import {
6162
viewAttachedToChangeDetector,
6263
} from '../util/view_utils';
6364

65+
import {isDestroyed} from '../interfaces/type_checks';
66+
import {runEffectsInView} from '../reactivity/view_effect_runner';
6467
import {
6568
executeTemplate,
6669
executeViewQueryFn,
6770
handleError,
6871
processHostBindingOpCodes,
6972
refreshContentQueries,
7073
} from './shared';
71-
import {runEffectsInView} from '../reactivity/view_effect_runner';
72-
import {isDestroyed} from '../interfaces/type_checks';
7374

7475
/**
7576
* The maximum number of times the change detection traversal will rerun before throwing an error.
@@ -355,6 +356,8 @@ export function refreshView<T>(
355356
// no changes cycle, the component would be not be dirty for the next update pass. This would
356357
// be different in production mode where the component dirty state is not reset.
357358
if (!isInCheckNoChangesPass) {
359+
addAfterRenderSequencesForView(lView);
360+
358361
lView[FLAGS] &= ~(LViewFlags.Dirty | LViewFlags.FirstLViewPass);
359362
}
360363
} catch (e) {
@@ -501,6 +504,9 @@ function detectChangesInView(lView: LView, mode: ChangeDetectionMode) {
501504
if (components !== null) {
502505
detectChangesInChildComponents(lView, components, ChangeDetectionMode.Targeted);
503506
}
507+
if (!isInCheckNoChangesPass) {
508+
addAfterRenderSequencesForView(lView);
509+
}
504510
}
505511
}
506512

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

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import {ProviderToken} from '../../di/provider_token';
1313
import {DehydratedView} from '../../hydration/interfaces';
1414
import {SchemaMetadata} from '../../metadata/schema';
1515
import {Sanitizer} from '../../sanitization/sanitizer';
16+
import type {AfterRenderSequence} from '../after_render/manager';
1617
import type {ReactiveLViewConsumer} from '../reactive_lview_consumer';
1718
import type {ViewEffectNode} from '../reactivity/effect';
1819

@@ -67,6 +68,7 @@ export const ON_DESTROY_HOOKS = 21;
6768
export const EFFECTS_TO_SCHEDULE = 22;
6869
export const EFFECTS = 23;
6970
export const REACTIVE_TEMPLATE_CONSUMER = 24;
71+
export const AFTER_RENDER_SEQUENCES_TO_ADD = 25;
7072

7173
/**
7274
* Size of LView's header. Necessary to adjust for it when setting slots.
@@ -75,7 +77,7 @@ export const REACTIVE_TEMPLATE_CONSUMER = 24;
7577
* instruction index into `LView` index. All other indexes should be in the `LView` index space and
7678
* there should be no need to refer to `HEADER_OFFSET` anywhere else.
7779
*/
78-
export const HEADER_OFFSET = 25;
80+
export const HEADER_OFFSET = 26;
7981

8082
// This interface replaces the real LView interface if it is an arg or a
8183
// 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> {
362364
* if any signals were read.
363365
*/
364366
[REACTIVE_TEMPLATE_CONSUMER]: ReactiveLViewConsumer | null;
367+
368+
// AfterRenderSequences that need to be scheduled
369+
[AFTER_RENDER_SEQUENCES_TO_ADD]: AfterRenderSequence[] | null;
365370
}
366371

367372
/**

0 commit comments

Comments
 (0)