Skip to content

Commit ac2dbe3

Browse files
AndrewKushnirkirjs
AndrewKushnir
authored and
kirjs
committed
Revert "fix(core): Defer afterRender until after first CD (#58250)" (#59455)
This reverts commit 9870b64. PR Close #59455
1 parent 21f1ba2 commit ac2dbe3

24 files changed

Lines changed: 138 additions & 198 deletions

File tree

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

Lines changed: 16 additions & 1 deletion
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';
4847
import {ApplicationInitStatus} from './application_init';
4948
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,6 +321,13 @@ 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+
324331
/**
325332
* Most recent snapshot from the `TracingService`, if any.
326333
*
@@ -640,6 +647,10 @@ export class ApplicationRef {
640647
this._rendererFactory = this._injector.get(RendererFactory2, null, {optional: true});
641648
}
642649

650+
// When beginning synchronization, all deferred dirtiness becomes active dirtiness.
651+
this.dirtyFlags |= this.deferredDirtyFlags;
652+
this.deferredDirtyFlags = ApplicationRefDirtyFlags.None;
653+
643654
let runs = 0;
644655
while (this.dirtyFlags !== ApplicationRefDirtyFlags.None && runs++ < MAXIMUM_REFRESH_RERUNS) {
645656
this.synchronizeOnce();
@@ -660,6 +671,10 @@ export class ApplicationRef {
660671
* Perform a single synchronization pass.
661672
*/
662673
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+
663678
// First, process any dirty root effects.
664679
if (this.dirtyFlags & ApplicationRefDirtyFlags.RootEffects) {
665680
this.dirtyFlags &= ~ApplicationRefDirtyFlags.RootEffects;

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ 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,
3637
// Views might be created outside and manipulated in ways that
3738
// we cannot be aware of. When a view is attached, Angular now "knows"
3839
// 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: 9 additions & 2 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-
PROVIDED_ZONELESS,
29-
SCHEDULE_IN_ROOT_ZONE,
3028
ZONELESS_ENABLED,
29+
PROVIDED_ZONELESS,
3130
ZONELESS_SCHEDULER_DISABLED,
31+
SCHEDULE_IN_ROOT_ZONE,
3232
} from './zoneless_scheduling';
3333
import {TracingService} from '../../application/tracing';
3434

@@ -140,6 +140,13 @@ 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+
}
143150
case NotificationSource.CustomElement: {
144151
// We use `ViewTreeTraversal` to ensure we refresh the element even if this is triggered
145152
// 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: 3 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,8 @@
66
* found in the LICENSE file at https://angular.dev/license
77
*/
88

9+
import {afterNextRender} from '../render3/after_render/hooks';
910
import type {Injector} from '../di';
10-
import {AfterRenderRef} from '../render3/after_render/api';
11-
import {afterRender} from '../render3/after_render/hooks';
1211
import {assertLContainer, assertLView} from '../render3/assert';
1312
import {CONTAINER_HEADER_OFFSET} from '../render3/interfaces/container';
1413
import {TNode} from '../render3/interfaces/node';
@@ -281,11 +280,9 @@ export function registerDomTrigger(
281280
) {
282281
const injector = initialLView[INJECTOR];
283282
const zone = injector.get(NgZone);
284-
let poll: AfterRenderRef;
285283
function pollDomTrigger() {
286284
// If the initial view was destroyed, we don't need to do anything.
287285
if (isDestroyed(initialLView)) {
288-
poll.destroy();
289286
return;
290287
}
291288

@@ -297,20 +294,17 @@ export function registerDomTrigger(
297294
renderedState !== DeferBlockInternalState.Initial &&
298295
renderedState !== DeferBlockState.Placeholder
299296
) {
300-
poll.destroy();
301297
return;
302298
}
303299

304300
const triggerLView = getTriggerLView(initialLView, tNode, walkUpTimes);
305301

306302
// Keep polling until we resolve the trigger's LView.
307303
if (!triggerLView) {
308-
// Keep polling.
304+
afterNextRender({read: pollDomTrigger}, {injector});
309305
return;
310306
}
311307

312-
poll.destroy();
313-
314308
// It's possible that the trigger's view was destroyed before we resolved the trigger element.
315309
if (isDestroyed(triggerLView)) {
316310
return;
@@ -345,5 +339,5 @@ export function registerDomTrigger(
345339
}
346340

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

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

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@ 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';
1716
import {AfterRenderPhase, AfterRenderRef} from './api';
1817
import {
1918
AfterRenderHooks,
@@ -460,11 +459,9 @@ function afterRenderImpl(
460459

461460
const hooks = options?.phase ?? AfterRenderPhase.MixedReadWrite;
462461
const destroyRef = options?.manualCleanup !== true ? injector.get(DestroyRef) : null;
463-
const viewContext = injector.get(ViewContext, null, {optional: true});
464462
const sequence = new AfterRenderSequence(
465463
manager.impl,
466464
getHooks(callbackOrSpec, hooks),
467-
viewContext?.view,
468465
once,
469466
destroyRef,
470467
tracing?.snapshot(null),

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

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

9-
import {TracingAction, TracingService, TracingSnapshot} from '../../application/tracing';
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';
1014
import {
1115
ChangeDetectionScheduler,
1216
NotificationSource,
1317
} 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';
1718
import {type DestroyRef} from '../../linker/destroy_ref';
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';
19+
import {TracingAction, TracingService, TracingSnapshot} from '../../application/tracing';
2220

2321
export class AfterRenderManager {
2422
impl: AfterRenderImpl | null = null;
@@ -104,34 +102,22 @@ export class AfterRenderImpl {
104102
this.sequences.add(sequence);
105103
}
106104
if (this.deferredRegistrations.size > 0) {
107-
this.scheduler.notify(NotificationSource.RenderHook);
105+
this.scheduler.notify(NotificationSource.DeferredRenderHook);
108106
}
109107
this.deferredRegistrations.clear();
110108
}
111109

112110
register(sequence: AfterRenderSequence): void {
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);
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);
123116
} else {
124117
this.deferredRegistrations.add(sequence);
125118
}
126119
}
127120

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-
135121
unregister(sequence: AfterRenderSequence): void {
136122
if (this.executing && this.sequences.has(sequence)) {
137123
// We can't remove an `AfterRenderSequence` in the middle of iteration.
@@ -186,7 +172,6 @@ export class AfterRenderSequence implements AfterRenderRef {
186172
constructor(
187173
readonly impl: AfterRenderImpl,
188174
readonly hooks: AfterRenderHooks,
189-
readonly view: LView | undefined,
190175
public once: boolean,
191176
destroyRef: DestroyRef | null,
192177
public snapshot: TracingSnapshot | null = null,
@@ -209,9 +194,5 @@ export class AfterRenderSequence implements AfterRenderRef {
209194
destroy(): void {
210195
this.impl.unregister(this);
211196
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-
}
216197
}
217198
}

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

Lines changed: 0 additions & 18 deletions
This file was deleted.

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

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

1818
import {RuntimeError, RuntimeErrorCode} from '../../errors';
1919
import {assertDefined, assertEqual} from '../../util/assert';
20-
import {addAfterRenderSequencesForView} from '../after_render/view';
2120
import {executeCheckHooks, executeInitAndCheckHooks, incrementInitPhaseFlags} from '../hooks';
2221
import {CONTAINER_HEADER_OFFSET, LContainerFlags, MOVED_VIEWS} from '../interfaces/container';
2322
import {ComponentTemplate, RenderFlags} from '../interfaces/definition';
@@ -34,8 +33,8 @@ import {
3433
TView,
3534
} from '../interfaces/view';
3635
import {
37-
getOrBorrowReactiveLViewConsumer,
3836
getOrCreateTemporaryConsumer,
37+
getOrBorrowReactiveLViewConsumer,
3938
maybeReturnReactiveLViewConsumer,
4039
ReactiveLViewConsumer,
4140
viewShouldHaveReactiveConsumer,
@@ -62,15 +61,15 @@ import {
6261
viewAttachedToChangeDetector,
6362
} from '../util/view_utils';
6463

65-
import {isDestroyed} from '../interfaces/type_checks';
66-
import {runEffectsInView} from '../reactivity/view_effect_runner';
6764
import {
6865
executeTemplate,
6966
executeViewQueryFn,
7067
handleError,
7168
processHostBindingOpCodes,
7269
refreshContentQueries,
7370
} from './shared';
71+
import {runEffectsInView} from '../reactivity/view_effect_runner';
72+
import {isDestroyed} from '../interfaces/type_checks';
7473

7574
/**
7675
* The maximum number of times the change detection traversal will rerun before throwing an error.
@@ -356,8 +355,6 @@ export function refreshView<T>(
356355
// no changes cycle, the component would be not be dirty for the next update pass. This would
357356
// be different in production mode where the component dirty state is not reset.
358357
if (!isInCheckNoChangesPass) {
359-
addAfterRenderSequencesForView(lView);
360-
361358
lView[FLAGS] &= ~(LViewFlags.Dirty | LViewFlags.FirstLViewPass);
362359
}
363360
} catch (e) {
@@ -504,9 +501,6 @@ function detectChangesInView(lView: LView, mode: ChangeDetectionMode) {
504501
if (components !== null) {
505502
detectChangesInChildComponents(lView, components, ChangeDetectionMode.Targeted);
506503
}
507-
if (!isInCheckNoChangesPass) {
508-
addAfterRenderSequencesForView(lView);
509-
}
510504
}
511505
}
512506

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

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@ 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';
1716
import type {ReactiveLViewConsumer} from '../reactive_lview_consumer';
1817
import type {ViewEffectNode} from '../reactivity/effect';
1918

@@ -68,7 +67,6 @@ export const ON_DESTROY_HOOKS = 21;
6867
export const EFFECTS_TO_SCHEDULE = 22;
6968
export const EFFECTS = 23;
7069
export const REACTIVE_TEMPLATE_CONSUMER = 24;
71-
export const AFTER_RENDER_SEQUENCES_TO_ADD = 25;
7270

7371
/**
7472
* Size of LView's header. Necessary to adjust for it when setting slots.
@@ -77,7 +75,7 @@ export const AFTER_RENDER_SEQUENCES_TO_ADD = 25;
7775
* instruction index into `LView` index. All other indexes should be in the `LView` index space and
7876
* there should be no need to refer to `HEADER_OFFSET` anywhere else.
7977
*/
80-
export const HEADER_OFFSET = 26;
78+
export const HEADER_OFFSET = 25;
8179

8280
// This interface replaces the real LView interface if it is an arg or a
8381
// return value of a public instruction. This ensures we don't need to expose
@@ -364,9 +362,6 @@ export interface LView<T = unknown> extends Array<any> {
364362
* if any signals were read.
365363
*/
366364
[REACTIVE_TEMPLATE_CONSUMER]: ReactiveLViewConsumer | null;
367-
368-
// AfterRenderSequences that need to be scheduled
369-
[AFTER_RENDER_SEQUENCES_TO_ADD]: AfterRenderSequence[] | null;
370365
}
371366

372367
/**

0 commit comments

Comments
 (0)