Skip to content

Commit

Permalink
refactor(core): Use single source of truth for `ApplicationRef.isStab…
Browse files Browse the repository at this point in the history 
…le` (#53576)

This commit updates the `ApplicationRef.isStable` implementation to use
a single `Observable` to manage the state. This simplifies the mental
model quite a bit and removes the need for rx operators like
`distinctUntilChanged` and `combineLatest`.

PR Close #53576
  • Loading branch information
@atscott
atscott committed Dec 19, 2023
1 parent d49333e commit 12181b9
Show file tree
Hide file tree
Showing 17 changed files with 94 additions and 1,091 deletions.
10 changes: 5 additions & 5 deletions goldens/size-tracking/integration-payloads.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,21 +2,21 @@
"cli-hello-world": {
"uncompressed": {
"runtime": 908,
"main": 134468,
"main": 127322,
"polyfills": 33792
}
},
"cli-hello-world-ivy-i18n": {
"uncompressed": {
"runtime": 926,
"main": 131777,
"main": 125263,
"polyfills": 34676
}
},
"cli-hello-world-lazy": {
"uncompressed": {
"runtime": 2734,
"main": 238106,
"main": 231349,
"polyfills": 33810,
"src_app_lazy_lazy_routes_ts": 487
}
Expand Down Expand Up @@ -49,14 +49,14 @@
"standalone-bootstrap": {
"uncompressed": {
"runtime": 918,
"main": 91485,
"main": 85055,
"polyfills": 33802
}
},
"defer": {
"uncompressed": {
"runtime": 2689,
"main": 121811,
"main": 115193,
"polyfills": 33807,
"src_app_defer_component_ts": 450
}
Expand Down
15 changes: 4 additions & 11 deletions packages/core/src/application/application_ref.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@
import '../util/ng_jit_mode';

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

import {getCompilerFacade, JitCompilerUsage} from '../compiler/compiler_facade';
import {Console} from '../console';
Expand Down Expand Up @@ -38,7 +38,7 @@ import {publishDefaultGlobalUtils as _publishDefaultGlobalUtils} from '../render
import {ViewRef as InternalViewRef} from '../render3/view_ref';
import {TESTABILITY} from '../testability/testability';
import {isPromise} from '../util/lang';
import {NgZone, ZONE_IS_STABLE_OBSERVABLE} from '../zone/ng_zone';
import {NgZone} from '../zone/ng_zone';

import {ApplicationInitStatus} from './application_init';

Expand Down Expand Up @@ -323,9 +323,6 @@ export class ApplicationRef {
/** @internal */
_views: InternalViewRef<unknown>[] = [];
private readonly internalErrorHandler = inject(INTERNAL_APPLICATION_ERROR_HANDLER);
private readonly zoneIsStable = inject(ZONE_IS_STABLE_OBSERVABLE, {optional: true}) ?? of(true);
private readonly noPendingTasks =
inject(PendingTasks).hasPendingTasks.pipe(map(pending => !pending));

/**
* Indicates whether this instance was destroyed.
Expand All @@ -349,11 +346,7 @@ export class ApplicationRef {
* Returns an Observable that indicates when the application is stable or unstable.
*/
public readonly isStable: Observable<boolean> =
combineLatest([this.zoneIsStable, this.noPendingTasks])
.pipe(
map(indicators => indicators.every(stable => stable)),
distinctUntilChanged(),
);
inject(PendingTasks).hasPendingTasks.pipe(map(pending => !pending));

private readonly _injector = inject(EnvironmentInjector);
/**
Expand Down
59 changes: 57 additions & 2 deletions packages/core/src/change_detection/scheduling.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,9 @@ import {ApplicationRef} from '../application/application_ref';
import {ENVIRONMENT_INITIALIZER, EnvironmentProviders, inject, Injectable, InjectionToken, makeEnvironmentProviders, StaticProvider} from '../di';
import {ErrorHandler, INTERNAL_APPLICATION_ERROR_HANDLER} from '../error_handler';
import {RuntimeError, RuntimeErrorCode} from '../errors';
import {PendingTasks} from '../pending_tasks';
import {NgZone} from '../zone';
import {InternalNgZoneOptions, isStableFactory, ZONE_IS_STABLE_OBSERVABLE} from '../zone/ng_zone';
import {InternalNgZoneOptions} from '../zone/ng_zone';

@Injectable({providedIn: 'root'})
export class NgZoneChangeDetectionScheduler {
Expand Down Expand Up @@ -68,8 +69,17 @@ export function internalProvideZoneChangeDetection(ngZoneFactory: () => NgZone):
return () => ngZoneChangeDetectionScheduler!.initialize();
},
},
{
provide: ENVIRONMENT_INITIALIZER,
multi: true,
useFactory: () => {
const service = inject(ZoneStablePendingTask);
return () => {
service.initialize();
};
}
},
{provide: INTERNAL_APPLICATION_ERROR_HANDLER, useFactory: ngZoneApplicationErrorHandlerFactory},
{provide: ZONE_IS_STABLE_OBSERVABLE, useFactory: isStableFactory},
];
}

Expand Down Expand Up @@ -171,3 +181,48 @@ export function getNgZoneOptions(options?: NgZoneOptions): InternalNgZoneOptions
shouldCoalesceRunChangeDetection: options?.runCoalescing ?? false,
};
}

@Injectable({providedIn: 'root'})
export class ZoneStablePendingTask {
private readonly subscription = new Subscription();
private initialized = false;
private readonly zone = inject(NgZone);
private readonly pendingTasks = inject(PendingTasks);

initialize() {
if (this.initialized) {
return;
}
this.initialized = true;

let task: number|null = null;
if (!this.zone.isStable && !this.zone.hasPendingMacrotasks && !this.zone.hasPendingMicrotasks) {
task = this.pendingTasks.add();
}

this.zone.runOutsideAngular(() => {
this.subscription.add(this.zone.onStable.subscribe(() => {
NgZone.assertNotInAngularZone();

// Check whether there are no pending macro/micro tasks in the next tick
// to allow for NgZone to update the state.
queueMicrotask(() => {
if (task !== null && !this.zone.hasPendingMacrotasks && !this.zone.hasPendingMicrotasks) {
this.pendingTasks.remove(task);
task = null;
}
});
}));
});

this.subscription.add(this.zone.onUnstable.subscribe(() => {
NgZone.assertInAngularZone();
task ??= this.pendingTasks.add();
}));
}


ngOnDestroy() {
this.subscription.unsubscribe();
}
}
13 changes: 10 additions & 3 deletions packages/core/src/pending_tasks.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,24 +25,31 @@ import {OnDestroy} from './interface/lifecycle_hooks';
export class PendingTasks implements OnDestroy {
private taskId = 0;
private pendingTasks = new Set<number>();
private get _hasPendingTasks() {
return this.hasPendingTasks.value;
}
hasPendingTasks = new BehaviorSubject<boolean>(false);

add(): number {
this.hasPendingTasks.next(true);
if (!this._hasPendingTasks) {
this.hasPendingTasks.next(true);
}
const taskId = this.taskId++;
this.pendingTasks.add(taskId);
return taskId;
}

remove(taskId: number): void {
this.pendingTasks.delete(taskId);
if (this.pendingTasks.size === 0) {
if (this.pendingTasks.size === 0 && this._hasPendingTasks) {
this.hasPendingTasks.next(false);
}
}

ngOnDestroy(): void {
this.pendingTasks.clear();
this.hasPendingTasks.next(false);
if (this._hasPendingTasks) {
this.hasPendingTasks.next(false);
}
}
}
2 changes: 1 addition & 1 deletion packages/core/src/render3/after_render_hooks.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import {RuntimeError, RuntimeErrorCode} from '../errors';
import {DestroyRef} from '../linker/destroy_ref';
import {assertGreaterThan} from '../util/assert';
import {performanceMarkFeature} from '../util/performance';
import {NgZone} from '../zone';
import {NgZone} from '../zone/ng_zone';

import {isPlatformBrowser} from './util/misc_utils';

Expand Down
56 changes: 0 additions & 56 deletions packages/core/src/zone/ng_zone.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,7 @@
* found in the LICENSE file at https://angular.io/license
*/

import {merge, Observable, Observer, Subscription} from 'rxjs';
import {share} from 'rxjs/operators';

import {inject, InjectionToken} from '../di';
import {RuntimeError, RuntimeErrorCode} from '../errors';
import {EventEmitter} from '../event_emitter';
import {global} from '../util/global';
Expand Down Expand Up @@ -526,59 +523,6 @@ export class NoopNgZone implements NgZone {
}
}

/**
* Token used to drive ApplicationRef.isStable
*/
export const ZONE_IS_STABLE_OBSERVABLE =
new InjectionToken<Observable<boolean>>(ngDevMode ? 'zone isStable Observable' : '');

export function isStableFactory() {
const zone = inject(NgZone);
let _stable = true;
const isCurrentlyStable = new Observable<boolean>((observer: Observer<boolean>) => {
_stable = zone.isStable && !zone.hasPendingMacrotasks && !zone.hasPendingMicrotasks;
zone.runOutsideAngular(() => {
observer.next(_stable);
observer.complete();
});
});

const isStable = new Observable<boolean>((observer: Observer<boolean>) => {
// Create the subscription to onStable outside the Angular Zone so that
// the callback is run outside the Angular Zone.
let stableSub: Subscription;
zone.runOutsideAngular(() => {
stableSub = zone.onStable.subscribe(() => {
NgZone.assertNotInAngularZone();

// Check whether there are no pending macro/micro tasks in the next tick
// to allow for NgZone to update the state.
queueMicrotask(() => {
if (!_stable && !zone.hasPendingMacrotasks && !zone.hasPendingMicrotasks) {
_stable = true;
observer.next(true);
}
});
});
});

const unstableSub: Subscription = zone.onUnstable.subscribe(() => {
NgZone.assertInAngularZone();
if (_stable) {
_stable = false;
zone.runOutsideAngular(() => {
observer.next(false);
});
}
});

return () => {
stableSub.unsubscribe();
unstableSub.unsubscribe();
};
});
return merge(isCurrentlyStable, isStable.pipe(share()));
}

function shouldBeIgnoredByZone(applyArgs: unknown): boolean {
if (!Array.isArray(applyArgs)) {
Expand Down

0 comments on commit 12181b9

Please sign in to comment.