Skip to content

Commit ffad7b8

Browse files
alxhubpkozlowski-opensource
alxhub
authored and
pkozlowski-opensource
committed
fix(core): untrack various core operations (#54614)
One downside of implicit dependency tracking in `effect()`s is that it's easy to for downstream code to end up running inside the effect context by accident. For example, if an effect raises an event (e.g. by `next()`ing a `Subject`), the subscribers to that `Observable` will run inside the effect's reactive context, and any signals read within the subscriber will end up as dependencies of the effect. This is why the `untracked` function is useful, to run certain operations without incidental signal reads ending up tracked. However, knowing when this is necessary is non-trivial. For example, injecting a dependency might cause it to be instantiated, which would run the constructor in the effect context unless the injection operation is untracked. Therefore, Angular will automatically drop the reactive context within a number of framework APIs. This commit addresses these use cases: * creating and destroying views * creating and destroying DI injectors * injecting dependencies * emitting outputs Fixes #54548 There are likely other APIs which would benefit from this approach, but this is a start. PR Close #54614
1 parent 9a8a544 commit ffad7b8

13 files changed

Lines changed: 447 additions & 160 deletions

File tree

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

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88

99
import '../util/ng_jit_mode';
1010

11-
import {setThrowInvalidWriteToSignalError} from '@angular/core/primitives/signals';
11+
import {setActiveConsumer, setThrowInvalidWriteToSignalError} from '@angular/core/primitives/signals';
1212
import {Observable} from 'rxjs';
1313
import {first, map} from 'rxjs/operators';
1414

@@ -493,6 +493,7 @@ export class ApplicationRef {
493493
ngDevMode && 'ApplicationRef.tick is called recursively');
494494
}
495495

496+
const prevConsumer = setActiveConsumer(null);
496497
try {
497498
this._runningTick = true;
498499

@@ -508,6 +509,7 @@ export class ApplicationRef {
508509
this.internalErrorHandler(e);
509510
} finally {
510511
this._runningTick = false;
512+
setActiveConsumer(prevConsumer);
511513
}
512514
}
513515

‎packages/core/src/di/r3_injector.ts‎

Lines changed: 25 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ import {NullInjector} from './null_injector';
3434
import {isExistingProvider, isFactoryProvider, isTypeProvider, isValueProvider, SingleProvider} from './provider_collection';
3535
import {ProviderToken} from './provider_token';
3636
import {INJECTOR_SCOPE, InjectorScope} from './scope';
37+
import {setActiveConsumer} from '@angular/core/primitives/signals';
3738

3839
/**
3940
* Marker which indicates that a value has not yet been created from the factory function.
@@ -194,6 +195,7 @@ export class R3Injector extends EnvironmentInjector {
194195

195196
// Set destroyed = true first, in case lifecycle hooks re-enter destroy().
196197
this._destroyed = true;
198+
const prevConsumer = setActiveConsumer(null);
197199
try {
198200
// Call all the lifecycle hooks.
199201
for (const service of this._ngOnDestroyHooks) {
@@ -211,6 +213,7 @@ export class R3Injector extends EnvironmentInjector {
211213
this.records.clear();
212214
this._ngOnDestroyHooks.clear();
213215
this.injectorDefTypes.clear();
216+
setActiveConsumer(prevConsumer);
214217
}
215218
}
216219

@@ -322,6 +325,7 @@ export class R3Injector extends EnvironmentInjector {
322325

323326
/** @internal */
324327
resolveInjectorInitializers() {
328+
const prevConsumer = setActiveConsumer(null);
325329
const previousInjector = setCurrentInjector(this);
326330
const previousInjectImplementation = setInjectImplementation(undefined);
327331
let prevInjectContext: InjectorProfilerContext|undefined;
@@ -346,6 +350,7 @@ export class R3Injector extends EnvironmentInjector {
346350
setCurrentInjector(previousInjector);
347351
setInjectImplementation(previousInjectImplementation);
348352
ngDevMode && setInjectorProfilerContext(prevInjectContext!);
353+
setActiveConsumer(prevConsumer);
349354
}
350355
}
351356

@@ -419,24 +424,29 @@ export class R3Injector extends EnvironmentInjector {
419424
}
420425

421426
private hydrate<T>(token: ProviderToken<T>, record: Record<T>): T {
422-
if (ngDevMode && record.value === CIRCULAR) {
423-
throwCyclicDependencyError(stringify(token));
424-
} else if (record.value === NOT_YET) {
425-
record.value = CIRCULAR;
426-
427-
if (ngDevMode) {
428-
runInInjectorProfilerContext(this, token as Type<T>, () => {
427+
const prevConsumer = setActiveConsumer(null);
428+
try {
429+
if (ngDevMode && record.value === CIRCULAR) {
430+
throwCyclicDependencyError(stringify(token));
431+
} else if (record.value === NOT_YET) {
432+
record.value = CIRCULAR;
433+
434+
if (ngDevMode) {
435+
runInInjectorProfilerContext(this, token as Type<T>, () => {
436+
record.value = record.factory!();
437+
emitInstanceCreatedByInjectorEvent(record.value);
438+
});
439+
} else {
429440
record.value = record.factory!();
430-
emitInstanceCreatedByInjectorEvent(record.value);
431-
});
432-
} else {
433-
record.value = record.factory!();
441+
}
434442
}
443+
if (typeof record.value === 'object' && record.value && hasOnDestroy(record.value)) {
444+
this._ngOnDestroyHooks.add(record.value);
445+
}
446+
return record.value as T;
447+
} finally {
448+
setActiveConsumer(prevConsumer);
435449
}
436-
if (typeof record.value === 'object' && record.value && hasOnDestroy(record.value)) {
437-
this._ngOnDestroyHooks.add(record.value);
438-
}
439-
return record.value as T;
440450
}
441451

442452
private injectableDefInScope(def: ɵɵInjectableDeclaration<any>): boolean {

‎packages/core/src/event_emitter.ts‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
* found in the LICENSE file at https://angular.io/license
77
*/
88

9+
import {setActiveConsumer} from '@angular/core/primitives/signals';
910
import {PartialObserver, Subject, Subscription} from 'rxjs';
1011

1112
/**
@@ -109,7 +110,12 @@ class EventEmitter_ extends Subject<any> {
109110
}
110111

111112
emit(value?: any) {
112-
super.next(value);
113+
const prevConsumer = setActiveConsumer(null);
114+
try {
115+
super.next(value);
116+
} finally {
117+
setActiveConsumer(prevConsumer);
118+
}
113119
}
114120

115121
override subscribe(observerOrNext?: any, error?: any, complete?: any): Subscription {

0 commit comments

Comments
 (0)