Skip to content

Commit

Permalink
fix(core): untrack various core operations
Browse files Browse the repository at this point in the history
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

There are likely other APIs which would benefit from this approach, but this
is a start.
  • Loading branch information
alxhub committed Feb 27, 2024
1 parent 54b5a2f commit 1b2ee78
Show file tree
Hide file tree
Showing 12 changed files with 442 additions and 160 deletions.
4 changes: 3 additions & 1 deletion packages/core/src/application/application_ref.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@

import '../util/ng_jit_mode';

import {setThrowInvalidWriteToSignalError} from '@angular/core/primitives/signals';
import {setActiveConsumer, setThrowInvalidWriteToSignalError} from '@angular/core/primitives/signals';
import {Observable} from 'rxjs';
import {first, map} from 'rxjs/operators';

Expand Down Expand Up @@ -493,6 +493,7 @@ export class ApplicationRef {
ngDevMode && 'ApplicationRef.tick is called recursively');
}

const prevConsumer = setActiveConsumer(null);
try {
this._runningTick = true;

Expand All @@ -508,6 +509,7 @@ export class ApplicationRef {
this.internalErrorHandler(e);
} finally {
this._runningTick = false;
setActiveConsumer(prevConsumer);
}
}

Expand Down
40 changes: 25 additions & 15 deletions packages/core/src/di/r3_injector.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ import {NullInjector} from './null_injector';
import {isExistingProvider, isFactoryProvider, isTypeProvider, isValueProvider, SingleProvider} from './provider_collection';
import {ProviderToken} from './provider_token';
import {INJECTOR_SCOPE, InjectorScope} from './scope';
import {setActiveConsumer} from '@angular/core/primitives/signals';

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

// Set destroyed = true first, in case lifecycle hooks re-enter destroy().
this._destroyed = true;
const prevConsumer = setActiveConsumer(null);
try {
// Call all the lifecycle hooks.
for (const service of this._ngOnDestroyHooks) {
Expand All @@ -211,6 +213,7 @@ export class R3Injector extends EnvironmentInjector {
this.records.clear();
this._ngOnDestroyHooks.clear();
this.injectorDefTypes.clear();
setActiveConsumer(prevConsumer);
}
}

Expand Down Expand Up @@ -322,6 +325,7 @@ export class R3Injector extends EnvironmentInjector {

/** @internal */
resolveInjectorInitializers() {
const prevConsumer = setActiveConsumer(null);
const previousInjector = setCurrentInjector(this);
const previousInjectImplementation = setInjectImplementation(undefined);
let prevInjectContext: InjectorProfilerContext|undefined;
Expand All @@ -346,6 +350,7 @@ export class R3Injector extends EnvironmentInjector {
setCurrentInjector(previousInjector);
setInjectImplementation(previousInjectImplementation);
ngDevMode && setInjectorProfilerContext(prevInjectContext!);
setActiveConsumer(prevConsumer);
}
}

Expand Down Expand Up @@ -419,24 +424,29 @@ export class R3Injector extends EnvironmentInjector {
}

private hydrate<T>(token: ProviderToken<T>, record: Record<T>): T {
if (ngDevMode && record.value === CIRCULAR) {
throwCyclicDependencyError(stringify(token));
} else if (record.value === NOT_YET) {
record.value = CIRCULAR;

if (ngDevMode) {
runInInjectorProfilerContext(this, token as Type<T>, () => {
const prevConsumer = setActiveConsumer(null);
try {
if (ngDevMode && record.value === CIRCULAR) {
throwCyclicDependencyError(stringify(token));
} else if (record.value === NOT_YET) {
record.value = CIRCULAR;

if (ngDevMode) {
runInInjectorProfilerContext(this, token as Type<T>, () => {
record.value = record.factory!();
emitInstanceCreatedByInjectorEvent(record.value);
});
} else {
record.value = record.factory!();
emitInstanceCreatedByInjectorEvent(record.value);
});
} else {
record.value = record.factory!();
}
}
if (typeof record.value === 'object' && record.value && hasOnDestroy(record.value)) {
this._ngOnDestroyHooks.add(record.value);
}
return record.value as T;
} finally {
setActiveConsumer(prevConsumer);
}
if (typeof record.value === 'object' && record.value && hasOnDestroy(record.value)) {
this._ngOnDestroyHooks.add(record.value);
}
return record.value as T;
}

private injectableDefInScope(def: ɵɵInjectableDeclaration<any>): boolean {
Expand Down
9 changes: 8 additions & 1 deletion packages/core/src/event_emitter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@

import {PartialObserver, Subject, Subscription} from 'rxjs';

import {setActiveConsumer} from '../primitives/signals';

/**
* Use in components with the `@Output` directive to emit custom events
* synchronously or asynchronously, and register handlers for those events
Expand Down Expand Up @@ -109,7 +111,12 @@ class EventEmitter_ extends Subject<any> {
}

emit(value?: any) {
super.next(value);
const prevConsumer = setActiveConsumer(null);
try {
super.next(value);
} finally {
setActiveConsumer(prevConsumer);
}
}

override subscribe(observerOrNext?: any, error?: any, complete?: any): Subscription {
Expand Down
Loading

0 comments on commit 1b2ee78

Please sign in to comment.