Skip to content

Commit 38c9f08

Browse files
alxhubAndrewKushnir
authored andcommitted
refactor(core): decouple effects from change detection (#51049)
Previously effects were queued as they became dirty, and this queue was flushed at various checkpoints during the change detection cycle. The result was that change detection _was_ the effect runner, and without executing CD, effects would not execute. This leads a particular tradeoff: * effects are subject to unidirectional data flow (bad for dx) * effects don't cause a new round of CD (good/bad depending on use case) * effects can be used to implement control flow efficiently (desirable) This commit changes the scheduling mechanism. Effects are now scheduled via the microtask queue. This changes the tradeoffs: * effects are no longer limited by unidirectional data flow (easy dx) * effects registered in the Angular zone will trigger CD after they run (same as `Promise.resolve` really) * the public `effect()` type of effect probably isn't a good building block for our built-in control flow, and we'll need a new internal abstraction. As `effect()` is in developer preview, changing the execution timing is not considered breaking even though it may impact current users. PR Close #51049
1 parent e86d6db commit 38c9f08

27 files changed

Lines changed: 363 additions & 407 deletions

File tree

goldens/public-api/core/testing/index.md

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import { PlatformRef } from '@angular/core';
2020
import { ProviderToken } from '@angular/core';
2121
import { SchemaMetadata } from '@angular/core';
2222
import { Type } from '@angular/core';
23+
import { ɵFlushableEffectRunner } from '@angular/core';
2324

2425
// @public
2526
export const __core_private_testing_placeholder__ = "";
@@ -29,7 +30,7 @@ export function async(fn: Function): (done: any) => any;
2930

3031
// @public
3132
export class ComponentFixture<T> {
32-
constructor(componentRef: ComponentRef<T>, ngZone: NgZone | null, _autoDetect: boolean);
33+
constructor(componentRef: ComponentRef<T>, ngZone: NgZone | null, effectRunner: ɵFlushableEffectRunner | null, _autoDetect: boolean);
3334
autoDetectChanges(autoDetect?: boolean): void;
3435
changeDetectorRef: ChangeDetectorRef;
3536
checkNoChanges(): void;
@@ -110,6 +111,7 @@ export interface TestBed {
110111
createComponent<T>(component: Type<T>): ComponentFixture<T>;
111112
// (undocumented)
112113
execute(tokens: any[], fn: Function, context?: any): any;
114+
flushEffects(): void;
113115
// @deprecated (undocumented)
114116
get<T>(token: ProviderToken<T>, notFoundValue?: T, flags?: InjectFlags): any;
115117
// @deprecated (undocumented)

packages/core/src/core_reactivity_export_internal.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,5 +23,8 @@ export {
2323
effect,
2424
EffectRef,
2525
EffectCleanupFn,
26+
EffectScheduler as ɵEffectScheduler,
27+
ZoneAwareQueueingScheduler as ɵZoneAwareQueueingScheduler,
28+
FlushableEffectRunner as ɵFlushableEffectRunner,
2629
} from './render3/reactivity/effect';
27-
// clang-format on
30+
// clang-format on

packages/core/src/render3/component_ref.ts

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ import {CONTEXT, HEADER_OFFSET, INJECTOR, LView, LViewEnvironment, LViewFlags, T
4444
import {MATH_ML_NAMESPACE, SVG_NAMESPACE} from './namespaces';
4545
import {createElementNode, setupStaticAttributes, writeDirectClass} from './node_manipulation';
4646
import {extractAttrsAndClassesFromSelector, stringifyCSSSelectorList} from './node_selector_matcher';
47-
import {EffectManager} from './reactivity/effect';
47+
import {EffectScheduler} from './reactivity/effect';
4848
import {enterView, getCurrentTNode, getLView, leaveView} from './state';
4949
import {computeStaticStyling} from './styling/static_styling';
5050
import {mergeHostAttrs, setUpAttributes} from './util/attrs_utils';
@@ -188,14 +188,13 @@ export class ComponentFactory<T> extends AbstractComponentFactory<T> {
188188
}
189189
const sanitizer = rootViewInjector.get(Sanitizer, null);
190190

191-
const effectManager = rootViewInjector.get(EffectManager, null);
192-
193191
const afterRenderEventManager = rootViewInjector.get(AfterRenderEventManager, null);
194192

195193
const environment: LViewEnvironment = {
196194
rendererFactory,
197195
sanitizer,
198-
effectManager,
196+
// We don't use inline effects (yet).
197+
inlineEffectRunner: null,
199198
afterRenderEventManager,
200199
};
201200

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ export function detectChangesInternal<T>(
4848

4949
// One final flush of the effects queue to catch any effects created in `ngAfterViewInit` or
5050
// other post-order hooks.
51-
environment.effectManager?.flush();
51+
environment.inlineEffectRunner?.flush();
5252

5353
// Invoke all callbacks registered via `after*Render`, if needed.
5454
afterRenderEventManager?.end();
@@ -117,7 +117,7 @@ export function refreshView<T>(
117117
// since they were assigned. We do not want to execute lifecycle hooks in that mode.
118118
const isInCheckNoChangesPass = ngDevMode && isInCheckNoChangesMode();
119119

120-
!isInCheckNoChangesPass && lView[ENVIRONMENT].effectManager?.flush();
120+
!isInCheckNoChangesPass && lView[ENVIRONMENT].inlineEffectRunner?.flush();
121121

122122
enterView(lView);
123123
try {

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ import {DehydratedView} from '../../hydration/interfaces';
1212
import {SchemaMetadata} from '../../metadata/schema';
1313
import {Sanitizer} from '../../sanitization/sanitizer';
1414
import type {ReactiveLViewConsumer} from '../reactive_lview_consumer';
15-
import type {EffectManager} from '../reactivity/effect';
15+
import type {FlushableEffectRunner} from '../reactivity/effect';
1616
import type {AfterRenderEventManager} from '../after_render_hooks';
1717

1818
import {LContainer} from './container';
@@ -372,7 +372,7 @@ export interface LViewEnvironment {
372372
sanitizer: Sanitizer|null;
373373

374374
/** Container for reactivity system `effect`s. */
375-
effectManager: EffectManager|null;
375+
inlineEffectRunner: FlushableEffectRunner|null;
376376

377377
/** Container for after render hooks */
378378
afterRenderEventManager: AfterRenderEventManager|null;

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

Lines changed: 187 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -7,11 +7,14 @@
77
*/
88

99
import {assertInInjectionContext} from '../../di/contextual';
10+
import {InjectionToken} from '../../di/injection_token';
1011
import {Injector} from '../../di/injector';
1112
import {inject} from '../../di/injector_compatibility';
1213
import {ɵɵdefineInjectable} from '../../di/interface/defs';
14+
import {ErrorHandler} from '../../error_handler';
1315
import {DestroyRef} from '../../linker/destroy_ref';
14-
import {Watch, watch} from '../../signals';
16+
import {isInNotificationPhase, watch, Watch, WatchCleanupFn, WatchCleanupRegisterFn} from '../../signals';
17+
1518

1619
/**
1720
* An effect can, optionally, register a cleanup function. If registered, the cleanup is executed
@@ -27,73 +30,201 @@ export type EffectCleanupFn = () => void;
2730
*/
2831
export type EffectCleanupRegisterFn = (cleanupFn: EffectCleanupFn) => void;
2932

33+
export interface SchedulableEffect {
34+
run(): void;
35+
creationZone: unknown;
36+
}
37+
3038
/**
31-
* Tracks all effects registered within a given application and runs them via `flush`.
39+
* Not public API, which guarantees `EffectScheduler` only ever comes from the application root
40+
* injector.
3241
*/
33-
export class EffectManager {
34-
private all = new Set<Watch>();
35-
private queue = new Map<Watch, Zone|null>();
36-
37-
create(
38-
effectFn: (onCleanup: (cleanupFn: EffectCleanupFn) => void) => void,
39-
destroyRef: DestroyRef|null, allowSignalWrites: boolean): EffectRef {
40-
const zone = (typeof Zone === 'undefined') ? null : Zone.current;
41-
const w = watch(effectFn, (watch) => {
42-
if (!this.all.has(watch)) {
43-
return;
44-
}
45-
46-
this.queue.set(watch, zone);
47-
}, allowSignalWrites);
42+
export const APP_EFFECT_SCHEDULER = new InjectionToken('', {
43+
providedIn: 'root',
44+
factory: () => inject(EffectScheduler),
45+
});
4846

49-
this.all.add(w);
50-
51-
// Effects start dirty.
52-
w.notify();
47+
/**
48+
* A scheduler which manages the execution of effects.
49+
*/
50+
export abstract class EffectScheduler {
51+
/**
52+
* Schedule the given effect to be executed at a later time.
53+
*
54+
* It is an error to attempt to execute any effects synchronously during a scheduling operation.
55+
*/
56+
abstract scheduleEffect(e: SchedulableEffect): void;
5357

54-
let unregisterOnDestroy: (() => void)|undefined;
58+
/** @nocollapse */
59+
static ɵprov = /** @pureOrBreakMyCode */ ɵɵdefineInjectable({
60+
token: EffectScheduler,
61+
providedIn: 'root',
62+
factory: () => new ZoneAwareMicrotaskScheduler(),
63+
});
64+
}
5565

56-
const destroy = () => {
57-
w.cleanup();
58-
unregisterOnDestroy?.();
59-
this.all.delete(w);
60-
this.queue.delete(w);
61-
};
66+
/**
67+
* Interface to an `EffectScheduler` capable of running scheduled effects synchronously.
68+
*/
69+
export interface FlushableEffectRunner {
70+
/**
71+
* Run any scheduled effects.
72+
*/
73+
flush(): void;
74+
}
6275

63-
unregisterOnDestroy = destroyRef?.onDestroy(destroy);
76+
/**
77+
* An `EffectScheduler` which is capable of queueing scheduled effects per-zone, and flushing them
78+
* as an explicit operation.
79+
*/
80+
export class ZoneAwareQueueingScheduler implements EffectScheduler, FlushableEffectRunner {
81+
private queuedEffectCount = 0;
82+
private queues = new Map<Zone|null, Set<SchedulableEffect>>();
6483

65-
return {
66-
destroy,
67-
};
68-
}
84+
scheduleEffect(handle: SchedulableEffect): void {
85+
const zone = handle.creationZone as Zone | null;
86+
if (!this.queues.has(zone)) {
87+
this.queues.set(zone, new Set());
88+
}
6989

70-
flush(): void {
71-
if (this.queue.size === 0) {
90+
const queue = this.queues.get(zone)!;
91+
if (queue.has(handle)) {
7292
return;
7393
}
94+
this.queuedEffectCount++;
95+
queue.add(handle);
96+
}
7497

75-
for (const [watch, zone] of this.queue) {
76-
this.queue.delete(watch);
77-
if (zone) {
78-
zone.run(() => watch.run());
79-
} else {
80-
watch.run();
98+
/**
99+
* Run all scheduled effects.
100+
*
101+
* Execution order of effects within the same zone is guaranteed to be FIFO, but there is no
102+
* ordering guarantee between effects scheduled in different zones.
103+
*/
104+
flush(): void {
105+
while (this.queuedEffectCount > 0) {
106+
for (const [zone, queue] of this.queues) {
107+
// `zone` here must be defined.
108+
if (zone === null) {
109+
this.flushQueue(queue);
110+
} else {
111+
zone.run(() => this.flushQueue(queue));
112+
}
81113
}
82114
}
83115
}
84116

85-
get isQueueEmpty(): boolean {
86-
return this.queue.size === 0;
117+
private flushQueue(queue: Set<SchedulableEffect>): void {
118+
for (const handle of queue) {
119+
queue.delete(handle);
120+
this.queuedEffectCount--;
121+
122+
// TODO: what happens if this throws an error?
123+
handle.run();
124+
}
87125
}
88126

89127
/** @nocollapse */
90128
static ɵprov = /** @pureOrBreakMyCode */ ɵɵdefineInjectable({
91-
token: EffectManager,
129+
token: ZoneAwareQueueingScheduler,
92130
providedIn: 'root',
93-
factory: () => new EffectManager(),
131+
factory: () => new ZoneAwareQueueingScheduler(),
94132
});
95133
}
96134

135+
/**
136+
* A wrapper around `ZoneAwareQueueingScheduler` that schedules flushing via the microtask queue
137+
* when.
138+
*/
139+
export class ZoneAwareMicrotaskScheduler implements EffectScheduler {
140+
private hasQueuedFlush = false;
141+
private delegate = new ZoneAwareQueueingScheduler();
142+
private flushTask = () => {
143+
// Leave `hasQueuedFlush` as `true` so we don't queue another microtask if more effects are
144+
// scheduled during flushing. The flush of the `ZoneAwareQueueingScheduler` delegate is
145+
// guaranteed to empty the queue.
146+
this.delegate.flush();
147+
this.hasQueuedFlush = false;
148+
149+
// This is a variable initialization, not a method.
150+
// tslint:disable-next-line:semicolon
151+
};
152+
153+
scheduleEffect(handle: SchedulableEffect): void {
154+
this.delegate.scheduleEffect(handle);
155+
156+
if (!this.hasQueuedFlush) {
157+
queueMicrotask(this.flushTask);
158+
this.hasQueuedFlush = true;
159+
}
160+
}
161+
}
162+
163+
/**
164+
* Core reactive node for an Angular effect.
165+
*
166+
* `EffectHandle` combines the reactive graph's `Watch` base node for effects with the framework's
167+
* scheduling abstraction (`EffectScheduler`) as well as automatic cleanup via `DestroyRef` if
168+
* available/requested.
169+
*/
170+
class EffectHandle implements EffectRef, SchedulableEffect {
171+
private alive = true;
172+
unregisterOnDestroy: (() => void)|undefined;
173+
protected watcher: Watch;
174+
175+
constructor(
176+
private scheduler: EffectScheduler,
177+
private effectFn: (onCleanup: EffectCleanupRegisterFn) => void,
178+
public creationZone: Zone|null, destroyRef: DestroyRef|null,
179+
private errorHandler: ErrorHandler|null, allowSignalWrites: boolean) {
180+
this.watcher =
181+
watch((onCleanup) => this.runEffect(onCleanup), () => this.schedule(), allowSignalWrites);
182+
this.unregisterOnDestroy = destroyRef?.onDestroy(() => this.destroy());
183+
}
184+
185+
private runEffect(onCleanup: WatchCleanupRegisterFn): void {
186+
if (!this.alive) {
187+
// Running a destroyed effect is a no-op.
188+
return;
189+
}
190+
if (ngDevMode && isInNotificationPhase()) {
191+
throw new Error(`Schedulers cannot synchronously execute effects while scheduling.`);
192+
}
193+
194+
try {
195+
this.effectFn(onCleanup);
196+
} catch (err) {
197+
this.errorHandler?.handleError(err);
198+
}
199+
}
200+
201+
run(): void {
202+
this.watcher.run();
203+
}
204+
205+
private schedule(): void {
206+
if (!this.alive) {
207+
return;
208+
}
209+
210+
this.scheduler.scheduleEffect(this);
211+
}
212+
213+
notify(): void {
214+
this.watcher.notify();
215+
}
216+
217+
destroy(): void {
218+
this.alive = false;
219+
220+
this.watcher.cleanup();
221+
this.unregisterOnDestroy?.();
222+
223+
// Note: if the effect is currently scheduled, it's not un-scheduled, and so the scheduler will
224+
// retain a reference to it. Attempting to execute it will be a no-op.
225+
}
226+
}
227+
97228
/**
98229
* A global reactive effect, which can be manually destroyed.
99230
*
@@ -147,7 +278,16 @@ export function effect(
147278
options?: CreateEffectOptions): EffectRef {
148279
!options?.injector && assertInInjectionContext(effect);
149280
const injector = options?.injector ?? inject(Injector);
150-
const effectManager = injector.get(EffectManager);
281+
const errorHandler = injector.get(ErrorHandler, null, {optional: true});
151282
const destroyRef = options?.manualCleanup !== true ? injector.get(DestroyRef) : null;
152-
return effectManager.create(effectFn, destroyRef, !!options?.allowSignalWrites);
283+
284+
const handle = new EffectHandle(
285+
injector.get(APP_EFFECT_SCHEDULER), effectFn,
286+
(typeof Zone === 'undefined') ? null : Zone.current, destroyRef, errorHandler,
287+
options?.allowSignalWrites ?? false);
288+
289+
// Effects start dirty.
290+
handle.notify();
291+
292+
return handle;
153293
}

packages/core/src/signals/index.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,8 @@
99
export {defaultEquals, isSignal, Signal, SIGNAL, ValueEqualityFn} from './src/api';
1010
export {computed, CreateComputedOptions} from './src/computed';
1111
export {setThrowInvalidWriteToSignalError} from './src/errors';
12-
export {consumerAfterComputation, consumerBeforeComputation, consumerDestroy, producerAccessed, producerNotifyConsumers, producerUpdatesAllowed, producerUpdateValueVersion, REACTIVE_NODE, ReactiveNode, setActiveConsumer} from './src/graph';
12+
export {consumerAfterComputation, consumerBeforeComputation, consumerDestroy, isInNotificationPhase, producerAccessed, producerNotifyConsumers, producerUpdatesAllowed, producerUpdateValueVersion, REACTIVE_NODE, ReactiveNode, setActiveConsumer} from './src/graph';
1313
export {CreateSignalOptions, setPostSignalSetFn, signal, WritableSignal} from './src/signal';
1414
export {untracked} from './src/untracked';
15-
export {Watch, watch, WatchCleanupFn} from './src/watch';
15+
export {Watch, watch, WatchCleanupFn, WatchCleanupRegisterFn} from './src/watch';
1616
export {setAlternateWeakRefImpl} from './src/weak_ref';

packages/core/src/signals/src/graph.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,10 @@ export function setActiveConsumer(consumer: ReactiveNode|null): ReactiveNode|nul
2626
return prev;
2727
}
2828

29+
export function isInNotificationPhase(): boolean {
30+
return inNotificationPhase;
31+
}
32+
2933
export const REACTIVE_NODE = {
3034
version: 0 as Version,
3135
dirty: false,

0 commit comments

Comments
 (0)