Skip to content

Commit 06e74a5

Browse files
alxhubatscott
alxhub
authored and
atscott
committed
refactor(core): run effects during change detection (#49641)
This commit updates the `effect` primitive and significantly changes the timing of effect execution. Previously, effects were scheduled via the microtask queue. This commit changes effects to run throughout the change detection process instead. Running effects this way avoids needing additional rounds of change detection to resolve effects, with the tradeoff that they're harder to use for model-to-model synchronization (which can be seen as a good thing). PR Close #49641
1 parent e254671 commit 06e74a5

File tree

17 files changed

+234
-69
lines changed

17 files changed

+234
-69
lines changed

‎packages/core/src/render3/component_ref.ts‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@ import {CONTEXT, HEADER_OFFSET, INJECTOR, LView, LViewEnvironment, LViewFlags, T
4242
import {MATH_ML_NAMESPACE, SVG_NAMESPACE} from './namespaces';
4343
import {createElementNode, setupStaticAttributes, writeDirectClass} from './node_manipulation';
4444
import {extractAttrsAndClassesFromSelector, stringifyCSSSelectorList} from './node_selector_matcher';
45+
import {EffectManager} from './reactivity/effect';
4546
import {enterView, getCurrentTNode, getLView, leaveView} from './state';
4647
import {computeStaticStyling} from './styling/static_styling';
4748
import {mergeHostAttrs, setUpAttributes} from './util/attrs_utils';
@@ -165,9 +166,12 @@ export class ComponentFactory<T> extends AbstractComponentFactory<T> {
165166
}
166167
const sanitizer = rootViewInjector.get(Sanitizer, null);
167168

169+
const effectManager = rootViewInjector.get(EffectManager, null);
170+
168171
const environment: LViewEnvironment = {
169172
rendererFactory,
170173
sanitizer,
174+
effectManager,
171175
};
172176

173177
const hostRenderer = rendererFactory.createRenderer(null, this.componentDef);

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

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -365,10 +365,14 @@ export function refreshView<T>(
365365
ngDevMode && assertEqual(isCreationMode(lView), false, 'Should be run in update mode');
366366
const flags = lView[FLAGS];
367367
if ((flags & LViewFlags.Destroyed) === LViewFlags.Destroyed) return;
368-
enterView(lView);
368+
369369
// Check no changes mode is a dev only mode used to verify that bindings have not changed
370370
// since they were assigned. We do not want to execute lifecycle hooks in that mode.
371371
const isInCheckNoChangesPass = ngDevMode && isInCheckNoChangesMode();
372+
373+
!isInCheckNoChangesPass && lView[ENVIRONMENT].effectManager?.flush();
374+
375+
enterView(lView);
372376
try {
373377
resetPreOrderHookFlags(lView);
374378

@@ -1819,6 +1823,10 @@ export function detectChangesInternal<T>(
18191823
throw error;
18201824
} finally {
18211825
if (!checkNoChangesMode && rendererFactory.end) rendererFactory.end();
1826+
1827+
// One final flush of the effects queue to catch any effects created in `ngAfterViewInit` or
1828+
// other post-order hooks.
1829+
!checkNoChangesMode && lView[ENVIRONMENT].effectManager?.flush();
18221830
}
18231831
}
18241832

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

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +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';
1516

1617
import {LContainer} from './container';
1718
import {ComponentDef, ComponentTemplate, DirectiveDef, DirectiveDefList, HostBindingsFunction, PipeDef, PipeDefList, ViewQueriesFunction} from './definition';
@@ -367,6 +368,9 @@ export interface LViewEnvironment {
367368

368369
/** An optional custom sanitizer. */
369370
sanitizer: Sanitizer|null;
371+
372+
/** Container for reactivity system `effect`s. */
373+
effectManager: EffectManager|null;
370374
}
371375

372376
/** Flags associated with an LView (saved in LView[FLAGS]) */

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

Lines changed: 62 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
import {assertInInjectionContext} from '../../di/contextual';
1010
import {Injector} from '../../di/injector';
1111
import {inject} from '../../di/injector_compatibility';
12+
import {ɵɵdefineInjectable} from '../../di/interface/defs';
1213
import {DestroyRef} from '../../linker/destroy_ref';
1314
import {Watch} from '../../signals';
1415

@@ -21,10 +22,66 @@ import {Watch} from '../../signals';
2122
*/
2223
export type EffectCleanupFn = () => void;
2324

24-
const globalWatches = new Set<Watch>();
25-
const queuedWatches = new Map<Watch, Zone>();
25+
/**
26+
* Tracks all effects registered within a given application and runs them via `flush`.
27+
*/
28+
export class EffectManager {
29+
private all = new Set<Watch>();
30+
private queue = new Map<Watch, Zone>();
31+
32+
create(effectFn: () => void, destroyRef: DestroyRef|null): EffectRef {
33+
const zone = Zone.current;
34+
const watch = new Watch(effectFn, (watch) => {
35+
if (!this.all.has(watch)) {
36+
return;
37+
}
38+
39+
this.queue.set(watch, zone);
40+
});
41+
42+
this.all.add(watch);
43+
44+
// Effects start dirty.
45+
watch.notify();
46+
47+
let unregisterOnDestroy: (() => void)|undefined;
48+
49+
const destroy = () => {
50+
watch.cleanup();
51+
unregisterOnDestroy?.();
52+
this.all.delete(watch);
53+
this.queue.delete(watch);
54+
};
55+
56+
unregisterOnDestroy = destroyRef?.onDestroy(destroy);
57+
58+
return {
59+
destroy,
60+
};
61+
}
62+
63+
flush(): void {
64+
if (this.queue.size === 0) {
65+
return;
66+
}
67+
68+
for (const [watch, zone] of this.queue) {
69+
this.queue.delete(watch);
70+
zone.run(() => watch.run());
71+
}
72+
}
2673

27-
let watchQueuePromise: {promise: Promise<void>; resolveFn: () => void;}|null = null;
74+
get isQueueEmpty(): boolean {
75+
return this.queue.size === 0;
76+
}
77+
78+
/** @nocollapse */
79+
static ɵprov = /** @pureOrBreakMyCode */ ɵɵdefineInjectable({
80+
token: EffectManager,
81+
providedIn: 'root',
82+
factory: () => new EffectManager(),
83+
});
84+
}
2885

2986
/**
3087
* A global reactive effect, which can be manually destroyed.
@@ -68,62 +125,8 @@ export interface CreateEffectOptions {
68125
export function effect(
69126
effectFn: () => EffectCleanupFn | void, options?: CreateEffectOptions): EffectRef {
70127
!options?.injector && assertInInjectionContext(effect);
71-
72-
const zone = Zone.current;
73-
const watch = new Watch(effectFn, (watch) => queueWatch(watch, zone));
74-
75128
const injector = options?.injector ?? inject(Injector);
129+
const effectManager = injector.get(EffectManager);
76130
const destroyRef = options?.manualCleanup !== true ? injector.get(DestroyRef) : null;
77-
78-
globalWatches.add(watch);
79-
80-
// Effects start dirty.
81-
watch.notify();
82-
83-
let unregisterOnDestroy: (() => void)|undefined;
84-
85-
const destroy = () => {
86-
watch.cleanup();
87-
unregisterOnDestroy?.();
88-
queuedWatches.delete(watch);
89-
globalWatches.delete(watch);
90-
};
91-
92-
unregisterOnDestroy = destroyRef?.onDestroy(destroy);
93-
94-
return {
95-
destroy,
96-
};
97-
}
98-
99-
function queueWatch(watch: Watch, zone: Zone): void {
100-
if (queuedWatches.has(watch) || !globalWatches.has(watch)) {
101-
return;
102-
}
103-
104-
queuedWatches.set(watch, zone);
105-
106-
if (watchQueuePromise === null) {
107-
Promise.resolve().then(runWatchQueue);
108-
109-
let resolveFn!: () => void;
110-
const promise = new Promise<void>((resolve) => {
111-
resolveFn = resolve;
112-
});
113-
114-
watchQueuePromise = {
115-
promise,
116-
resolveFn,
117-
};
118-
}
119-
}
120-
121-
function runWatchQueue(): void {
122-
for (const [watch, zone] of queuedWatches) {
123-
queuedWatches.delete(watch);
124-
zone.run(() => watch.run());
125-
}
126-
127-
watchQueuePromise!.resolveFn();
128-
watchQueuePromise = null;
131+
return effectManager.create(effectFn, destroyRef);
129132
}

‎packages/core/test/bundling/animations/bundle.golden_symbols.json‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -224,6 +224,9 @@
224224
{
225225
"name": "EVENT_MANAGER_PLUGINS"
226226
},
227+
{
228+
"name": "EffectManager"
229+
},
227230
{
228231
"name": "ElementInstructionMap"
229232
},
@@ -365,6 +368,9 @@
365368
{
366369
"name": "NG_TOKEN_PATH"
367370
},
371+
{
372+
"name": "NOOP_CLEANUP_FN"
373+
},
368374
{
369375
"name": "NOT_FOUND"
370376
},
@@ -602,6 +608,9 @@
602608
{
603609
"name": "ViewRef"
604610
},
611+
{
612+
"name": "Watch"
613+
},
605614
{
606615
"name": "WeakRefImpl"
607616
},

‎packages/core/test/bundling/cyclic_import/bundle.golden_symbols.json‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -131,6 +131,9 @@
131131
{
132132
"name": "EVENT_MANAGER_PLUGINS"
133133
},
134+
{
135+
"name": "EffectManager"
136+
},
134137
{
135138
"name": "ElementRef"
136139
},
@@ -266,6 +269,9 @@
266269
{
267270
"name": "NG_TOKEN_PATH"
268271
},
272+
{
273+
"name": "NOOP_CLEANUP_FN"
274+
},
269275
{
270276
"name": "NOT_FOUND"
271277
},
@@ -455,6 +461,9 @@
455461
{
456462
"name": "ViewRef"
457463
},
464+
{
465+
"name": "Watch"
466+
},
458467
{
459468
"name": "WeakRefImpl"
460469
},

‎packages/core/test/bundling/forms_reactive/bundle.golden_symbols.json‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -176,6 +176,9 @@
176176
{
177177
"name": "EVENT_MANAGER_PLUGINS"
178178
},
179+
{
180+
"name": "EffectManager"
181+
},
179182
{
180183
"name": "ElementRef"
181184
},
@@ -356,6 +359,9 @@
356359
{
357360
"name": "NG_VALUE_ACCESSOR"
358361
},
362+
{
363+
"name": "NOOP_CLEANUP_FN"
364+
},
359365
{
360366
"name": "NOT_FOUND"
361367
},
@@ -614,6 +620,9 @@
614620
{
615621
"name": "ViewRef"
616622
},
623+
{
624+
"name": "Watch"
625+
},
617626
{
618627
"name": "WeakRefImpl"
619628
},

‎packages/core/test/bundling/forms_template_driven/bundle.golden_symbols.json‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -179,6 +179,9 @@
179179
{
180180
"name": "EVENT_MANAGER_PLUGINS"
181181
},
182+
{
183+
"name": "EffectManager"
184+
},
182185
{
183186
"name": "ElementRef"
184187
},
@@ -338,6 +341,9 @@
338341
{
339342
"name": "NG_VALUE_ACCESSOR"
340343
},
344+
{
345+
"name": "NOOP_CLEANUP_FN"
346+
},
341347
{
342348
"name": "NOT_FOUND"
343349
},
@@ -605,6 +611,9 @@
605611
{
606612
"name": "ViewRef"
607613
},
614+
{
615+
"name": "Watch"
616+
},
608617
{
609618
"name": "WeakRefImpl"
610619
},

‎packages/core/test/bundling/hello_world/bundle.golden_symbols.json‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,9 @@
8080
{
8181
"name": "ERROR_ORIGINAL_ERROR"
8282
},
83+
{
84+
"name": "EffectManager"
85+
},
8386
{
8487
"name": "ElementRef"
8588
},
@@ -188,6 +191,9 @@
188191
{
189192
"name": "NG_TOKEN_PATH"
190193
},
194+
{
195+
"name": "NOOP_CLEANUP_FN"
196+
},
191197
{
192198
"name": "NOT_FOUND"
193199
},
@@ -347,6 +353,9 @@
347353
{
348354
"name": "ViewRef"
349355
},
356+
{
357+
"name": "Watch"
358+
},
350359
{
351360
"name": "WeakRefImpl"
352361
},

‎packages/core/test/bundling/router/bundle.golden_symbols.json‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -203,6 +203,9 @@
203203
{
204204
"name": "EVENT_MANAGER_PLUGINS"
205205
},
206+
{
207+
"name": "EffectManager"
208+
},
206209
{
207210
"name": "ElementRef"
208211
},
@@ -404,6 +407,9 @@
404407
{
405408
"name": "NONE"
406409
},
410+
{
411+
"name": "NOOP_CLEANUP_FN"
412+
},
407413
{
408414
"name": "NOT_FOUND"
409415
},
@@ -803,6 +809,9 @@
803809
{
804810
"name": "ViewRef"
805811
},
812+
{
813+
"name": "Watch"
814+
},
806815
{
807816
"name": "WeakRefImpl"
808817
},

0 commit comments

Comments
 (0)